Skip to content

Commit

Permalink
Bug fix: SSR setState in diff components don't mix (#12323)
Browse files Browse the repository at this point in the history
Previously, the `queue` and `replace` arguments were leaking across loops even though they should be captured.
  • Loading branch information
sophiebits authored Mar 3, 2018
1 parent 373a33f commit 1d220ce
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 32 deletions.
34 changes: 34 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,38 @@ describe('ReactDOMServerLifecycles', () => {
ReactDOMServer.renderToString(<Component />);
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 <Inner updateParent={this.updateParent}>{this.state.x}</Inner>;
}
updateParent = () => {
this.setState({x: 3});
};
}
class Inner extends React.Component {
UNSAFE_componentWillMount() {
this.setState({x: 2});
this.props.updateParent();
}
render() {
return <div>{this.props.children + '-' + this.state.x}</div>;
}
}
expect(() => {
// Shouldn't be 1-3.
expect(ReactDOMServer.renderToStaticMarkup(<Outer />)).toBe(
'<div>1-2</div>',
);
}).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.',
);
});
});
56 changes: 24 additions & 32 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
Expand All @@ -433,6 +423,7 @@ function resolve(
},
};

let inst;
if (shouldConstruct(Component)) {
inst = new Component(element.props, publicContext, updater);

Expand All @@ -453,7 +444,7 @@ function resolve(
}
}

partialState = Component.getDerivedStateFromProps.call(
let partialState = Component.getDerivedStateFromProps.call(
null,
element.props,
inst.state,
Expand Down Expand Up @@ -502,15 +493,15 @@ function resolve(
if (inst == null || inst.render == null) {
child = inst;
validateRenderResult(child, Component);
continue;
return;
}
}

inst.props = element.props;
inst.context = publicContext;
inst.updater = updater;

initialState = inst.state;
let initialState = inst.state;
if (initialState === undefined) {
inst.state = initialState = null;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.',
Expand Down

0 comments on commit 1d220ce

Please sign in to comment.