No nested updates
We may decide to provide this ability as an escape hatch in the future.
Andrew Clark committed
Dec 30, 2016 at 11:02 UTC
b79531eefee46a3cefeefa0ad79414546ebdafd3
6 files changed
+30
-120
scripts/fiber/tests-passing.txt
-1
@@ -1207,7 +1207,6 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js
1207
* can opt-in to deferred/animation scheduling inside componentDidMount/Update
1208
* performs Task work even after time runs out
1209
* does not perform animation work after time runs out
1210
-* can force synchronous updates with syncUpdates, even inside batchedUpdates
1210
1211
src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
1212
* can update child nodes of a host instance
src/renderers/shared/__tests__/ReactPerf-test.js
-15
@@ -16,7 +16,6 @@ describe('ReactPerf', () => {
16
var ReactDOM;
17
var ReactPerf;
18
var ReactTestUtils;
19
- var ReactDOMFeatureFlags;
19
var emptyFunction;
20
21
var App;
@@ -39,7 +38,6 @@ describe('ReactPerf', () => {
38
ReactDOM = require('ReactDOM');
39
ReactPerf = require('ReactPerf');
40
ReactTestUtils = require('ReactTestUtils');
42
- ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
41
emptyFunction = require('emptyFunction');
42
43
App = class extends React.Component {
@@ -647,10 +645,6 @@ describe('ReactPerf', () => {
645
var container = document.createElement('div');
646
var thrownErr = new Error('Muhaha!');
647
650
- if (ReactDOMFeatureFlags.useFiber) {
651
- spyOn(console, 'error');
652
- }
653
-
648
class Evil extends React.Component {
649
componentWillMount() {
650
throw thrownErr;
@@ -685,15 +679,6 @@ describe('ReactPerf', () => {
679
}
680
ReactDOM.unmountComponentAtNode(container);
681
ReactPerf.stop();
688
-
689
- if (ReactDOMFeatureFlags.useFiber) {
690
- // A sync `render` inside cWM will print a warning. That should be the
691
- // only warning.
692
- expect(console.error.calls.count()).toEqual(1);
693
- expect(console.error.calls.argsFor(0)[0]).toMatch(
694
- /Render methods should be a pure function of props and state/
695
- );
696
- }
682
});
683
684
it('should not print errant warnings if portal throws in componentDidMount()', () => {
src/renderers/shared/fiber/ReactFiberScheduler.js
+21
-61
@@ -69,7 +69,6 @@ var {
69
if (__DEV__) {
70
var ReactFiberInstrumentation = require('ReactFiberInstrumentation');
71
var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber');
72
- var warning = require('warning');
72
}
73
74
var timeHeuristicForUnitOfWork = 1;
@@ -111,8 +110,8 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
110
// Keeps track of whether we're currently in a work loop.
111
let isPerformingWork : boolean = false;
112
114
- // Keeps track of whether sync updates should be downgraded to task updates.
115
- let shouldDeferSyncUpdates : boolean = false;
113
+ // Keeps track of whether we should should batch sync updates.
114
+ let isBatchingUpdates : boolean = false;
115
116
// The next work in progress fiber that we're currently working on.
117
let nextUnitOfWork : ?Fiber = null;
@@ -675,31 +674,10 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
674
}
675
676
function performWork(priorityLevel : PriorityLevel, deadline : Deadline | null) {
678
- // If performWork is called recursively, we need to save the previous state
679
- // of the scheduler so it can be restored before the function exits.
680
- // Recursion is only possible when using syncUpdates.
681
- const previousPriorityContext = priorityContext;
682
- const previousPriorityContextBeforeReconciliation = priorityContextBeforeReconciliation;
683
- const previousIsPerformingWork = isPerformingWork;
684
- const previousShouldDeferSyncUpdates = shouldDeferSyncUpdates;
685
- const previousNextEffect = nextEffect;
686
- const previousCommitPhaseBoundaries = commitPhaseBoundaries;
687
- const previousFirstUncaughtError = firstUncaughtError;
688
- const previousFatalError = fatalError;
689
- const previousIsCommitting = isCommitting;
690
- const previousIsUnmounting = isUnmounting;
691
-
692
- priorityContext = NoWork;
693
- priorityContextBeforeReconciliation = NoWork;
677
+ if (isPerformingWork) {
678
+ throw new Error('performWork was called recursively.');
679
+ }
680
isPerformingWork = true;
695
- shouldDeferSyncUpdates = true;
696
- nextEffect = null;
697
- commitPhaseBoundaries = null;
698
- firstUncaughtError = null;
699
- fatalError = null;
700
- isCommitting = false;
701
- isUnmounting = false;
702
-
681
const isPerformingDeferredWork = Boolean(deadline);
682
let deadlineHasExpired = false;
683
@@ -805,17 +783,12 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
783
784
const errorToThrow = fatalError || firstUncaughtError;
785
808
- // We're done performing work. Restore the previous state of the scheduler.
809
- priorityContext = previousPriorityContext;
810
- priorityContextBeforeReconciliation = previousPriorityContextBeforeReconciliation;
811
- isPerformingWork = previousIsPerformingWork;
812
- shouldDeferSyncUpdates = previousShouldDeferSyncUpdates;
813
- nextEffect = previousNextEffect;
814
- commitPhaseBoundaries = previousCommitPhaseBoundaries;
815
- firstUncaughtError = previousFirstUncaughtError;
816
- fatalError = previousFatalError;
817
- isCommitting = previousIsCommitting;
818
- isUnmounting = previousIsUnmounting;
786
+ // We're done performing work. Time to clean up.
787
+ isPerformingWork = false;
788
+ fatalError = null;
789
+ firstUncaughtError = null;
790
+ capturedErrors = null;
791
+ failedBoundaries = null;
792
793
// It's safe to throw any unhandled errors.
794
if (errorToThrow) {
@@ -1030,20 +1003,6 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
1003
}
1004
1005
function scheduleUpdate(fiber : Fiber, priorityLevel : PriorityLevel) {
1033
- // Detect if a synchronous update is made during render (or begin phase).
1034
- if (priorityLevel === SynchronousPriority && isPerformingWork && !isCommitting) {
1035
- if (__DEV__) {
1036
- warning(
1037
- false,
1038
- 'Render methods should be a pure function of props and state; ' +
1039
- 'triggering nested component updates from render is not allowed. ' +
1040
- 'If necessary, trigger nested updates in componentDidUpdate.'
1041
- );
1042
- }
1043
- // Downgrade to Task priority to prevent an infinite loop.
1044
- priorityLevel = TaskPriority;
1045
- }
1046
-
1006
let node = fiber;
1007
let shouldContinue = true;
1008
while (node && shouldContinue) {
@@ -1098,8 +1057,9 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
1057
}
1058
1059
function getPriorityContext() : PriorityLevel {
1101
- // If we're in a batch, downgrade sync priority to task priority
1102
- if (priorityContext === SynchronousPriority && shouldDeferSyncUpdates) {
1060
+ // If we're in a batch, or if we're already performing work, downgrade sync
1061
+ // priority to task priority
1062
+ if (priorityContext === SynchronousPriority && (isPerformingWork || isBatchingUpdates)) {
1063
return TaskPriority;
1064
}
1065
return priorityContext;
@@ -1120,15 +1080,15 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
1080
}
1081
1082
function batchedUpdates<A, R>(fn : (a: A) => R, a : A) : R {
1123
- const previousShouldDeferSyncUpdates = shouldDeferSyncUpdates;
1124
- shouldDeferSyncUpdates = true;
1083
+ const previousIsBatchingUpdates = isBatchingUpdates;
1084
+ isBatchingUpdates = true;
1085
try {
1086
return fn(a);
1087
} finally {
1128
- shouldDeferSyncUpdates = previousShouldDeferSyncUpdates;
1088
+ isBatchingUpdates = previousIsBatchingUpdates;
1089
// If we're not already inside a batch, we need to flush any task work
1090
// that was created by the user-provided function.
1131
- if (!shouldDeferSyncUpdates) {
1091
+ if (!isPerformingWork && !isBatchingUpdates) {
1092
performWork(TaskPriority);
1093
}
1094
}
@@ -1136,14 +1096,14 @@ module.exports = function<T, P, I, TI, C, CX, CI>(config : HostConfig<T, P, I, T
1096
1097
function syncUpdates<A>(fn : () => A) : A {
1098
const previousPriorityContext = priorityContext;
1139
- const previousShouldDeferSyncUpdates = shouldDeferSyncUpdates;
1099
+ const previousIsBatchingUpdates = isBatchingUpdates;
1100
priorityContext = SynchronousPriority;
1141
- shouldDeferSyncUpdates = false;
1101
+ isBatchingUpdates = false;
1102
try {
1103
return fn();
1104
} finally {
1105
priorityContext = previousPriorityContext;
1146
- shouldDeferSyncUpdates = previousShouldDeferSyncUpdates;
1106
+ isBatchingUpdates = previousIsBatchingUpdates;
1107
}
1108
}
1109
src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js
-10
@@ -340,14 +340,4 @@ describe('ReactIncrementalScheduling', () => {
340
// animation priority.
341
expect(ReactNoop.getChildren()).toEqual([span(1)]);
342
});
343
-
344
- it('can force synchronous updates with syncUpdates, even inside batchedUpdates', done => {
345
- ReactNoop.batchedUpdates(() => {
346
- ReactNoop.syncUpdates(() => {
347
- ReactNoop.render(<span />);
348
- expect(ReactNoop.getChildren()).toEqual([span()]);
349
- done();
350
- });
351
- });
352
- });
343
});
src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js
-13
@@ -18,7 +18,6 @@ describe('ReactComponentTreeHook', () => {
18
var ReactInstanceMap;
19
var ReactComponentTreeHook;
20
var ReactComponentTreeTestUtils;
21
- var ReactDOMFeatureFlags;
21
22
beforeEach(() => {
23
jest.resetModules();
@@ -29,7 +28,6 @@ describe('ReactComponentTreeHook', () => {
28
ReactInstanceMap = require('ReactInstanceMap');
29
ReactComponentTreeHook = require('ReactComponentTreeHook');
30
ReactComponentTreeTestUtils = require('ReactComponentTreeTestUtils');
32
- ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
31
});
32
33
function assertTreeMatches(pairs) {
@@ -1843,9 +1841,6 @@ describe('ReactComponentTreeHook', () => {
1841
// https://github.com/facebook/react/issues/7187
1842
var el = document.createElement('div');
1843
var portalEl = document.createElement('div');
1846
- if (ReactDOMFeatureFlags.useFiber) {
1847
- spyOn(console, 'error');
1848
- }
1844
class Foo extends React.Component {
1845
componentWillMount() {
1846
ReactDOM.render(<div />, portalEl);
@@ -1855,14 +1850,6 @@ describe('ReactComponentTreeHook', () => {
1850
}
1851
}
1852
ReactDOM.render(<Foo />, el);
1858
- if (ReactDOMFeatureFlags.useFiber) {
1859
- // A sync `render` inside cWM will print a warning. That should be the
1860
- // only warning.
1861
- expect(console.error.calls.count()).toEqual(1);
1862
- expect(console.error.calls.argsFor(0)[0]).toMatch(
1863
- /Render methods should be a pure function of props and state/
1864
- );
1865
- }
1853
});
1854
1855
it('is created when calling renderToString during render', () => {
src/renderers/shared/shared/__tests__/ReactUpdates-test.js
+9
-20
@@ -1143,25 +1143,14 @@ describe('ReactUpdates', () => {
1143
expect(mounts).toBe(1);
1144
});
1145
1146
- it('mounts and unmounts are sync even in a batch', () => {
1147
- var container1 = document.createElement('div');
1148
- var container2 = document.createElement('div');
1149
-
1150
- let called = false;
1151
- class Foo extends React.Component {
1152
- componentDidMount() {
1153
- called = true;
1154
- ReactDOM.render(<div>Hello</div>, container2);
1155
- expect(container2.textContent).toEqual('Hello');
1156
- ReactDOM.unmountComponentAtNode(container2);
1157
- expect(container2.textContent).toEqual('');
1158
- }
1159
- render() {
1160
- return <div>{this.props.step}</div>;
1161
- }
1162
- }
1163
-
1164
- ReactDOM.render(<Foo />, container1);
1165
- expect(called).toEqual(true);
1146
+ it('mounts and unmounts are sync even in a batch', done => {
1147
+ var container = document.createElement('div');
1148
+ ReactDOM.unstable_batchedUpdates(() => {
1149
+ ReactDOM.render(<div>Hello</div>, container);
1150
+ expect(container.textContent).toEqual('Hello');
1151
+ ReactDOM.unmountComponentAtNode(container);
1152
+ expect(container.textContent).toEqual('');
1153
+ done();
1154
+ });
1155
});
1156
});