-
Notifications
You must be signed in to change notification settings - Fork 0
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
REACT - Do we need to detach event handlers in componentWillUnmount? #2
Comments
@mantagen what problem are you trying to solve...? |
@nelsonic I'm not trying to solve a problem, I was asking precautionarily so as to avoid a potential problem. From what I've read here, event listeners which you attach to elements not in your component (eg to the window) will remain after your component unmounts unless you make it happen in componentWillUnmount. So the answer to my (perhaps poorly phrased) question is yes, because react doesn't do that clean up for you. So far the clean-up operations I've found which might be handy in componentWillUnmount are: Clearing setTimeout What else is there? (maybe this should be a separate question) |
I'm unclear what event handlers you're trying to remove. Attaching event handlers to the window That said if you are mutating some state on a global (like, for example, a timeout or an interval (RAF, I think, is a little bit more complicated)), you probably will want to clean that up at some point. This can normally be managed within your top-level react component. I've not heard of a use case for dynamically mounting and unmounting your whole react app (which is what |
Agreed it wouldn't be that self contained (so not really in the spirit of react components per se) to attach an event handler to the window on render etc. But it might be that someone did want to do it for some niche reason. |
For instance, in a React component, if an event handler is attached to the window on render (is that bad practise in itself?/ always avoidable) , should it it detached in the componentWillUnmount method?
The text was updated successfully, but these errors were encountered: