Dont recreate maked context unless unmasked context changes
Doing so can lead to infinite loops if componentWillReceiveProps() calls setState()
Brian Vaughn committed
Jan 7, 2017 at 08:53 UTC
a68205975795e6fd631acbf44623f970951f5224
5 files changed
+88
-9
scripts/fiber/tests-passing.txt
+1
@@ -1165,6 +1165,7 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
1165
* reads context when setState is above the provider
1166
* maintains the correct context when providers bail out due to low priority
1167
* maintains the correct context when unwinding due to an error in render
1168
+* should not recreate masked context unless inputs have changed
1169
1170
src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
1171
* catches render error in a boundary during full deferred mounting
src/renderers/shared/fiber/ReactFiberBeginWork.js
+5
-2
@@ -31,6 +31,7 @@ var {
31
var ReactTypeOfWork = require('ReactTypeOfWork');
32
var {
33
getMaskedContext,
34
+ getUnmaskedContext,
35
hasContextChanged,
36
pushContextProvider,
37
pushTopLevelContextObject,
@@ -210,7 +211,8 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
211
return bailoutOnAlreadyFinishedWork(current, workInProgress);
212
}
213
213
- var context = getMaskedContext(workInProgress);
214
+ var unmaskedContext = getUnmaskedContext(workInProgress);
215
+ var context = getMaskedContext(workInProgress, unmaskedContext);
216
217
var nextChildren;
218
@@ -427,7 +429,8 @@ module.exports = function<T, P, I, TI, C, CX, CI>(
429
}
430
var fn = workInProgress.type;
431
var props = workInProgress.pendingProps;
430
- var context = getMaskedContext(workInProgress);
432
+ var unmaskedContext = getUnmaskedContext(workInProgress);
433
+ var context = getMaskedContext(workInProgress, unmaskedContext);
434
435
var value;
436
src/renderers/shared/fiber/ReactFiberClassComponent.js
+17
-5
@@ -17,6 +17,7 @@ import type { PriorityLevel } from 'ReactPriorityLevel';
17
18
var {
19
getMaskedContext,
20
+ getUnmaskedContext,
21
} = require('ReactFiberContext');
22
var {
23
addUpdate,
@@ -204,10 +205,17 @@ module.exports = function(
205
function constructClassInstance(workInProgress : Fiber) : any {
206
const ctor = workInProgress.type;
207
const props = workInProgress.pendingProps;
207
- const context = getMaskedContext(workInProgress);
208
+ const unmaskedContext = getUnmaskedContext(workInProgress);
209
+ const context = getMaskedContext(workInProgress, unmaskedContext);
210
const instance = new ctor(props, context);
211
adoptClassInstance(workInProgress, instance);
212
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
+
219
return instance;
220
}
221
@@ -221,9 +229,11 @@ module.exports = function(
229
throw new Error('There must be pending props for an initial mount.');
230
}
231
232
+ const unmaskedContext = getUnmaskedContext(workInProgress);
233
+
234
instance.props = props;
235
instance.state = state;
226
- instance.context = getMaskedContext(workInProgress);
236
+ instance.context = getMaskedContext(workInProgress, unmaskedContext);
237
238
if (typeof instance.componentWillMount === 'function') {
239
instance.componentWillMount();
@@ -256,7 +266,8 @@ module.exports = function(
266
throw new Error('There should always be pending or memoized props.');
267
}
268
}
259
- const newContext = getMaskedContext(workInProgress);
269
+ const newUnmaskedContext = getUnmaskedContext(workInProgress);
270
+ const newContext = getMaskedContext(workInProgress, newUnmaskedContext);
271
272
// TODO: Should we deal with a setState that happened after the last
273
// componentWillMount and before this componentWillMount? Probably
@@ -277,7 +288,7 @@ module.exports = function(
288
const newInstance = constructClassInstance(workInProgress);
289
newInstance.props = newProps;
290
newInstance.state = newState = newInstance.state || null;
280
- newInstance.context = getMaskedContext(workInProgress);
291
+ newInstance.context = newContext;
292
293
if (typeof newInstance.componentWillMount === 'function') {
294
newInstance.componentWillMount();
@@ -314,7 +325,8 @@ module.exports = function(
325
}
326
}
327
const oldContext = instance.context;
317
- const newContext = getMaskedContext(workInProgress);
328
+ const newUnmaskedContext = getUnmaskedContext(workInProgress);
329
+ const newContext = getMaskedContext(workInProgress, newUnmaskedContext);
330
331
// Note: During these life-cycles, instance.props/instance.state are what
332
// ever the previously attempted to render - not the "current". However,
src/renderers/shared/fiber/ReactFiberContext.js
+19
-2
@@ -55,15 +55,26 @@ function getUnmaskedContext(workInProgress : Fiber) : Object {
55
}
56
return contextStackCursor.current;
57
}
58
+exports.getUnmaskedContext = getUnmaskedContext;
59
59
-exports.getMaskedContext = function(workInProgress : Fiber) {
60
+exports.getMaskedContext = function(workInProgress : Fiber, unmaskedContext : Object) {
61
const type = workInProgress.type;
62
const contextTypes = type.contextTypes;
63
if (!contextTypes) {
64
return emptyObject;
65
}
66
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
+
78
const context = {};
79
for (let key in contextTypes) {
80
context[key] = unmaskedContext[key];
@@ -74,6 +85,12 @@ exports.getMaskedContext = function(workInProgress : Fiber) {
85
checkReactTypeSpec(contextTypes, context, 'context', name, null, workInProgress);
86
}
87
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
+
94
return context;
95
};
96
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+46
@@ -2021,4 +2021,50 @@ describe('ReactIncremental', () => {
2021
});
2022
ReactNoop.flush();
2023
});
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
+ });
2070
});