After server-side renderToString() component claims it isMounted()
Closed this issue · 14 comments
If you keep a _this
reference at the componentWillMount()
lifecycle method of a component that is then rendered with renderToString()
, then _this.isMounted() === true
later on. The documentation however claims that server-side components aren't really mounted; since componentDidMount()
is never called.
I think this behavior changed in React 0.14 but that might be wrong.
I'm a little curious how you ran into this, but yeah, looks like a bug to me.
I noticed this while working on react-transmit. Its universal components started to spit warnings when doing server-side rendering: those components should call setState() only on the client-side, but started doing that on the server too since upgrading to React 0.14. Traced that behavior and found that isMounted() now returns true on the server.
Note that componentDidMount() is still not called, so my current work-around is to keep track of the mounted state manually.
(Very unlikely to be a good first bug.)
I start fixing it but I don't understand something. If we allow to call componentWillMount
here then why _this.isMounted()
should return false? The cleanest way to mark component as unmounted is simply unmount it after renderToString
but it will call componentWillUnmount
.
We have below options:
- Custom mount, which will not mark component as mounted (but it should not call
componentWillMount
then?) - Simply unmount component after render (which will call
componentWillUnmount
) - the best way IMO - Another hack, which will not break previous tests
@Laskos componentWillMount
is called on the server, where componentDidMount
is not. Therefore the component is never really mounted on the server. That is the big difference between those two distinct lifecycle methods. @spicyj Referring to the suggested fixes: To change the behavior of invoked lifecycle method calls would mean breaking existing components. For example by calling componentDidMount
and/or componentWillUnmount
some components will attempt to run client-side code that they specifically put in those skipped-on-the-server lifecycle methods. IMHO (I don't have in-depth knowledge of React core) it's not worth it to fix a small regression like this by overhauling the component lifecycle, affecting examples, tests, documentation et cetera.
@RickWong is correct, we certainly don't want to call componentDidMount
or ComponentWillUnmount
on the server. The lifecycle is that the component will mount, but the component is not yet mounted (isMounted()
should be false) on the server side.
Just dropping by to say I am observing this is still the case under React 0.14.7
It looks like the current implementation just checks for the existence of __renderedComponent
which is actually assigned in performInitialMount
before mountComponent
is called. If isMounted
should only be true after componentDidMount
then it seems like it'd be better attach a flag in invokeComponentDidMountWithTimer
indicating it mounted?
it'd be better attach a flag in invokeComponentDidMountWithTimer indicating it mounted?
(Unrelated, but invokeComponentDidMountWithTimer
is not a good place to do anything—it is only called in DEV. It specifically exists so we don’t pay for unnecessary indirection and/or code size.)
Thanks @gaearon I didn't notice that. Looking at reconciler/ReactCompositeComponent.js#L337-L343
if (inst.componentDidMount) {
if (__DEV__) {
transaction.getReactMountReady().enqueue(invokeComponentDidMountWithTimer, this);
} else {
transaction.getReactMountReady().enqueue(inst.componentDidMount, inst);
}
}
componentDidMount
calls are enqueued in the transaction
in all cases. I would expect isMounted
to be set right after componentDidMount
is called, so perhaps it could enqueue a function that calls componentDidMount
and also sets a flag used by isMounted
// inline arrow function and flag name just used to illustrate the gist of it
transaction.getReactMountReady().enqueue(() => {
inst.componentDidMount;
inst._isMouted = true
}, inst);
Then isMounted
can check that flag instead of __renderedComponent
.
isMounted can have many meanings and there's no clear correct one so I'm inclined to leave the current behavior even though it's a little bit confusing. Non-createClass classes do not support isMounted anyway.