Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature flag to revert #15650 #15659

Merged
merged 1 commit into from May 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Expand Up @@ -52,7 +52,9 @@ import {
requestCurrentTime,
computeExpirationForFiber,
scheduleWork,
flushPassiveEffects,
} from './ReactFiberScheduler';
import {revertPassiveEffectsChange} from 'shared/ReactFeatureFlags';

const fakeInternalInstance = {};
const isArray = Array.isArray;
Expand Down Expand Up @@ -193,6 +195,9 @@ const classComponentUpdater = {
update.callback = callback;
}

if (revertPassiveEffectsChange) {
acdlite marked this conversation as resolved.
Show resolved Hide resolved
flushPassiveEffects();
}
enqueueUpdate(fiber, update);
scheduleWork(fiber, expirationTime);
},
Expand All @@ -212,6 +217,9 @@ const classComponentUpdater = {
update.callback = callback;
}

if (revertPassiveEffectsChange) {
flushPassiveEffects();
}
enqueueUpdate(fiber, update);
scheduleWork(fiber, expirationTime);
},
Expand All @@ -230,6 +238,9 @@ const classComponentUpdater = {
update.callback = callback;
}

if (revertPassiveEffectsChange) {
flushPassiveEffects();
}
enqueueUpdate(fiber, update);
scheduleWork(fiber, expirationTime);
},
Expand Down
6 changes: 6 additions & 0 deletions packages/react-reconciler/src/ReactFiberHooks.js
Expand Up @@ -31,6 +31,7 @@ import {
import {
scheduleWork,
computeExpirationForFiber,
flushPassiveEffects,
requestCurrentTime,
warnIfNotCurrentlyActingUpdatesInDev,
markRenderEventTime,
Expand All @@ -41,6 +42,7 @@ import warning from 'shared/warning';
import getComponentName from 'shared/getComponentName';
import is from 'shared/objectIs';
import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
import {revertPassiveEffectsChange} from 'shared/ReactFeatureFlags';

const {ReactCurrentDispatcher} = ReactSharedInternals;

Expand Down Expand Up @@ -1107,6 +1109,10 @@ function dispatchAction<S, A>(
lastRenderPhaseUpdate.next = update;
}
} else {
if (revertPassiveEffectsChange) {
flushPassiveEffects();
}

const currentTime = requestCurrentTime();
const expirationTime = computeExpirationForFiber(currentTime, fiber);

Expand Down
14 changes: 14 additions & 0 deletions packages/react-reconciler/src/ReactFiberReconciler.js
Expand Up @@ -64,6 +64,7 @@ import {
} from './ReactCurrentFiber';
import {StrictMode} from './ReactTypeOfMode';
import {Sync} from './ReactFiberExpirationTime';
import {revertPassiveEffectsChange} from 'shared/ReactFeatureFlags';

type OpaqueRoot = FiberRoot;

Expand Down Expand Up @@ -152,6 +153,9 @@ function scheduleRootUpdate(
update.callback = callback;
}

if (revertPassiveEffectsChange) {
flushPassiveEffects();
}
enqueueUpdate(current, update);
scheduleWork(current, expirationTime);

Expand Down Expand Up @@ -391,6 +395,10 @@ if (__DEV__) {
id--;
}
if (currentHook !== null) {
if (revertPassiveEffectsChange) {
flushPassiveEffects();
}

const newState = copyWithSet(currentHook.memoizedState, path, value);
currentHook.memoizedState = newState;
currentHook.baseState = newState;
Expand All @@ -408,6 +416,9 @@ if (__DEV__) {

// Support DevTools props for function components, forwardRef, memo, host components, etc.
overrideProps = (fiber: Fiber, path: Array<string | number>, value: any) => {
if (revertPassiveEffectsChange) {
flushPassiveEffects();
}
fiber.pendingProps = copyWithSet(fiber.memoizedProps, path, value);
if (fiber.alternate) {
fiber.alternate.pendingProps = fiber.pendingProps;
Expand All @@ -416,6 +427,9 @@ if (__DEV__) {
};

scheduleUpdate = (fiber: Fiber) => {
if (revertPassiveEffectsChange) {
flushPassiveEffects();
}
scheduleWork(fiber, Sync);
};

Expand Down
19 changes: 13 additions & 6 deletions packages/react-reconciler/src/ReactFiberScheduler.js
Expand Up @@ -24,6 +24,7 @@ import {
enableProfilerTimer,
disableYielding,
enableSchedulerTracing,
revertPassiveEffectsChange,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -560,9 +561,11 @@ export function flushInteractiveUpdates() {
return;
}
flushPendingDiscreteUpdates();
// If the discrete updates scheduled passive effects, flush them now so that
// they fire before the next serial event.
flushPassiveEffects();
if (!revertPassiveEffectsChange) {
// If the discrete updates scheduled passive effects, flush them now so that
// they fire before the next serial event.
flushPassiveEffects();
}
}

function resolveLocksOnRoot(root: FiberRoot, expirationTime: ExpirationTime) {
Expand Down Expand Up @@ -598,8 +601,10 @@ export function interactiveUpdates<A, B, C, R>(
// should explicitly call flushInteractiveUpdates.
flushPendingDiscreteUpdates();
}
// TODO: Remove this call for the same reason as above.
flushPassiveEffects();
if (!revertPassiveEffectsChange) {
// TODO: Remove this call for the same reason as above.
flushPassiveEffects();
}
return runWithPriority(UserBlockingPriority, fn.bind(null, a, b, c));
}

Expand Down Expand Up @@ -750,7 +755,9 @@ function renderRoot(
return commitRoot.bind(null, root);
}

flushPassiveEffects();
if (!revertPassiveEffectsChange) {
flushPassiveEffects();
}

// If the root or expiration time have changed, throw out the existing stack
// and prepare a fresh one. Otherwise we'll continue where we left off.
Expand Down
Expand Up @@ -2077,4 +2077,46 @@ describe('ReactHooksWithNoopRenderer', () => {
expect(Scheduler).toFlushAndYield(['Step: 5, Shadow: 5']);
expect(ReactNoop).toMatchRenderedOutput('5');
});

describe('revertPassiveEffectsChange', () => {
it('flushes serial effects before enqueueing work', () => {
jest.resetModules();

ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
ReactFeatureFlags.enableSchedulerTracing = true;
ReactFeatureFlags.revertPassiveEffectsChange = true;
React = require('react');
ReactNoop = require('react-noop-renderer');
Scheduler = require('scheduler');
SchedulerTracing = require('scheduler/tracing');
useState = React.useState;
useEffect = React.useEffect;
act = ReactNoop.act;

let _updateCount;
function Counter(props) {
const [count, updateCount] = useState(0);
_updateCount = updateCount;
useEffect(() => {
Scheduler.yieldValue(`Will set count to 1`);
updateCount(1);
}, []);
return <Text text={'Count: ' + count} />;
}

ReactNoop.render(<Counter count={0} />, () =>
Scheduler.yieldValue('Sync effect'),
);
expect(Scheduler).toFlushAndYieldThrough(['Count: 0', 'Sync effect']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);

// Enqueuing this update forces the passive effect to be flushed --
// updateCount(1) happens first, so 2 wins.
act(() => _updateCount(2));
expect(Scheduler).toHaveYielded(['Will set count to 1']);
expect(Scheduler).toFlushAndYield(['Count: 2']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 2')]);
});
});
});
3 changes: 3 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Expand Up @@ -67,3 +67,6 @@ export const enableEventAPI = false;

// New API for JSX transforms to target - https://github.com/reactjs/rfcs/pull/107
export const enableJSXTransformAPI = false;

// Temporary flag to revert the fix in #15650
export const revertPassiveEffectsChange = false;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Expand Up @@ -32,6 +32,7 @@ export const warnAboutDeprecatedLifecycles = true;
export const warnAboutDeprecatedSetNativeProps = true;
export const enableEventAPI = false;
export const enableJSXTransformAPI = false;
export const revertPassiveEffectsChange = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Expand Up @@ -29,6 +29,7 @@ export const enableSchedulerDebugging = false;
export const warnAboutDeprecatedSetNativeProps = false;
export const enableEventAPI = false;
export const enableJSXTransformAPI = false;
export const revertPassiveEffectsChange = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.persistent.js
Expand Up @@ -29,6 +29,7 @@ export const enableSchedulerDebugging = false;
export const warnAboutDeprecatedSetNativeProps = false;
export const enableEventAPI = false;
export const enableJSXTransformAPI = false;
export const revertPassiveEffectsChange = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Expand Up @@ -29,6 +29,7 @@ export const enableSchedulerDebugging = false;
export const warnAboutDeprecatedSetNativeProps = false;
export const enableEventAPI = false;
export const enableJSXTransformAPI = false;
export const revertPassiveEffectsChange = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
Expand Up @@ -27,6 +27,7 @@ export const disableJavaScriptURLs = false;
export const disableYielding = false;
export const enableEventAPI = true;
export const enableJSXTransformAPI = true;
export const revertPassiveEffectsChange = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Expand Up @@ -20,6 +20,7 @@ export const {
disableInputAttributeSyncing,
warnAboutShorthandPropertyCollision,
warnAboutDeprecatedSetNativeProps,
revertPassiveEffectsChange,
} = require('ReactFeatureFlags');

// In www, we have experimental support for gathering data
Expand Down