Skip to content
Permalink
Browse files
Replace unbatchedUpdates with flushSync (#21776)
There's a weird quirk leftover from the old Stack (pre-Fiber)
implementation where the initial mount of a leagcy (ReactDOM.render)
root is flushed synchronously even inside `batchedUpdates`.

The original workaround for this was an internal method called
`unbatchedUpdates`. We've since added another API that works almost the
same way, `flushSync`.

The only difference is that `unbatchedUpdates` would not cause other
pending updates to flush too, only the newly mounted root. `flushSync`
flushes all pending sync work across all roots. This was to preserve
the exact behavior of the Stack implementation.

But since it's close enough, let's just use `flushSync`. It's unlikely
anyone's app accidentally relies on this subtle difference, and the
legacy API is deprecated in 18, anyway.
  • Loading branch information
acdlite committed Jul 1, 2021
1 parent 32eefcb commit ed6c091fe961a3b95e956ebcefe8f152177b1fb7
Showing 12 changed files with 59 additions and 239 deletions.
@@ -40,7 +40,7 @@ Object {
6 => 1,
},
"passiveEffectDuration": null,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 16,
"updaters": Array [
Object {
@@ -87,7 +87,7 @@ Object {
4 => 2,
},
"passiveEffectDuration": null,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 15,
"updaters": Array [
Object {
@@ -186,7 +186,7 @@ Object {
6 => 1,
},
"passiveEffectDuration": null,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 12,
"updaters": Array [
Object {
@@ -445,7 +445,7 @@ Object {
],
],
"passiveEffectDuration": null,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 12,
"updaters": Array [
Object {
@@ -938,7 +938,7 @@ Object {
],
],
"passiveEffectDuration": null,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 11,
"updaters": Array [
Object {
@@ -1597,7 +1597,7 @@ Object {
17 => 1,
},
"passiveEffectDuration": null,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 24,
"updaters": Array [
Object {
@@ -1687,7 +1687,7 @@ Object {
"fiberActualDurations": Map {},
"fiberSelfDurations": Map {},
"passiveEffectDuration": 0,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 34,
"updaters": Array [
Object {
@@ -2223,7 +2223,7 @@ Object {
],
],
"passiveEffectDuration": null,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 24,
"updaters": Array [
Object {
@@ -2310,7 +2310,7 @@ Object {
"fiberActualDurations": Array [],
"fiberSelfDurations": Array [],
"passiveEffectDuration": 0,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 34,
"updaters": Array [
Object {
@@ -2431,7 +2431,7 @@ Object {
2 => 0,
},
"passiveEffectDuration": null,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 0,
"updaters": Array [
Object {
@@ -2506,7 +2506,7 @@ Object {
3 => 0,
},
"passiveEffectDuration": 0,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 0,
"updaters": Array [
Object {
@@ -2715,7 +2715,7 @@ Object {
],
],
"passiveEffectDuration": 0,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 0,
"updaters": Array [
Object {
@@ -3071,7 +3071,7 @@ Object {
7 => 0,
},
"passiveEffectDuration": null,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 0,
"updaters": Array [
Object {
@@ -3515,7 +3515,7 @@ Object {
],
],
"passiveEffectDuration": null,
"priorityLevel": "Normal",
"priorityLevel": "Immediate",
"timestamp": 0,
"updaters": Array [
Object {
@@ -277,7 +277,7 @@ describe('ReactMount', () => {
expect(calls).toBe(5);
});

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

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

// Initial mount on another root. Should flush immediately.
ReactDOM.render(<Foo>a</Foo>, container2);
// 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');
// 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!');
});
// All updates have flushed.
expect(container1.textContent).toEqual('2');
expect(container2.textContent).toEqual('a!');
});
@@ -29,7 +29,7 @@ import {
createContainer,
findHostInstanceWithNoPortals,
updateContainer,
unbatchedUpdates,
flushSyncWithoutWarningIfAlreadyRendering,
getPublicRootInstance,
findHostInstance,
findHostInstanceWithWarning,
@@ -174,7 +174,7 @@ function legacyRenderSubtreeIntoContainer(
};
}
// Initial mount should not be batched.
unbatchedUpdates(() => {
flushSyncWithoutWarningIfAlreadyRendering(() => {
updateContainer(children, fiberRoot, parentComponent, callback);
});
} else {
@@ -357,7 +357,7 @@ export function unmountComponentAtNode(container: Container) {
}

// Unmount should not be batched.
unbatchedUpdates(() => {
flushSyncWithoutWarningIfAlreadyRendering(() => {
legacyRenderSubtreeIntoContainer(null, null, container, false, () => {
// $FlowFixMe This should probably use `delete container._reactRootContainer`
container._reactRootContainer = null;
@@ -38,7 +38,6 @@ export const {
flushExpired,
batchedUpdates,
deferredUpdates,
unbatchedUpdates,
discreteUpdates,
idleUpdates,
flushSync,
@@ -38,7 +38,6 @@ export const {
flushExpired,
batchedUpdates,
deferredUpdates,
unbatchedUpdates,
discreteUpdates,
idleUpdates,
flushDiscreteUpdates,
@@ -901,8 +901,6 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

deferredUpdates: NoopRenderer.deferredUpdates,

unbatchedUpdates: NoopRenderer.unbatchedUpdates,

discreteUpdates: NoopRenderer.discreteUpdates,

idleUpdates<T>(fn: () => T): T {
@@ -18,7 +18,6 @@ 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,
flushControlled as flushControlled_old,
@@ -56,7 +55,6 @@ 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,
flushControlled as flushControlled_new,
@@ -99,9 +97,6 @@ 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;
@@ -52,7 +52,6 @@ import {
scheduleUpdateOnFiber,
flushRoot,
batchedUpdates,
unbatchedUpdates,
flushSync,
flushControlled,
deferredUpdates,
@@ -327,7 +326,6 @@ export function updateContainer(

export {
batchedUpdates,
unbatchedUpdates,
deferredUpdates,
discreteUpdates,
flushControlled,
@@ -52,7 +52,6 @@ import {
scheduleUpdateOnFiber,
flushRoot,
batchedUpdates,
unbatchedUpdates,
flushSync,
flushControlled,
deferredUpdates,
@@ -327,7 +326,6 @@ export function updateContainer(

export {
batchedUpdates,
unbatchedUpdates,
deferredUpdates,
discreteUpdates,
flushControlled,
@@ -246,12 +246,11 @@ const {

type ExecutionContext = number;

export const NoContext = /* */ 0b00000;
const BatchedContext = /* */ 0b00001;
const LegacyUnbatchedContext = /* */ 0b00010;
const RenderContext = /* */ 0b00100;
const CommitContext = /* */ 0b01000;
export const RetryAfterError = /* */ 0b10000;
export const NoContext = /* */ 0b0000;
const BatchedContext = /* */ 0b0001;
const RenderContext = /* */ 0b0010;
const CommitContext = /* */ 0b0100;
export const RetryAfterError = /* */ 0b1000;

type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5;
const RootIncomplete = 0;
@@ -515,35 +514,19 @@ export function scheduleUpdateOnFiber(
}
}

if (lane === SyncLane) {
if (
// Check if we're inside unbatchedUpdates
(executionContext & LegacyUnbatchedContext) !== NoContext &&
// Check if we're not already rendering
(executionContext & (RenderContext | CommitContext)) === NoContext
) {
// This is a legacy edge case. The initial mount of a ReactDOM.render-ed
// root inside of batchedUpdates should be synchronous, but layout updates
// should be deferred until the end of the batch.
performSyncWorkOnRoot(root);
} else {
ensureRootIsScheduled(root, eventTime);
if (
executionContext === NoContext &&
(fiber.mode & ConcurrentMode) === NoMode
) {
// Flush the synchronous work now, unless we're already working or inside
// a batch. This is intentionally inside scheduleUpdateOnFiber instead of
// scheduleCallbackForFiber to preserve the ability to schedule a callback
// without immediately flushing it. We only do this for user-initiated
// updates, to preserve historical behavior of legacy mode.
resetRenderTimer();
flushSyncCallbacksOnlyInLegacyMode();
}
}
} else {
// Schedule other updates after in case the callback is sync.
ensureRootIsScheduled(root, eventTime);
ensureRootIsScheduled(root, eventTime);
if (
lane === SyncLane &&
executionContext === NoContext &&
(fiber.mode & ConcurrentMode) === NoMode
) {
// Flush the synchronous work now, unless we're already working or inside
// a batch. This is intentionally inside scheduleUpdateOnFiber instead of
// scheduleCallbackForFiber to preserve the ability to schedule a callback
// without immediately flushing it. We only do this for user-initiated
// updates, to preserve historical behavior of legacy mode.
resetRenderTimer();
flushSyncCallbacksOnlyInLegacyMode();
}

return root;
@@ -1095,25 +1078,6 @@ export function discreteUpdates<A, B, C, D, R>(
}
}

export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
const prevExecutionContext = executionContext;
executionContext &= ~BatchedContext;
executionContext |= LegacyUnbatchedContext;
try {
return fn(a);
} finally {
executionContext = prevExecutionContext;
// If there were legacy sync updates, flush them at the end of the outer
// most batchedUpdates-like method.
if (executionContext === NoContext) {
resetRenderTimer();
// TODO: I think this call is redundant, because we flush inside
// scheduleUpdateOnFiber when LegacyUnbatchedContext is set.
flushSyncCallbacksOnlyInLegacyMode();
}
}
}

export function flushSyncWithoutWarningIfAlreadyRendering<A, R>(
fn: A => R,
a: A,
@@ -1954,24 +1918,6 @@ function commitRootImpl(root, renderPriorityLevel) {
throw error;
}

if ((executionContext & LegacyUnbatchedContext) !== NoContext) {
if (__DEV__) {
if (enableDebugTracing) {
logCommitStopped();
}
}

if (enableSchedulingProfiler) {
markCommitStopped();
}

// This is a legacy edge case. We just committed the initial mount of
// a ReactDOM.render-ed root inside of batchedUpdates. The commit fired
// synchronously, but layout updates should be deferred until the end
// of the batch.
return null;
}

// If the passive effects are the result of a discrete render, flush them
// synchronously at the end of the current task so that the result is
// immediately observable. Otherwise, we assume that they are not

0 comments on commit ed6c091

Please sign in to comment.