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
Event binding in componentDidMount #34
Conversation
doing event binding in componentWillMount means that if this view is ever rendered server side, you'll have a leak because componentWillUnmount will never be called.
Thanks for the PR (and for trying out the plugin)! So I'm not totally familiar with the differences between It's quite possible that I'm missing something here, so please enlighten me if so. |
Also, I just realized you submitted a change to an example, not the actual plugin, but I guess my question still stands as to why |
I confess that I hadn't even checked to see if the plugin was doing the same thing. If we come to agreement, I'd be happy to make a pull request for that as well. the issue is that on the server the component is never mounted at all. this means both that |
just took a peek at the plugin src and realized that the plugin itself doesn't actually do event binding (its still early...brain not working...), so there's no leak, unless the consumer uses it incorrectly. |
Cool, thanks for the info. Going to close this PR then. Let me know if you have any other ideas! |
oh, i meant that the actual plugin isn't an issue. the example still would leak, it would just be the example's fault, not the plugin's. |
What exactly is being leaked here? The Firebase client will clean up after itself even if the user (or the plugin for that matter) doesn't remove the event listeners explicitly. So, we should be good there. Is there something else which is being leaked? Also, I still don't quite fully understand how the Firebase data would ever be downloaded on the server if we change this to |
The plugin can't clean up after itself server side because ps. lots of discussion for just an example. I'm happy to go on, but cut me off whenever you get tired :) |
doing event binding in componentWillMount means that if this view is ever rendered server side, you'll have a leak because componentWillUnmount will never be called. Not a big deal, but this demo was my first taste of using react, so it kind of formed my initial opinion of where event binding should occur.