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

componentWillUnmount not called server side #3714

Closed
kryptt opened this issue Apr 21, 2015 · 4 comments
Closed

componentWillUnmount not called server side #3714

kryptt opened this issue Apr 21, 2015 · 4 comments

Comments

@kryptt
Copy link

kryptt commented Apr 21, 2015

When using renderToString or renderToStaticMarkup
componentWillMount is called but the corresponding
componentWillUnmount is never called. This leads to memory leaks on the server when using a mixin, or code that does cleanup whenever a component unmounts.

@bloodyowl
Copy link
Contributor

the component doesn't actually mount on the server. you can use componentDidMount instead of componentWillMount to hook things, that way mixins acting on lifecycle are not called on the server at all.

@kryptt
Copy link
Author

kryptt commented Apr 21, 2015

That doesn't work if I need to initialize things on both the client AND the server.
For example react-bacon initializes event streams that need to work both on the client and on the server, so, cleanup for that also needs to happen on the server.

@jimfb
Copy link
Contributor

jimfb commented Apr 21, 2015

The decision to not call componentWillUnmount on the server side is intentional. We can't call componentWillUnmount on the server side because that indicates that it's time for a component to cleanup and shutdown (calling it would mean that components get cleanedup before they ever get sent to the client, which is obviously wrong).

At the moment, some components will attempt to "detect" their runtime environment through the presence (or lack thereof) of particular features, thereby differentiating their server/browser environments. This can work, but feature detection is fragile and prone to breaking.

Eventually, we may provide a way for users to build isomorphic components against a sever-side or client-side environment, allowing for components to be more aware of their local environment and the features available in that environment (this is still in the early discussion/idea phase). There are other issues tracking the ideas in this area, including #3398 #2928 and #1979.

Ideally your components get their renderable data directly from their parent (instead of side-loading the data), which eliminates the vast majority of cases where you would otherwise be inclined to open resources on the server side. Following the simple-data-from-parent model also makes your code much easier to reason about.

Since the current solutions are (1) runtime detection or (2) reorganize app to get renderable data from the parent, and since there are other issues tracking this request, I'm going to close out this issue. Feel free to continue the conversation here.

@jimfb jimfb closed this as completed Apr 21, 2015
@kryptt
Copy link
Author

kryptt commented Apr 22, 2015

@JSFB thanks for the reply.

there is a problem with considering a singular component as existing on the sever and then on the client:
"We can't call componentWillUnmount on the server side because that indicates that it's time for a component to cleanup and shutdown (calling it would mean that components get cleanedup before they ever get sent to the client, which is obviously wrong)." -> For a component to actually move between server and client you would need to provide a clean serialization mechanism which would allow server-only resources to be cleaned up (which is something that would satisfactorily solve my issue). Otherwise attempting to abstract a component as both server & client this way is a leaky abstraction (and the cause of my writing this issue).

Regarding (2) : that implies the root node orchestrating the entire state of the application and is not ideal for non-trivial systems, its hard to make the case for such coupling.

#3398 seems to offer too little expressive power / flexibility for the additional step in the component life-cycle. For example it doesn't address clean-up, or provide a clean abstraction over the details between callback based, promise based, or stream based asynchronous programming. I'll see what comments I can bring up in that thread.

#2928 sounds very promising, and could make many things simpler for those of us using React under RFP dataflow / controlflow models, but might be something coming further down the line, or possibly forked from React itself which might not be interested in going that route.

#1979 Doesn't address this problem directly, and runs counter to key notions of symmetry and isomorphism.

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

No branches or pull requests

3 participants