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

setState in componentWillMount causes TypeError when rendering with renderToString and preceded by a component that renders with renderToString #6423

Closed
PKJedi opened this issue Apr 6, 2016 · 8 comments

Comments

@PKJedi
Copy link

PKJedi commented Apr 6, 2016

Calling setState in componentWillMount of a component causes the error

TypeError: Cannot read property '_currentElement' of null
    at ReactCompositeComponentMixin._updateRenderedComponent

when rendering server-side with ReactDOMServer.renderToString and preceded by a component that renders content using ReactDOMServer.renderToString or ReactDOMServer.renderToStaticMarkup.

Causes TypeError with 0.14.8 and 15.0.0-rc.2:
https://github.com/PKJedi/react-dom-server-test/blob/master/test.js

Works with 0.13:
https://github.com/PKJedi/react-dom-server-test/blob/master/0.13/test.js

@jimfb jimfb added the Type: Bug label Apr 6, 2016
@gaearon
Copy link
Collaborator

gaearon commented Apr 8, 2016

Do we even support this? If we don't we should have a good early error message.

@Merri
Copy link

Merri commented Apr 11, 2016

Yes, it is a supported feature according to the docs:

componentWillMount
Invoked once, both on the client and server, immediately before the initial rendering occurs. If you call setState within this method, render() will see the updated state and will be executed only once despite the state change.

Based on that the feature ought to work regardless of rendering method.

@aweary
Copy link
Contributor

aweary commented Jun 13, 2016

I've been looking at this issue this morning and it seems that it stems, at least partially, from the fact that ReactDefaultBatchingStrategy is being called when the first child component rendered has a call to renderToStaticMarkup or renderToString.

If you switch the order and render it second, then ReactServerBatchingStrategy (which is a noop) is used and the error isn't thrown.

If you remove the final

ReactUpdates.injection.injectBatchingStrategy(ReactDefaultBatchingStrategy);

from renderToStringImpl then the error isn't thrown, but that of course will have other implications.

@aweary
Copy link
Contributor

aweary commented Jun 13, 2016

So it looks like by the time enqueueUpdate is called, the batchingStrategy is already reset to ReactDefaultBatchingStrategy, even though the transaction that wrapped the ReactReconciler.mountComponent call was pulled from the ReactServerRenderingTransaction object pool.

Since ReactServerRenderingTransaction.batchedUpdates is a noop, and ReactDefaultBatchingStrategy.batchedUpdates is not, it's incorrectly attempting to apply the batched updates when it shouldn't.

@jimfb
Copy link
Contributor

jimfb commented Jun 13, 2016

cc @spicyj

@aweary
Copy link
Contributor

aweary commented Jun 13, 2016

I still have a rather naive understanding of how ReactUpdates works, but would it be reasonable to just enqueue any injectBatchingStrategy calls when there is still a transaction that hasn't been released, instead of applying it immediately? That way any actions in transaction.perform would finish will the batching strategy they expected?

@sophiebits
Copy link
Collaborator

IIRC we don't support nested server rendering at the moment and have another issue about it somewhere.

@aweary
Copy link
Contributor

aweary commented Jun 13, 2016

@spicyj I opened a PR #7033 with a fix that resolves this issue, let me know what you think.

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

6 participants