From 3521cac6382e8aec1eb2b80c605f82ee1f33f5f3 Mon Sep 17 00:00:00 2001 From: Sophie Alpert Date: Sat, 3 Mar 2018 10:27:57 -0800 Subject: [PATCH] Bug fix: SSR setState in diff components don't mix (#12323) Previously, the `queue` and `replace` arguments were leaking across loops even though they should be captured. --- .../ReactDOMServerLifecycles-test.js | 34 +++++++++++ .../src/server/ReactPartialRenderer.js | 56 ++++++++----------- 2 files changed, 58 insertions(+), 32 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js b/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js index 862bc27e53dc7..53a384b5cda9d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js @@ -226,4 +226,38 @@ describe('ReactDOMServerLifecycles', () => { ReactDOMServer.renderToString(); expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']); }); + + it('tracks state updates across components', () => { + class Outer extends React.Component { + UNSAFE_componentWillMount() { + this.setState({x: 1}); + } + render() { + return {this.state.x}; + } + updateParent = () => { + this.setState({x: 3}); + }; + } + class Inner extends React.Component { + UNSAFE_componentWillMount() { + this.setState({x: 2}); + this.props.updateParent(); + } + render() { + return
{this.props.children + '-' + this.state.x}
; + } + } + expect(() => { + // Shouldn't be 1-3. + expect(ReactDOMServer.renderToStaticMarkup()).toBe( + '
1-2
', + ); + }).toWarnDev( + 'Warning: setState(...): Can only update a mounting component. This ' + + 'usually means you called setState() outside componentWillMount() on ' + + 'the server. This is a no-op.\n\n' + + 'Please check the code for the Outer component.', + ); + }); }); diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js index 01dcbf079ebcf..0fa13932651ba 100644 --- a/packages/react-dom/src/server/ReactPartialRenderer.js +++ b/packages/react-dom/src/server/ReactPartialRenderer.js @@ -381,36 +381,26 @@ function resolve( child: mixed, context: Object, |} { - let element: ReactElement; - - let Component; - let publicContext; - let inst, queue, replace; - let updater; - - let initialState; - let oldQueue, oldReplace; - let nextState, dontMutate; - let partial, partialState; - - let childContext; - let childContextTypes, contextKey; - while (React.isValidElement(child)) { // Safe because we just checked it's an element. - element = ((child: any): ReactElement); + let element: ReactElement = (child: any); + let Component = element.type; if (__DEV__) { pushElementToDebugStack(element); } - Component = element.type; if (typeof Component !== 'function') { break; } - publicContext = processContext(Component, context); + processChild(element, Component); + } + + // Extra closure so queue and replace can be captured properly + function processChild(element, Component) { + let publicContext = processContext(Component, context); - queue = []; - replace = false; - updater = { + let queue = []; + let replace = false; + let updater = { isMounted: function(publicInstance) { return false; }, @@ -433,6 +423,7 @@ function resolve( }, }; + let inst; if (shouldConstruct(Component)) { inst = new Component(element.props, publicContext, updater); @@ -453,7 +444,7 @@ function resolve( } } - partialState = Component.getDerivedStateFromProps.call( + let partialState = Component.getDerivedStateFromProps.call( null, element.props, inst.state, @@ -502,7 +493,7 @@ function resolve( if (inst == null || inst.render == null) { child = inst; validateRenderResult(child, Component); - continue; + return; } } @@ -510,7 +501,7 @@ function resolve( inst.context = publicContext; inst.updater = updater; - initialState = inst.state; + let initialState = inst.state; if (initialState === undefined) { inst.state = initialState = null; } @@ -558,19 +549,19 @@ function resolve( inst.UNSAFE_componentWillMount(); } if (queue.length) { - oldQueue = queue; - oldReplace = replace; + let oldQueue = queue; + let oldReplace = replace; queue = null; replace = false; if (oldReplace && oldQueue.length === 1) { inst.state = oldQueue[0]; } else { - nextState = oldReplace ? oldQueue[0] : inst.state; - dontMutate = true; + let nextState = oldReplace ? oldQueue[0] : inst.state; + let dontMutate = true; for (let i = oldReplace ? 1 : 0; i < oldQueue.length; i++) { - partial = oldQueue[i]; - partialState = + let partial = oldQueue[i]; + let partialState = typeof partial === 'function' ? partial.call(inst, nextState, element.props, publicContext) : partial; @@ -600,11 +591,12 @@ function resolve( } validateRenderResult(child, Component); + let childContext; if (typeof inst.getChildContext === 'function') { - childContextTypes = Component.childContextTypes; + let childContextTypes = Component.childContextTypes; if (typeof childContextTypes === 'object') { childContext = inst.getChildContext(); - for (contextKey in childContext) { + for (let contextKey in childContext) { invariant( contextKey in childContextTypes, '%s.getChildContext(): key "%s" is not defined in childContextTypes.',