Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

oldSub[2] is not a function #1105

Open
IamCarbonMan opened this issue Jan 18, 2023 · 3 comments
Open

oldSub[2] is not a function #1105

IamCarbonMan opened this issue Jan 18, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@IamCarbonMan
Copy link

when stopping a subscription- by &&ing it with a falsy state value in app()- and then restarting it, I get an error "oldSub[2] is not a function". It looks like it's trying to call the 3rd element of the subscription array but according to the docs a subscription array only has 2 elements

@IamCarbonMan
Copy link
Author

https://github.com/IamCarbonMan/pokegotchi

code is here. a bit messy and written in coffeescript but as far as I can tell everything meets the docs

@jorgebucaran jorgebucaran added the bug Something isn't working label Jan 20, 2023
@mshgh
Copy link

mshgh commented May 6, 2023

I cannot read coffeescript, but I guess this would be the same issue I run into when using subscriptions.
I cannot see anything in the documentation related to the return value of subscription unsubscribe handler, but based on the source code the only meaningful return values are undefined or false

see line 59 in

hyperapp/index.js

Lines 48 to 59 in c3717e3

subs.push(
newSub && newSub !== true
? !oldSub ||
newSub[0] !== oldSub[0] ||
shouldRestart(newSub[1], oldSub[1])
? [
newSub[0],
newSub[1],
(oldSub && oldSub[2](), newSub[0](dispatch, newSub[1])),
]
: oldSub
: oldSub && oldSub[2]()

in may case my issue was caused by using one-liner. this is failing

  ...
  return () => function_returning_something()
}

while this works perfectly fine

  ...
  return () => {
    function_returning_something()
  }
}

I am curious if changing the line 59 like this would be the fix?

        : (oldSub && oldSub[2](), false)

@jorgebucaran
Copy link
Owner

If you'd like, feel free to send over a patch along with a test script. If you need a hand with writing the test, just let me know and I'll be happy to help you out or show you how to get started. I hope that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants