Skip to content

Commit af5c342

Browse files
author
Brian Vaughn
committed
Dont recreate maked context unless unmasked context changes
Doing so can lead to infinite loops if componentWillReceiveProps() calls setState()
1 parent 7f0dc41 commit af5c342

File tree

5 files changed

+88
-9
lines changed

5 files changed

+88
-9
lines changed

scripts/fiber/tests-passing.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,6 +1165,7 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
11651165
* reads context when setState is above the provider
11661166
* maintains the correct context when providers bail out due to low priority
11671167
* maintains the correct context when unwinding due to an error in render
1168+
* should not recreate context unless inputs have changed
11681169

11691170
src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
11701171
* catches render error in a boundary during full deferred mounting

src/renderers/shared/fiber/ReactFiberBeginWork.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ var {
3131
var ReactTypeOfWork = require('ReactTypeOfWork');
3232
var {
3333
getMaskedContext,
34+
getUnmaskedContext,
3435
hasContextChanged,
3536
pushContextProvider,
3637
pushTopLevelContextObject,
@@ -210,7 +211,8 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
210211
return bailoutOnAlreadyFinishedWork(current, workInProgress);
211212
}
212213

213-
var context = getMaskedContext(workInProgress);
214+
var unmaskedContext = getUnmaskedContext(workInProgress);
215+
var context = getMaskedContext(workInProgress, unmaskedContext);
214216

215217
var nextChildren;
216218

@@ -427,7 +429,8 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
427429
}
428430
var fn = workInProgress.type;
429431
var props = workInProgress.pendingProps;
430-
var context = getMaskedContext(workInProgress);
432+
var unmaskedContext = getUnmaskedContext(workInProgress);
433+
var context = getMaskedContext(workInProgress, unmaskedContext);
431434

432435
var value;
433436

src/renderers/shared/fiber/ReactFiberClassComponent.js

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type { PriorityLevel } from 'ReactPriorityLevel';
1717

