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

Uncaught TypeError: this.unsubscribe is not a function #28

Closed
aaronjensen opened this issue Aug 7, 2015 · 15 comments
Closed

Uncaught TypeError: this.unsubscribe is not a function #28

aaronjensen opened this issue Aug 7, 2015 · 15 comments
Labels

Comments

@aaronjensen
Copy link

Occasionally, when clicking through our app we get this:

oliver 2015-08-07 11-01-56

Is it possible for will unmount to fire before mount? Should we check to ensure unsubscribe is there before calling it or is something else going on?

@aaronjensen
Copy link
Author

I believe this has to do w/ some incorrect stuff that was happening in the app, though I don't know exactly what yet. This may be something that should never normally happen.

@aaronjensen
Copy link
Author

Ok, so here's what we were doing... in componentWillMount we fired an action via setTimeout so that the react-redux subscription in componentDidMount will have already happened by the time the action is dispatched. It appears that this is a bad idea, though I do not understand why.

@gaearon gaearon added the bug label Aug 7, 2015
@gaearon
Copy link
Contributor

gaearon commented Aug 7, 2015

OK I introduced this bug while tweaking @gnoff's code. :-(
Will push out a fix soon.

@gaearon gaearon closed this as completed in bda582b Aug 7, 2015
@gaearon
Copy link
Contributor

gaearon commented Aug 7, 2015

Can you verify the bug is fixed in 0.5.1?

@aaronjensen
Copy link
Author

yep, it is. But then I get this:

Uncaught Error: Invariant Violation: Component (with keys: getDOMNode,_handleChange,props,context,state,refs,_reactInternalInstance) contains `render` method but is not mounted in the DOM

which makes me think we're doing something very wrong. By removing the setTimeout in componentWillMount it works out.

What's the right way to fire an action when a component mounts? If the action immediately modifies state, componentWillMount is too soon, because react-redux doesn't subscribe until componentDidMount...

@gaearon
Copy link
Contributor

gaearon commented Aug 7, 2015

contains render method but is not mounted in the DOM

I found this: facebook/react#4233.

Sounds like React throws if you use findDOMNode(this) when component is already unmounted. You should clear any previously set interval in componentWillUnmount.

If the action immediately modifies state, componentWillMount is too soon, because react-redux doesn't subscribe until componentDidMount.

What do you mean by "too soon"? If React Redux doesn't update in some edge case, it's a bug, no matter where you call dispatch! You should file an issue, not just accept it like that. 😄

@aaronjensen
Copy link
Author

I just assume that you've thought of everything and there's a good reason it is as it is! I'll make sure it is what I think it is and file an issue when I get a sec. Thanks!

@gaearon gaearon reopened this Aug 7, 2015
@gaearon
Copy link
Contributor

gaearon commented Aug 7, 2015

Keeping it open for now so I don't forget about it.
I make mistakes all the time, you should help me figure them out!

@gaearon
Copy link
Contributor

gaearon commented Aug 7, 2015

Ideally, the fastest way to figure out a solution is to discuss a failing test. Can I ask you to contribute it?

@aaronjensen
Copy link
Author

Looks like you beat me to it! feel free to close #31, thanks. Btw, is there a reason to do it this way instead of by subscribing in componentWillMount?

@gaearon
Copy link
Contributor

gaearon commented Aug 8, 2015

Yes, componentWillMount executes on server. This would cause memory leaks.

@aaronjensen
Copy link
Author

The second error I saw is actually this: facebook/react#2410

I'm not sure why yet, in my case, it has to do w/ clicking submit on a form triggering a router transition. I'll keep digging

@gaearon
Copy link
Contributor

gaearon commented Aug 8, 2015

If you can reliably reproduce it on a small project let the folks know. It could be a React bug after all.

Are you by chance using React.addons.batchedUpdates or similar? Not saying I understand the issue but if so, try removing it and check if it makes the bug go away. I've heard about problems with batching and inputs although not sure what kind. Sorry if this is too vague. :-)

@aaronjensen
Copy link
Author

We are, but removing it has no effect. I'll work on a small repro.

@aaronjensen
Copy link
Author

You may be following that thread, so you may have seen it, but here's a repro: https://github.com/aaronjensen/react-2410-repro

The error does not happen if this line is removed: https://github.com/aaronjensen/react-2410-repro/blob/master/app/create_router.jsx#L10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants