Skip to content

Commit

Permalink
Reverting recent flushSync changes (facebook#21816)
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn authored and zhengjitf committed Apr 15, 2022
1 parent d09c6f1 commit faa5d39
Show file tree
Hide file tree
Showing 16 changed files with 338 additions and 132 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Object {
6 => 1,
},
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 16,
"updaters": Array [
Object {
Expand Down Expand Up @@ -87,7 +87,7 @@ Object {
4 => 2,
},
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 15,
"updaters": Array [
Object {
Expand Down Expand Up @@ -186,7 +186,7 @@ Object {
6 => 1,
},
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 12,
"updaters": Array [
Object {
Expand Down Expand Up @@ -445,7 +445,7 @@ Object {
],
],
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 12,
"updaters": Array [
Object {
Expand Down Expand Up @@ -938,7 +938,7 @@ Object {
],
],
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 11,
"updaters": Array [
Object {
Expand Down Expand Up @@ -1597,7 +1597,7 @@ Object {
17 => 1,
},
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 24,
"updaters": Array [
Object {
Expand Down Expand Up @@ -1687,7 +1687,7 @@ Object {
"fiberActualDurations": Map {},
"fiberSelfDurations": Map {},
"passiveEffectDuration": 0,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 34,
"updaters": Array [
Object {
Expand Down Expand Up @@ -2223,7 +2223,7 @@ Object {
],
],
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 24,
"updaters": Array [
Object {
Expand Down Expand Up @@ -2310,7 +2310,7 @@ Object {
"fiberActualDurations": Array [],
"fiberSelfDurations": Array [],
"passiveEffectDuration": 0,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 34,
"updaters": Array [
Object {
Expand Down Expand Up @@ -2431,7 +2431,7 @@ Object {
2 => 0,
},
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 0,
"updaters": Array [
Object {
Expand Down Expand Up @@ -2506,7 +2506,7 @@ Object {
3 => 0,
},
"passiveEffectDuration": 0,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 0,
"updaters": Array [
Object {
Expand Down Expand Up @@ -2715,7 +2715,7 @@ Object {
],
],
"passiveEffectDuration": 0,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 0,
"updaters": Array [
Object {
Expand Down Expand Up @@ -3071,7 +3071,7 @@ Object {
7 => 0,
},
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 0,
"updaters": Array [
Object {
Expand Down Expand Up @@ -3515,7 +3515,7 @@ Object {
],
],
"passiveEffectDuration": null,
"priorityLevel": "Immediate",
"priorityLevel": "Normal",
"timestamp": 0,
"updaters": Array [
Object {
Expand Down
8 changes: 7 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMFiber-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1154,13 +1154,19 @@ describe('ReactDOMFiber', () => {
expect(ops).toEqual(['A']);

if (__DEV__) {
expect(console.error.calls.count()).toBe(2);
const errorCalls = console.error.calls.count();
expect(console.error.calls.argsFor(0)[0]).toMatch(
'ReactDOM.render is no longer supported in React 18',
);
expect(console.error.calls.argsFor(1)[0]).toMatch(
'ReactDOM.render is no longer supported in React 18',
);
// TODO: this warning shouldn't be firing in the first place if user didn't call it.
for (let i = 2; i < errorCalls; i++) {
expect(console.error.calls.argsFor(i)[0]).toMatch(
'unstable_flushDiscreteUpdates: Cannot flush updates when React is already rendering.',
);
}
}
});

Expand Down
12 changes: 6 additions & 6 deletions packages/react-dom/src/__tests__/ReactMount-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ describe('ReactMount', () => {
expect(calls).toBe(5);
});

it('initial mount of legacy root is sync inside batchedUpdates, as if it were wrapped in flushSync', () => {
it('initial mount is sync inside batchedUpdates, but task work is deferred until the end of the batch', () => {
const container1 = document.createElement('div');
const container2 = document.createElement('div');

Expand All @@ -302,12 +302,12 @@ describe('ReactMount', () => {

// Initial mount on another root. Should flush immediately.
ReactDOM.render(<Foo>a</Foo>, container2);
// The earlier update also flushed, since flushSync flushes all pending
// sync work across all roots.
expect(container1.textContent).toEqual('2');
// Layout updates are also flushed synchronously
expect(container2.textContent).toEqual('a!');
// The update did not flush yet.
expect(container1.textContent).toEqual('1');
// The initial mount flushed, but not the update scheduled in cDM.
expect(container2.textContent).toEqual('a');
});
// All updates have flushed.
expect(container1.textContent).toEqual('2');
expect(container2.textContent).toEqual('a!');
});
Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import {createEventHandle} from './ReactDOMEventHandle';
import {
batchedUpdates,
discreteUpdates,
flushDiscreteUpdates,
flushSync,
flushSyncWithoutWarningIfAlreadyRendering,
flushControlled,
injectIntoDevTools,
attemptSynchronousHydration,
Expand Down Expand Up @@ -100,7 +100,7 @@ setRestoreImplementation(restoreControlledState);
setBatchingImplementation(
batchedUpdates,
discreteUpdates,
flushSyncWithoutWarningIfAlreadyRendering,
flushDiscreteUpdates,
);

function createPortal(
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/client/ReactDOMLegacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
createContainer,
findHostInstanceWithNoPortals,
updateContainer,
flushSyncWithoutWarningIfAlreadyRendering,
unbatchedUpdates,
getPublicRootInstance,
findHostInstance,
findHostInstanceWithWarning,
Expand Down Expand Up @@ -174,7 +174,7 @@ function legacyRenderSubtreeIntoContainer(
};
}
// Initial mount should not be batched.
flushSyncWithoutWarningIfAlreadyRendering(() => {
unbatchedUpdates(() => {
updateContainer(children, fiberRoot, parentComponent, callback);
});
} else {
Expand Down Expand Up @@ -357,7 +357,7 @@ export function unmountComponentAtNode(container: Container) {
}

// Unmount should not be batched.
flushSyncWithoutWarningIfAlreadyRendering(() => {
unbatchedUpdates(() => {
legacyRenderSubtreeIntoContainer(null, null, container, false, () => {
// $FlowFixMe This should probably use `delete container._reactRootContainer`
container._reactRootContainer = null;
Expand Down
8 changes: 4 additions & 4 deletions packages/react-dom/src/events/ReactDOMUpdateBatching.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ let batchedUpdatesImpl = function(fn, bookkeeping) {
let discreteUpdatesImpl = function(fn, a, b, c, d) {
return fn(a, b, c, d);
};
let flushSyncImpl = function() {};
let flushDiscreteUpdatesImpl = function() {};

let isInsideEventHandler = false;

Expand All @@ -39,7 +39,7 @@ function finishEventHandler() {
// bails out of the update without touching the DOM.
// TODO: Restore state in the microtask, after the discrete updates flush,
// instead of early flushing them here.
flushSyncImpl();
flushDiscreteUpdatesImpl();
restoreStateIfNeeded();
}
}
Expand Down Expand Up @@ -67,9 +67,9 @@ export function discreteUpdates(fn, a, b, c, d) {
export function setBatchingImplementation(
_batchedUpdatesImpl,
_discreteUpdatesImpl,
_flushSyncImpl,
_flushDiscreteUpdatesImpl,
) {
batchedUpdatesImpl = _batchedUpdatesImpl;
discreteUpdatesImpl = _discreteUpdatesImpl;
flushSyncImpl = _flushSyncImpl;
flushDiscreteUpdatesImpl = _flushDiscreteUpdatesImpl;
}
2 changes: 2 additions & 0 deletions packages/react-noop-renderer/src/ReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ export const {
flushExpired,
batchedUpdates,
deferredUpdates,
unbatchedUpdates,
discreteUpdates,
idleUpdates,
flushDiscreteUpdates,
flushSync,
flushPassiveEffects,
act,
Expand Down
1 change: 1 addition & 0 deletions packages/react-noop-renderer/src/ReactNoopPersistent.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const {
flushExpired,
batchedUpdates,
deferredUpdates,
unbatchedUpdates,
discreteUpdates,
idleUpdates,
flushDiscreteUpdates,
Expand Down
4 changes: 4 additions & 0 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

deferredUpdates: NoopRenderer.deferredUpdates,

unbatchedUpdates: NoopRenderer.unbatchedUpdates,

discreteUpdates: NoopRenderer.discreteUpdates,

idleUpdates<T>(fn: () => T): T {
Expand All @@ -913,6 +915,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
}
},

flushDiscreteUpdates: NoopRenderer.flushDiscreteUpdates,

flushSync(fn: () => mixed) {
NoopRenderer.flushSync(fn);
},
Expand Down
15 changes: 10 additions & 5 deletions packages/react-reconciler/src/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ import {
createContainer as createContainer_old,
updateContainer as updateContainer_old,
batchedUpdates as batchedUpdates_old,
unbatchedUpdates as unbatchedUpdates_old,
deferredUpdates as deferredUpdates_old,
discreteUpdates as discreteUpdates_old,
flushDiscreteUpdates as flushDiscreteUpdates_old,
flushControlled as flushControlled_old,
flushSync as flushSync_old,
flushSyncWithoutWarningIfAlreadyRendering as flushSyncWithoutWarningIfAlreadyRendering_old,
flushPassiveEffects as flushPassiveEffects_old,
getPublicRootInstance as getPublicRootInstance_old,
attemptSynchronousHydration as attemptSynchronousHydration_old,
Expand Down Expand Up @@ -55,11 +56,12 @@ import {
createContainer as createContainer_new,
updateContainer as updateContainer_new,
batchedUpdates as batchedUpdates_new,
unbatchedUpdates as unbatchedUpdates_new,
deferredUpdates as deferredUpdates_new,
discreteUpdates as discreteUpdates_new,
flushDiscreteUpdates as flushDiscreteUpdates_new,
flushControlled as flushControlled_new,
flushSync as flushSync_new,
flushSyncWithoutWarningIfAlreadyRendering as flushSyncWithoutWarningIfAlreadyRendering_new,
flushPassiveEffects as flushPassiveEffects_new,
getPublicRootInstance as getPublicRootInstance_new,
attemptSynchronousHydration as attemptSynchronousHydration_new,
Expand Down Expand Up @@ -97,19 +99,22 @@ export const updateContainer = enableNewReconciler
export const batchedUpdates = enableNewReconciler
? batchedUpdates_new
: batchedUpdates_old;
export const unbatchedUpdates = enableNewReconciler
? unbatchedUpdates_new
: unbatchedUpdates_old;
export const deferredUpdates = enableNewReconciler
? deferredUpdates_new
: deferredUpdates_old;
export const discreteUpdates = enableNewReconciler
? discreteUpdates_new
: discreteUpdates_old;
export const flushDiscreteUpdates = enableNewReconciler
? flushDiscreteUpdates_new
: flushDiscreteUpdates_old;
export const flushControlled = enableNewReconciler
? flushControlled_new
: flushControlled_old;
export const flushSync = enableNewReconciler ? flushSync_new : flushSync_old;
export const flushSyncWithoutWarningIfAlreadyRendering = enableNewReconciler
? flushSyncWithoutWarningIfAlreadyRendering_new
: flushSyncWithoutWarningIfAlreadyRendering_old;
export const flushPassiveEffects = enableNewReconciler
? flushPassiveEffects_new
: flushPassiveEffects_old;
Expand Down
6 changes: 4 additions & 2 deletions packages/react-reconciler/src/ReactFiberReconciler.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@ import {
scheduleUpdateOnFiber,
flushRoot,
batchedUpdates,
unbatchedUpdates,
flushSync,
flushControlled,
deferredUpdates,
discreteUpdates,
flushSyncWithoutWarningIfAlreadyRendering,
flushDiscreteUpdates,
flushPassiveEffects,
} from './ReactFiberWorkLoop.new';
import {
Expand Down Expand Up @@ -326,11 +327,12 @@ export function updateContainer(

export {
batchedUpdates,
unbatchedUpdates,
deferredUpdates,
discreteUpdates,
flushDiscreteUpdates,
flushControlled,
flushSync,
flushSyncWithoutWarningIfAlreadyRendering,
flushPassiveEffects,
};

Expand Down
6 changes: 4 additions & 2 deletions packages/react-reconciler/src/ReactFiberReconciler.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@ import {
scheduleUpdateOnFiber,
flushRoot,
batchedUpdates,
unbatchedUpdates,
flushSync,
flushControlled,
deferredUpdates,
discreteUpdates,
flushSyncWithoutWarningIfAlreadyRendering,
flushDiscreteUpdates,
flushPassiveEffects,
} from './ReactFiberWorkLoop.old';
import {
Expand Down Expand Up @@ -326,11 +327,12 @@ export function updateContainer(

export {
batchedUpdates,
unbatchedUpdates,
deferredUpdates,
discreteUpdates,
flushDiscreteUpdates,
flushControlled,
flushSync,
flushSyncWithoutWarningIfAlreadyRendering,
flushPassiveEffects,
};

Expand Down
Loading

0 comments on commit faa5d39

Please sign in to comment.