Schedule sync updates in microtask (#20872)

* Schedule sync updates in microtask

* Updates from review

* Fix comment
This commit is contained in:
Ricky
2021-02-25 17:25:25 -05:00
committed by GitHub
parent 90bde6505e
commit c581cdd480
23 changed files with 121 additions and 51 deletions
@@ -380,6 +380,7 @@ describe('ReactDOMServerPartialHydration', () => {
resolve();
await promise;
Scheduler.unstable_flushAll();
await null;
jest.runAllTimers();
// We should now have hydrated with a ref on the existing span.
@@ -488,6 +488,7 @@ describe('ReactDOMServerHydration', () => {
jest.runAllTimers();
await Promise.resolve();
Scheduler.unstable_flushAll();
await null;
expect(element.textContent).toBe('Hello world');
});
@@ -16,6 +16,7 @@ import {__interactionsRef} from 'scheduler/tracing';
import {
enableSchedulerTracing,
decoupleUpdatePriorityFromScheduler,
enableSyncMicroTasks,
} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';
import {
@@ -23,6 +24,7 @@ import {
getCurrentUpdateLanePriority,
setCurrentUpdateLanePriority,
} from './ReactFiberLane.new';
import {scheduleMicrotask, supportsMicrotasks} from './ReactFiberHostConfig';
const {
unstable_runWithPriority: Scheduler_runWithPriority,
@@ -144,13 +146,19 @@ export function scheduleSyncCallback(callback: SchedulerCallback) {
// the next tick, or earlier if something calls `flushSyncCallbackQueue`.
if (syncQueue === null) {
syncQueue = [callback];
// Flush the queue in the next tick, at the earliest.
// TODO: Figure out how to remove this It's only here as a last resort if we
// forget to explicitly flush.
immediateQueueCallbackNode = Scheduler_scheduleCallback(
Scheduler_ImmediatePriority,
flushSyncCallbackQueueImpl,
);
if (enableSyncMicroTasks && supportsMicrotasks) {
// Flush the queue in a microtask.
scheduleMicrotask(flushSyncCallbackQueueImpl);
} else {
// Flush the queue in the next tick.
immediateQueueCallbackNode = Scheduler_scheduleCallback(
Scheduler_ImmediatePriority,
flushSyncCallbackQueueImpl,
);
}
} else {
// Push onto existing queue. Don't need to schedule a callback because
// we already scheduled one when we created the queue.
@@ -16,6 +16,7 @@ import {__interactionsRef} from 'scheduler/tracing';
import {
enableSchedulerTracing,
decoupleUpdatePriorityFromScheduler,
enableSyncMicroTasks,
} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';
import {
@@ -23,6 +24,7 @@ import {
getCurrentUpdateLanePriority,
setCurrentUpdateLanePriority,
} from './ReactFiberLane.old';
import {scheduleMicrotask, supportsMicrotasks} from './ReactFiberHostConfig';
const {
unstable_runWithPriority: Scheduler_runWithPriority,
@@ -144,13 +146,19 @@ export function scheduleSyncCallback(callback: SchedulerCallback) {
// the next tick, or earlier if something calls `flushSyncCallbackQueue`.
if (syncQueue === null) {
syncQueue = [callback];
// Flush the queue in the next tick, at the earliest.
// TODO: Figure out how to remove this It's only here as a last resort if we
// forget to explicitly flush.
immediateQueueCallbackNode = Scheduler_scheduleCallback(
Scheduler_ImmediatePriority,
flushSyncCallbackQueueImpl,
);
if (enableSyncMicroTasks && supportsMicrotasks) {
// Flush the queue in a microtask.
scheduleMicrotask(flushSyncCallbackQueueImpl);
} else {
// Flush the queue in the next tick.
immediateQueueCallbackNode = Scheduler_scheduleCallback(
Scheduler_ImmediatePriority,
flushSyncCallbackQueueImpl,
);
}
} else {
// Push onto existing queue. Don't need to schedule a callback because
// we already scheduled one when we created the queue.
@@ -598,12 +598,13 @@ describe('ReactExpiration', () => {
// second one.
Scheduler.unstable_advanceTime(1000);
// Attempt to interrupt with a high pri update.
updateHighPri();
await ReactNoop.act(async () => {
updateHighPri();
});
// The first update expired, so first will finish it without
// interrupting. But not the second update, which hasn't expired yet.
expect(Scheduler).toFlushExpired(['Sibling']);
expect(Scheduler).toFlushAndYield([
expect(Scheduler).toHaveYielded([
// The first update expired
'Sibling',
// Then render the high pri update
'High pri: 1',
'Normal pri: 1',
@@ -1792,7 +1792,7 @@ describe('ReactHooksWithNoopRenderer', () => {
it(
'in legacy mode, useEffect is deferred and updates finish synchronously ' +
'(in a single batch)',
() => {
async () => {
function Counter(props) {
const [count, updateCount] = useState('(empty)');
useEffect(() => {
@@ -1807,10 +1807,12 @@ describe('ReactHooksWithNoopRenderer', () => {
}, [props.count]);
return <Text text={'Count: ' + count} />;
}
act(() => {
await act(async () => {
ReactNoop.renderLegacySyncRoot(<Counter count={0} />);
// Even in legacy mode, effects are deferred until after paint
expect(Scheduler).toFlushAndYieldThrough(['Count: (empty)']);
ReactNoop.flushSync();
expect(Scheduler).toHaveYielded(['Count: (empty)']);
expect(ReactNoop.getChildren()).toEqual([span('Count: (empty)')]);
});
@@ -1400,6 +1400,10 @@ describe('ReactIncrementalErrorHandling', () => {
'BrokenRenderAndUnmount componentWillUnmount',
]);
expect(ReactNoop.getChildren()).toEqual([]);
expect(() => {
ReactNoop.flushSync();
}).toThrow('One does not simply unmount me.');
});
it('does not interrupt unmounting if detaching a ref throws', () => {
@@ -98,8 +98,11 @@ describe('ReactOffscreen', () => {
<Text text="Outside" />
</>,
);
ReactNoop.flushSync();
// Should not defer the hidden tree
expect(Scheduler).toFlushUntilNextPaint(['A', 'Outside']);
expect(Scheduler).toHaveYielded(['A', 'Outside']);
});
expect(root).toMatchRenderedOutput(
<>
@@ -569,9 +569,11 @@ describe(
ReactNoop.render(<App />);
});
ReactNoop.flushSync();
// Because the render expired, React should finish the tree without
// consulting `shouldYield` again
expect(Scheduler).toFlushExpired(['B', 'C']);
expect(Scheduler).toHaveYielded(['B', 'C']);
});
});
},
@@ -292,9 +292,11 @@ describe('ReactSuspenseList', () => {
</>,
);
await C.resolve();
await ReactNoop.act(async () => {
C.resolve();
});
expect(Scheduler).toFlushAndYield(['C']);
expect(Scheduler).toHaveYielded(['C']);
expect(ReactNoop).toMatchRenderedOutput(
<>
@@ -304,9 +306,11 @@ describe('ReactSuspenseList', () => {
</>,
);
await B.resolve();
await ReactNoop.act(async () => {
B.resolve();
});
expect(Scheduler).toFlushAndYield(['B']);
expect(Scheduler).toHaveYielded(['B']);
expect(ReactNoop).toMatchRenderedOutput(
<>
@@ -310,7 +310,7 @@ describe('ReactSuspensePlaceholder', () => {
});
describe('when suspending during mount', () => {
it('properly accounts for base durations when a suspended times out in a legacy tree', () => {
it('properly accounts for base durations when a suspended times out in a legacy tree', async () => {
ReactNoop.renderLegacySyncRoot(<App shouldSuspend={true} />);
expect(Scheduler).toHaveYielded([
'App',
@@ -331,7 +331,10 @@ describe('ReactSuspensePlaceholder', () => {
jest.advanceTimersByTime(1000);
expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']);
expect(Scheduler).toFlushExpired(['Loaded']);
ReactNoop.flushSync();
expect(Scheduler).toHaveYielded(['Loaded']);
expect(ReactNoop).toMatchRenderedOutput('LoadedText');
expect(onRender).toHaveBeenCalledTimes(2);
@@ -378,7 +381,7 @@ describe('ReactSuspensePlaceholder', () => {
});
describe('when suspending during update', () => {
it('properly accounts for base durations when a suspended times out in a legacy tree', () => {
it('properly accounts for base durations when a suspended times out in a legacy tree', async () => {
ReactNoop.renderLegacySyncRoot(
<App shouldSuspend={false} textRenderDuration={5} />,
);
@@ -427,7 +430,10 @@ describe('ReactSuspensePlaceholder', () => {
jest.advanceTimersByTime(1000);
expect(Scheduler).toHaveYielded(['Promise resolved [Loaded]']);
expect(Scheduler).toFlushExpired(['Loaded']);
ReactNoop.flushSync();
expect(Scheduler).toHaveYielded(['Loaded']);
expect(ReactNoop).toMatchRenderedOutput('LoadedNew');
expect(onRender).toHaveBeenCalledTimes(4);
@@ -1088,9 +1088,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
expect(Scheduler).toHaveYielded(['Suspend! [Result]', 'Loading...']);
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
await resolveText('Result');
await ReactNoop.act(async () => {
resolveText('Result');
});
expect(Scheduler).toFlushExpired(['Result']);
expect(Scheduler).toHaveYielded(['Result']);
expect(ReactNoop.getChildren()).toEqual([span('Result')]);
});
@@ -1156,8 +1158,10 @@ describe('ReactSuspenseWithNoopRenderer', () => {
</>,
);
await resolveText('Step: 2');
expect(Scheduler).toFlushExpired(['Step: 2']);
await ReactNoop.act(async () => {
resolveText('Step: 2');
});
expect(Scheduler).toHaveYielded(['Step: 2']);
expect(ReactNoop).toMatchRenderedOutput(
<>
<span prop="Step: 2" />
@@ -1227,9 +1231,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
</>,
);
await resolveText('B');
await ReactNoop.act(async () => {
resolveText('B');
});
expect(Scheduler).toFlushExpired(['B']);
expect(Scheduler).toHaveYielded(['B']);
expect(ReactNoop).toMatchRenderedOutput(
<>
<span prop="A" />
@@ -1271,9 +1277,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
]);
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
await resolveText('Hi');
await ReactNoop.act(async () => {
resolveText('Hi');
});
expect(Scheduler).toFlushExpired([
expect(Scheduler).toHaveYielded([
'constructor',
'Hi',
'componentDidMount',
@@ -1316,8 +1324,10 @@ describe('ReactSuspenseWithNoopRenderer', () => {
'Loading...',
]);
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
await resolveText('Hi');
expect(Scheduler).toFlushExpired(['Hi']);
await ReactNoop.act(async () => {
resolveText('Hi');
});
expect(Scheduler).toHaveYielded(['Hi']);
expect(ReactNoop.getChildren()).toEqual([span('Hi')]);
});
@@ -1360,8 +1370,10 @@ describe('ReactSuspenseWithNoopRenderer', () => {
</>,
]);
await resolveText('Hi');
expect(Scheduler).toFlushExpired(['Hi']);
await ReactNoop.act(async () => {
resolveText('Hi');
});
expect(Scheduler).toHaveYielded(['Hi']);
});
} else {
// @gate enableCache
@@ -1401,9 +1413,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
'Child is hidden: true',
]);
await resolveText('Hi');
await ReactNoop.act(async () => {
resolveText('Hi');
});
expect(Scheduler).toFlushExpired(['Hi']);
expect(Scheduler).toHaveYielded(['Hi']);
});
}
@@ -1647,9 +1661,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
</>,
);
await resolveText('B');
await ReactNoop.act(async () => {
resolveText('B');
});
expect(Scheduler).toFlushAndYield([
expect(Scheduler).toHaveYielded([
'B',
'Destroy Layout Effect [Loading...]',
'Layout Effect [B]',
@@ -1681,9 +1697,11 @@ describe('ReactSuspenseWithNoopRenderer', () => {
'Effect [Loading...]',
]);
await resolveText('B2');
await ReactNoop.act(async () => {
resolveText('B2');
});
expect(Scheduler).toFlushAndYield([
expect(Scheduler).toHaveYielded([
'B2',
'Destroy Layout Effect [Loading...]',
'Destroy Layout Effect [B]',
@@ -1794,7 +1794,7 @@ describe('useMutableSource', () => {
});
// @gate experimental
it('should not misidentify mutations after render as side effects', () => {
it('should not misidentify mutations after render as side effects', async () => {
const source = createSource('initial');
const mutableSource = createMutableSource(
source,
@@ -1811,15 +1811,16 @@ describe('useMutableSource', () => {
return null;
}
act(() => {
await act(async () => {
ReactNoop.renderLegacySyncRoot(
<React.StrictMode>
<MutateDuringRead />
</React.StrictMode>,
);
expect(Scheduler).toFlushAndYieldThrough([
'MutateDuringRead:initial',
]);
});
expect(Scheduler).toHaveYielded(['MutateDuringRead:initial']);
await act(async () => {
source.value = 'updated';
});
expect(Scheduler).toHaveYielded(['MutateDuringRead:updated']);
+2
View File
@@ -155,4 +155,6 @@ export const disableSchedulerTimeoutInWorkLoop = false;
export const enableDiscreteEventMicroTasks = false;
export const enableSyncMicroTasks = false;
export const enableNativeEventPriorityInference = false;
@@ -59,6 +59,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableDiscreteEventMicroTasks = false;
export const enableSyncMicroTasks = false;
export const enableNativeEventPriorityInference = false;
// Flow magic to verify the exports of this file match the original version.
@@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableDiscreteEventMicroTasks = false;
export const enableSyncMicroTasks = false;
export const enableNativeEventPriorityInference = false;
// Flow magic to verify the exports of this file match the original version.
@@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableDiscreteEventMicroTasks = false;
export const enableSyncMicroTasks = false;
export const enableNativeEventPriorityInference = false;
// Flow magic to verify the exports of this file match the original version.
@@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableDiscreteEventMicroTasks = false;
export const enableSyncMicroTasks = false;
export const enableNativeEventPriorityInference = false;
// Flow magic to verify the exports of this file match the original version.
@@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableDiscreteEventMicroTasks = false;
export const enableSyncMicroTasks = false;
export const enableNativeEventPriorityInference = false;
// Flow magic to verify the exports of this file match the original version.
@@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableDiscreteEventMicroTasks = false;
export const enableSyncMicroTasks = false;
export const enableNativeEventPriorityInference = false;
// Flow magic to verify the exports of this file match the original version.
@@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableDiscreteEventMicroTasks = false;
export const enableSyncMicroTasks = false;
export const enableNativeEventPriorityInference = false;
// Flow magic to verify the exports of this file match the original version.
@@ -57,4 +57,5 @@ export const enableUseRefAccessWarning = __VARIANT__;
export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
export const enableDiscreteEventMicroTasks = __VARIANT__;
export const enableSyncMicroTasks = __VARIANT__;
export const enableNativeEventPriorityInference = __VARIANT__;
@@ -33,6 +33,7 @@ export const {
disableNativeComponentFrames,
disableSchedulerTimeoutInWorkLoop,
enableDiscreteEventMicroTasks,
enableSyncMicroTasks,
enableNativeEventPriorityInference,
} = dynamicFeatureFlags;