1818
var {
1919
getMaskedContext,
20+
getUnmaskedContext,
2021
} = require('ReactFiberContext');
2122
var {
2223
addUpdate,
@@ -204,10 +205,17 @@ module.exports = function(
204205
function constructClassInstance(workInProgress : Fiber) : any {
205206
const ctor = workInProgress.type;
206207
const props = workInProgress.pendingProps;
207-
const context = getMaskedContext(workInProgress);
208+
const unmaskedContext = getUnmaskedContext(workInProgress);
209+
const context = getMaskedContext(workInProgress, unmaskedContext);
208210
const instance = new ctor(props, context);
209211
adoptClassInstance(workInProgress, instance);
210212
checkClassInstance(workInProgress);
213+
214+
// Cache unmasked context so we can avoid recreating masked context unless necessary.
215+
// ReactFiberContext usually updates this cache but can't for newly-created instances.
216+
instance.__reactInternalMemoizedUnmaskedChildContext = unmaskedContext;
217+
instance.__reactInternalMemoizedMaskedChildContext = context;
218+
211219
return instance;
212220
}
213221

@@ -221,9 +229,11 @@ module.exports = function(
221229
throw new Error('There must be pending props for an initial mount.');
222230
}
223231

232+
const unmaskedContext = getUnmaskedContext(workInProgress);
233+
224234
instance.props = props;
225235
instance.state = state;
226-
instance.context = getMaskedContext(workInProgress);
236+
instance.context = getMaskedContext(workInProgress, unmaskedContext);
227237

228238
if (typeof instance.componentWillMount === 'function') {
229239
instance.componentWillMount();
@@ -256,7 +266,8 @@ module.exports = function(
256266
throw new Error('There should always be pending or memoized props.');
257267
}
258268
}
259-
const newContext = getMaskedContext(workInProgress);
269+
const newUnmaskedContext = getUnmaskedContext(workInProgress);
270+
const newContext = getMaskedContext(workInProgress, newUnmaskedContext);
260271

261272
// TODO: Should we deal with a setState that happened after the last
262273
// componentWillMount and before this componentWillMount? Probably
@@ -277,7 +288,7 @@ module.exports = function(
277288
const newInstance = constructClassInstance(workInProgress);
278289
newInstance.props = newProps;
279290
newInstance.state = newState = newInstance.state || null;
280-
newInstance.context = getMaskedContext(workInProgress);
291+
newInstance.context = newContext;
281292

282293
if (typeof newInstance.componentWillMount === 'function') {
283294
newInstance.componentWillMount();
@@ -314,7 +325,8 @@ module.exports = function(
314325
}
315326
}
316327
const oldContext = instance.context;
317-
const newContext = getMaskedContext(workInProgress);
328+
const newUnmaskedContext = getUnmaskedContext(workInProgress);
329+
const newContext = getMaskedContext(workInProgress, newUnmaskedContext);
318330

319331
// Note: During these life-cycles, instance.props/instance.state are what
320332
// ever the previously attempted to render - not the "current". However,

src/renderers/shared/fiber/ReactFiberContext.js

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,26 @@ function getUnmaskedContext(workInProgress : Fiber) : Object {
5555
}
5656
return contextStackCursor.current;
5757
}
58+
exports.getUnmaskedContext = getUnmaskedContext;
5859

59-
exports.getMaskedContext = function(workInProgress : Fiber) {
60+
exports.getMaskedContext = function(workInProgress : Fiber, unmaskedContext : Object) {
6061
const type = workInProgress.type;
6162
const contextTypes = type.contextTypes;
6263
if (!contextTypes) {
6364
return emptyObject;
6465
}
6566

66-
const unmaskedContext = getUnmaskedContext(workInProgress);
67+
// Avoid recreating masked context unless unmasked context has changed.
68+
// Failing to do this will result in unnecessary calls to componentWillReceiveProps.
69+
// This may trigger infinite loops if componentWillReceiveProps calls setState.
70+
const instance = workInProgress.stateNode;
71+
if (
72+
instance &&
73+
instance.__reactInternalMemoizedUnmaskedChildContext === unmaskedContext
74+
) {
75+
return instance.__reactInternalMemoizedMaskedChildContext;
76+
}
77+
6778
const context = {};
6879
for (let key in contextTypes) {
6980
context[key] = unmaskedContext[key];
@@ -74,6 +85,12 @@ exports.getMaskedContext = function(workInProgress : Fiber) {
7485
checkReactTypeSpec(contextTypes, context, 'context', name, null, workInProgress);
7586
}
7687

88+
// Cache unmasked context so we can avoid recreating masked context unless necessary.
89+
if (instance) {
90+
instance.__reactInternalMemoizedUnmaskedChildContext = unmaskedContext;
91+
instance.__reactInternalMemoizedMaskedChildContext = context;
92+
}
93+
7794
return context;
7895
};
7996

src/renderers/shared/fiber/__tests__/ReactIncremental-test.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2021,4 +2021,50 @@ describe('ReactIncremental', () => {
20212021
});
20222022
ReactNoop.flush();
20232023
});
2024+
2025+
it('should not recreate masked context unless inputs have changed', () => {
2026+
const ops = [];
2027+
2028+
let scuCounter = 0;
2029+
2030+
class MyComponent extends React.Component {
2031+
static contextTypes = {};
2032+
componentDidMount(prevProps, prevState) {
2033+
ops.push('componentDidMount');
2034+
this.setState({ setStateInCDU: true });
2035+
}
2036+
componentDidUpdate(prevProps, prevState) {
2037+
ops.push('componentDidUpdate');
2038+
if (this.state.setStateInCDU) {
2039+
this.setState({ setStateInCDU: false });
2040+
}
2041+
}
2042+
componentWillReceiveProps(nextProps) {
2043+
ops.push('componentWillReceiveProps');
2044+
this.setState({ setStateInCDU: true });
2045+
}
2046+
render() {
2047+
ops.push('render');
2048+
return null;
2049+
}
2050+
shouldComponentUpdate(nextProps, nextState) {
2051+
ops.push('shouldComponentUpdate');
2052+
return scuCounter++ < 5; // Don't let test hang
2053+
}
2054+
}
2055+
2056+
ReactNoop.render(<MyComponent />);
2057+
ReactNoop.flush();
2058+
2059+
expect(ops).toEqual([
2060+
'render',
2061+
'componentDidMount',
2062+
'shouldComponentUpdate',
2063+
'render',
2064+
'componentDidUpdate',
2065+
'shouldComponentUpdate',
2066+
'render',
2067+
'componentDidUpdate',
2068+
]);
2069+
});
20242070
});

0 commit comments

Comments
 (0)