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

Can't call setState (or forceUpdate) on an unmounted component. #336

Closed
drcmda opened this issue Aug 16, 2018 · 7 comments
Closed

Can't call setState (or forceUpdate) on an unmounted component. #336

drcmda opened this issue Aug 16, 2018 · 7 comments

Comments

@drcmda
Copy link

drcmda commented Aug 16, 2018

Full message:

Warning: Can't call setState (or forceUpdate) on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method.
in ParentSize (created by ItemList)

This seems to happen because of https://github.com/hshoff/vx/blob/master/packages/vx-responsive/src/components/ParentSize.js#L18 The component lives inside a transition that removes it conditionally, the raf probably comes a single frame too late.

I think the best way around it would be to track mounted state in componentDidMount/componentWillUnmount: https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html , and then checking for it before doing setState.

@hshoff
Copy link
Member

hshoff commented Aug 16, 2018

so https://github.com/hshoff/vx/blob/master/packages/vx-responsive/src/components/ParentSize.js#L18 was just merged in yesterday but hasn't been published yet (#335).

Just a few question to help get this fixed: Which version of @vx/responsive are you using? also which browser and version do you see this? and finally, do you have a repro of the bug so I can ensure that this gets fixed? (no worries if not)

@drcmda
Copy link
Author

drcmda commented Aug 16, 2018

Oops, i was just browsing the code looking for what could've caused it, but setState in a raf can be troublesome when a transitiongroup wraps it, adding an this.mounted flag will prevent this warning from ever occuring. My dependencies are quite outdated atm: "@vx/responsive": "0.0.158" This version calls setState in a resizeObserver cb.

@hshoff
Copy link
Member

hshoff commented Aug 16, 2018

I'll add a is mounted guard around the setState call in the raf version and publish it in 0.0.172 and we can see if that fixes it for the transitiongroup case.

edit: actually shouldn't the window.cancelAnimationFrame in componentWillUnmount() take care of making sure that the setState in the raf will not be called after componentWillUnmount is called? let's try publishing it as is and then we can add the mounted guard if we still see the warning in the transitiongroup.

@hshoff
Copy link
Member

hshoff commented Aug 16, 2018

v0.0.172 is published: https://github.com/hshoff/vx/releases/tag/v0.0.172

@drcmda would you mind updating your @vx/responsive dep to this version and seeing if the warning is still present?

@JacquiManzi
Copy link
Contributor

JacquiManzi commented Aug 17, 2018

Upgrading @vx/responsive fixed the warning message for me.

Sorry I may have spoken too soon, I appear to be seeing it again. I will investigate further and will make sure all of my node_modules are up-to-date.

Update: The upgrade does in fact work- we had another component that was causing the same error.

@hshoff
Copy link
Member

hshoff commented Aug 20, 2018

@JacquiManzi thanks for following up @JacquiManzi!

Going to close this issue as it appears resolved. @drcmda happy to re-open if you find it did not fix for your use case.

@hshoff hshoff closed this as completed Aug 20, 2018
@drcmda
Copy link
Author

drcmda commented Aug 24, 2018

@hshoff thanks! it looks like it's fixed!

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

No branches or pull requests

3 participants