Skip to content

Commit 56b4459

Browse files
sophiebitsrhagigi
authored andcommitted
Bug fix: SSR setState in diff components don't mix (facebook#12323)
Previously, the `queue` and `replace` arguments were leaking across loops even though they should be captured.
1 parent 3519b00 commit 56b4459

File tree

2 files changed

+58
-32
lines changed

2 files changed

+58
-32
lines changed

packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,4 +226,38 @@ describe('ReactDOMServerLifecycles', () => {
226226
ReactDOMServer.renderToString(<Component />);
227227
expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']);
228228
});
229+
230+
it('tracks state updates across components', () => {
231+
class Outer extends React.Component {
232+
UNSAFE_componentWillMount() {
233+
this.setState({x: 1});
234+
}
235+
render() {
236+
return <Inner updateParent={this.updateParent}>{this.state.x}</Inner>;
237+
}
238+
updateParent = () => {
239+
this.setState({x: 3});
240+
};
241+
}
242+
class Inner extends React.Component {
243+
UNSAFE_componentWillMount() {
244+
this.setState({x: 2});
245+
this.props.updateParent();
246+
}
247+
render() {
248+
return <div>{this.props.children + '-' + this.state.x}</div>;
249+
}
250+
}
251+
expect(() => {
252+
// Shouldn't be 1-3.
253+
expect(ReactDOMServer.renderToStaticMarkup(<Outer />)).toBe(
254+
'<div>1-2</div>',
255+
);
256+
}).toWarnDev(
257+
'Warning: setState(...): Can only update a mounting component. This ' +
258+
'usually means you called setState() outside componentWillMount() on ' +
259+
'the server. This is a no-op.\n\n' +
260+
'Please check the code for the Outer component.',
261+
);
262+
});
229263
});

packages/react-dom/src/server/ReactPartialRenderer.js

Lines changed: 24 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -381,36 +381,26 @@ function resolve(
381381
child: mixed,
382382
context: Object,
383383
|} {
384-
let element: ReactElement;
385-
386-
let Component;
387-
let publicContext;
388-
let inst, queue, replace;
389-
let updater;
390-
391-
let initialState;
392-
let oldQueue, oldReplace;
393-
let nextState, dontMutate;
394-
let partial, partialState;
395-
396-
let childContext;
397-
let childContextTypes, contextKey;
398-
399384
while (React.isValidElement(child)) {
400385
// Safe because we just checked it's an element.
401-
element = ((child: any): ReactElement);
386+
let element: ReactElement = (child: any);
387+
let Component = element.type;
402388
if (__DEV__) {
403389
pushElementToDebugStack(element);
404390
}
405-
Component = element.type;
406391
if (typeof Component !== 'function') {
407392
break;
408393
}
409-
publicContext = processContext(Component, context);
394+
processChild(element, Component);
395+
}
396+
397+
// Extra closure so queue and replace can be captured properly
398+
function processChild(element, Component) {
399+
let publicContext = processContext(Component, context);
410400

411-
queue = [];
412-
replace = false;
413-
updater = {
401+
let queue = [];
402+
let replace = false;
403+
let updater = {
414404
isMounted: function(publicInstance) {
415405
return false;
416406
},
@@ -433,6 +423,7 @@ function resolve(
433423
},
434424
};
435425

426+
let inst;
436427
if (shouldConstruct(Component)) {
437428
inst = new Component(element.props, publicContext, updater);
438429

@@ -453,7 +444,7 @@ function resolve(
453444
}
454445
}
455446

456-
partialState = Component.getDerivedStateFromProps.call(
447+
let partialState = Component.getDerivedStateFromProps.call(
457448
null,
458449
element.props,
459450
inst.state,
@@ -502,15 +493,15 @@ function resolve(
502493
if (inst == null || inst.render == null) {
503494
child = inst;
504495
validateRenderResult(child, Component);
505-
continue;
496+
return;
506497
}
507498
}
508499

509500
inst.props = element.props;
510501
inst.context = publicContext;
511502
inst.updater = updater;
512503

513-
initialState = inst.state;
504+
let initialState = inst.state;
514505
if (initialState === undefined) {
515506
inst.state = initialState = null;
516507
}
@@ -558,19 +549,19 @@ function resolve(
558549
inst.UNSAFE_componentWillMount();
559550
}
560551
if (queue.length) {
561-
oldQueue = queue;
562-
oldReplace = replace;
552+
let oldQueue = queue;
553+
let oldReplace = replace;
563554
queue = null;
564555
replace = false;
565556

566557
if (oldReplace && oldQueue.length === 1) {
567558
inst.state = oldQueue[0];
568559
} else {
569-
nextState = oldReplace ? oldQueue[0] : inst.state;
570-
dontMutate = true;
560+
let nextState = oldReplace ? oldQueue[0] : inst.state;
561+
let dontMutate = true;
571562
for (let i = oldReplace ? 1 : 0; i < oldQueue.length; i++) {
572-
partial = oldQueue[i];
573-
partialState =
563+
let partial = oldQueue[i];
564+
let partialState =
574565
typeof partial === 'function'
575566
? partial.call(inst, nextState, element.props, publicContext)
576567
: partial;
@@ -600,11 +591,12 @@ function resolve(
600591
}
601592
validateRenderResult(child, Component);
602593

594+
let childContext;
603595
if (typeof inst.getChildContext === 'function') {
604-
childContextTypes = Component.childContextTypes;
596+
let childContextTypes = Component.childContextTypes;
605597
if (typeof childContextTypes === 'object') {
606598
childContext = inst.getChildContext();
607-
for (contextKey in childContext) {
599+
for (let contextKey in childContext) {
608600
invariant(
609601
contextKey in childContextTypes,
610602
'%s.getChildContext(): key "%s" is not defined in childContextTypes.',

0 commit comments

Comments
 (0)