Feature flag to revert #15650 (#15659)
PR #15650 is a bugfix but it's technically a semantic change that could cause regressions. I don't think it will be an issue, since the previous behavior was both broken and incoherent, but out of an abundance of caution, let's wrap it in a flag so we can easily revert it if necessary.
Andrew Clark committed
May 15, 2019 at 13:38 UTC
d34b457ce2a2cee1160dbc90b93312ff9850958c
12 files changed
+95
-6
packages/react-reconciler/src/ReactFiberClassComponent.js
+11
@@ -52,7 +52,9 @@ import {
52
requestCurrentTime,
53
computeExpirationForFiber,
54
scheduleWork,
55
+ flushPassiveEffects,
56
} from './ReactFiberScheduler';
57
+import {revertPassiveEffectsChange} from 'shared/ReactFeatureFlags';
58
59
const fakeInternalInstance = {};
60
const isArray = Array.isArray;
@@ -193,6 +195,9 @@ const classComponentUpdater = {
195
update.callback = callback;
196
}
197
198
+ if (revertPassiveEffectsChange) {
199
+ flushPassiveEffects();
200
+ }
201
enqueueUpdate(fiber, update);
202
scheduleWork(fiber, expirationTime);
203
},
@@ -212,6 +217,9 @@ const classComponentUpdater = {
217
update.callback = callback;
218
}
219
220
+ if (revertPassiveEffectsChange) {
221
+ flushPassiveEffects();
222
+ }
223
enqueueUpdate(fiber, update);
224
scheduleWork(fiber, expirationTime);
225
},
@@ -230,6 +238,9 @@ const classComponentUpdater = {
238
update.callback = callback;
239
}
240
241
+ if (revertPassiveEffectsChange) {
242
+ flushPassiveEffects();
243
+ }
244
enqueueUpdate(fiber, update);
245
scheduleWork(fiber, expirationTime);
246
},
packages/react-reconciler/src/ReactFiberHooks.js
+6
@@ -31,6 +31,7 @@ import {
31
import {
32
scheduleWork,
33
computeExpirationForFiber,
34
+ flushPassiveEffects,
35
requestCurrentTime,
36
warnIfNotCurrentlyActingUpdatesInDev,
37
markRenderEventTime,
@@ -41,6 +42,7 @@ import warning from 'shared/warning';
42
import getComponentName from 'shared/getComponentName';
43
import is from 'shared/objectIs';
44
import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
45
+import {revertPassiveEffectsChange} from 'shared/ReactFeatureFlags';
46
47
const {ReactCurrentDispatcher} = ReactSharedInternals;
48
@@ -1107,6 +1109,10 @@ function dispatchAction<S, A>(
1109
lastRenderPhaseUpdate.next = update;
1110
}
1111
} else {
1112
+ if (revertPassiveEffectsChange) {
1113
+ flushPassiveEffects();
1114
+ }
1115
+
1116
const currentTime = requestCurrentTime();
1117
const expirationTime = computeExpirationForFiber(currentTime, fiber);
1118
packages/react-reconciler/src/ReactFiberReconciler.js
+14
@@ -64,6 +64,7 @@ import {
64
} from './ReactCurrentFiber';
65
import {StrictMode} from './ReactTypeOfMode';
66
import {Sync} from './ReactFiberExpirationTime';
67
+import {revertPassiveEffectsChange} from 'shared/ReactFeatureFlags';
68
69
type OpaqueRoot = FiberRoot;
70
@@ -152,6 +153,9 @@ function scheduleRootUpdate(
153
update.callback = callback;
154
}
155
156
+ if (revertPassiveEffectsChange) {
157
+ flushPassiveEffects();
158
+ }
159
enqueueUpdate(current, update);
160
scheduleWork(current, expirationTime);
161
@@ -391,6 +395,10 @@ if (__DEV__) {
395
id--;
396
}
397
if (currentHook !== null) {
398
+ if (revertPassiveEffectsChange) {
399
+ flushPassiveEffects();
400
+ }
401
+
402
const newState = copyWithSet(currentHook.memoizedState, path, value);
403
currentHook.memoizedState = newState;
404
currentHook.baseState = newState;
@@ -408,6 +416,9 @@ if (__DEV__) {
416
417
// Support DevTools props for function components, forwardRef, memo, host components, etc.
418
overrideProps = (fiber: Fiber, path: Array<string | number>, value: any) => {
419
+ if (revertPassiveEffectsChange) {
420
+ flushPassiveEffects();
421
+ }
422
fiber.pendingProps = copyWithSet(fiber.memoizedProps, path, value);
423
if (fiber.alternate) {
424
fiber.alternate.pendingProps = fiber.pendingProps;
@@ -416,6 +427,9 @@ if (__DEV__) {
427
};
428
429
scheduleUpdate = (fiber: Fiber) => {
430
+ if (revertPassiveEffectsChange) {
431
+ flushPassiveEffects();
432
+ }
433
scheduleWork(fiber, Sync);
434
};
435
packages/react-reconciler/src/ReactFiberScheduler.js
+13
-6
@@ -24,6 +24,7 @@ import {
24
enableProfilerTimer,
25
disableYielding,
26
enableSchedulerTracing,
27
+ revertPassiveEffectsChange,
28
} from 'shared/ReactFeatureFlags';
29
import ReactSharedInternals from 'shared/ReactSharedInternals';
30
import invariant from 'shared/invariant';
@@ -560,9 +561,11 @@ export function flushInteractiveUpdates() {
561
return;
562
}
563
flushPendingDiscreteUpdates();
563
- // If the discrete updates scheduled passive effects, flush them now so that
564
- // they fire before the next serial event.
565
- flushPassiveEffects();
564
+ if (!revertPassiveEffectsChange) {
565
+ // If the discrete updates scheduled passive effects, flush them now so that
566
+ // they fire before the next serial event.
567
+ flushPassiveEffects();
568
+ }
569
}
570
571
function resolveLocksOnRoot(root: FiberRoot, expirationTime: ExpirationTime) {
@@ -598,8 +601,10 @@ export function interactiveUpdates<A, B, C, R>(
601
// should explicitly call flushInteractiveUpdates.
602
flushPendingDiscreteUpdates();
603
}
601
- // TODO: Remove this call for the same reason as above.
602
- flushPassiveEffects();
604
+ if (!revertPassiveEffectsChange) {
605
+ // TODO: Remove this call for the same reason as above.
606
+ flushPassiveEffects();
607
+ }
608
return runWithPriority(UserBlockingPriority, fn.bind(null, a, b, c));
609
}
610
@@ -750,7 +755,9 @@ function renderRoot(
755
return commitRoot.bind(null, root);
756
}
757
753
- flushPassiveEffects();
758
+ if (!revertPassiveEffectsChange) {
759
+ flushPassiveEffects();
760
+ }
761
762
// If the root or expiration time have changed, throw out the existing stack
763
// and prepare a fresh one. Otherwise we'll continue where we left off.
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js
+42
@@ -2077,4 +2077,46 @@ describe('ReactHooksWithNoopRenderer', () => {
2077
expect(Scheduler).toFlushAndYield(['Step: 5, Shadow: 5']);
2078
expect(ReactNoop).toMatchRenderedOutput('5');
2079
});
2080
+
2081
+ describe('revertPassiveEffectsChange', () => {
2082
+ it('flushes serial effects before enqueueing work', () => {
2083
+ jest.resetModules();
2084
+
2085
+ ReactFeatureFlags = require('shared/ReactFeatureFlags');
2086
+ ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
2087
+ ReactFeatureFlags.enableSchedulerTracing = true;
2088
+ ReactFeatureFlags.revertPassiveEffectsChange = true;
2089
+ React = require('react');
2090
+ ReactNoop = require('react-noop-renderer');
2091
+ Scheduler = require('scheduler');
2092
+ SchedulerTracing = require('scheduler/tracing');
2093
+ useState = React.useState;
2094
+ useEffect = React.useEffect;
2095
+ act = ReactNoop.act;
2096
+
2097
+ let _updateCount;
2098
+ function Counter(props) {
2099
+ const [count, updateCount] = useState(0);
2100
+ _updateCount = updateCount;
2101
+ useEffect(() => {
2102
+ Scheduler.yieldValue(`Will set count to 1`);
2103
+ updateCount(1);
2104
+ }, []);
2105
+ return <Text text={'Count: ' + count} />;
2106
+ }
2107
+
2108
+ ReactNoop.render(<Counter count={0} />, () =>
2109
+ Scheduler.yieldValue('Sync effect'),
2110
+ );
2111
+ expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']);
2112
+ expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
2113
+
2114
+ // Enqueuing this update forces the passive effect to be flushed --
2115
+ // updateCount(1) happens first, so 2 wins.
2116
+ act(() => _updateCount(2));
2117
+ expect(Scheduler).toHaveYielded(['Will set count to 1']);
2118
+ expect(Scheduler).toFlushAndYield(['Count: 2']);
2119
+ expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]);
2120
+ });
2121
+ });
2122
});
packages/shared/ReactFeatureFlags.js
+3
@@ -67,3 +67,6 @@ export const enableEventAPI = false;
67
68
// New API for JSX transforms to target - https://github.com/reactjs/rfcs/pull/107
69
export const enableJSXTransformAPI = false;
70
+
71
+// Temporary flag to revert the fix in #15650
72
+export const revertPassiveEffectsChange = false;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -32,6 +32,7 @@ export const warnAboutDeprecatedLifecycles = true;
32
export const warnAboutDeprecatedSetNativeProps = true;
33
export const enableEventAPI = false;
34
export const enableJSXTransformAPI = false;
35
+export const revertPassiveEffectsChange = false;
36
37
// Only used in www builds.
38
export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -29,6 +29,7 @@ export const enableSchedulerDebugging = false;
29
export const warnAboutDeprecatedSetNativeProps = false;
30
export const enableEventAPI = false;
31
export const enableJSXTransformAPI = false;
32
+export const revertPassiveEffectsChange = false;
33
34
// Only used in www builds.
35
export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.persistent.js
+1
@@ -29,6 +29,7 @@ export const enableSchedulerDebugging = false;
29
export const warnAboutDeprecatedSetNativeProps = false;
30
export const enableEventAPI = false;
31
export const enableJSXTransformAPI = false;
32
+export const revertPassiveEffectsChange = false;
33
34
// Only used in www builds.
35
export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -29,6 +29,7 @@ export const enableSchedulerDebugging = false;
29
export const warnAboutDeprecatedSetNativeProps = false;
30
export const enableEventAPI = false;
31
export const enableJSXTransformAPI = false;
32
+export const revertPassiveEffectsChange = false;
33
34
// Only used in www builds.
35
export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -27,6 +27,7 @@ export const disableJavaScriptURLs = false;
27
export const disableYielding = false;
28
export const enableEventAPI = true;
29
export const enableJSXTransformAPI = true;
30
+export const revertPassiveEffectsChange = false;
31
32
// Only used in www builds.
33
export function addUserTimingListener() {
packages/shared/forks/ReactFeatureFlags.www.js
+1
@@ -20,6 +20,7 @@ export const {
20
disableInputAttributeSyncing,
21
warnAboutShorthandPropertyCollision,
22
warnAboutDeprecatedSetNativeProps,
23
+ revertPassiveEffectsChange,
24
} = require('ReactFeatureFlags');
25
26
// In www, we have experimental support for gathering data