Skip to content

Commit

Permalink
Queue discrete events in microtask (#20669)
Browse files Browse the repository at this point in the history
* Queue discrete events in microtask

* Use callback priority to determine cancellation

* Add queueMicrotask to react-reconciler README

* Fix invatiant conditon for InputDiscrete

* Switch invariant null check

* Convert invariant to warning

* Remove warning from codes.json
  • Loading branch information
rickhanlonii authored Jan 27, 2021
1 parent aa736a0 commit e51bd6c
Show file tree
Hide file tree
Showing 18 changed files with 111 additions and 27 deletions.
2 changes: 1 addition & 1 deletion packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ export const queueMicrotask: any =
Promise.resolve(null)
.then(callback)
.catch(handleErrorInNextTick)
: scheduleTimeout;
: scheduleTimeout; // TODO: Determine the best fallback here.

function handleErrorInNextTick(error) {
setTimeout(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,15 @@ describe('ChangeEventPlugin', () => {

// Flush callbacks.
// Now the click update has flushed.
expect(Scheduler).toFlushAndYield(['render: ']);
expect(input.value).toBe('');
if (gate(flags => flags.enableDiscreteEventMicroTasks)) {
// Flush microtask queue.
await null;
expect(Scheduler).toHaveYielded(['render: ']);
expect(input.value).toBe('');
} else {
expect(Scheduler).toFlushAndYield(['render: ']);
expect(input.value).toBe('');
}
});

// @gate experimental
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,24 @@ describe('SimpleEventPlugin', function() {
'High-pri count: 7, Low-pri count: 0',
]);

// At the end, both counters should equal the total number of clicks
expect(Scheduler).toFlushAndYield([
'High-pri count: 8, Low-pri count: 0',
'High-pri count: 8, Low-pri count: 8',
]);
if (gate(flags => flags.enableDiscreteEventMicroTasks)) {
// Flush the microtask queue
await null;

// At the end, both counters should equal the total number of clicks
expect(Scheduler).toHaveYielded([
'High-pri count: 8, Low-pri count: 0',

// TODO: with cancellation, this required another flush?
'High-pri count: 8, Low-pri count: 8',
]);
} else {
// At the end, both counters should equal the total number of clicks
expect(Scheduler).toFlushAndYield([
'High-pri count: 8, Low-pri count: 0',
'High-pri count: 8, Low-pri count: 8',
]);
}
expect(button.textContent).toEqual('High-pri count: 8, Low-pri count: 8');
});
});
Expand Down
4 changes: 4 additions & 0 deletions packages/react-reconciler/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ You can proxy this to `clearTimeout` or its equivalent in your environment.

This is a property (not a function) that should be set to something that can never be a valid timeout ID. For example, you can set it to `-1`.

#### `queueMicrotask(fn)`

You can proxy this to `queueMicrotask` or its equivalent in your environment.

#### `isPrimaryRenderer`

This is a property (not a function) that should be set to `true` if your renderer is the main one on the page. For example, if you're writing a renderer for the Terminal, it makes sense to set it to `true`, but if your renderer is used *on top of* React DOM or some other existing renderer, set it to `false`.
Expand Down
42 changes: 33 additions & 9 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ import {
warnsIfNotActing,
afterActiveInstanceBlur,
clearContainer,
queueMicrotask,
} from './ReactFiberHostConfig';

import {
Expand Down Expand Up @@ -216,6 +217,7 @@ import {
syncNestedUpdateFlag,
} from './ReactProfilerTimer.new';

import {enableDiscreteEventMicroTasks} from 'shared/ReactFeatureFlags';
// DEV stuff
import getComponentName from 'shared/getComponentName';
import ReactStrictModeWarnings from './ReactStrictModeWarnings.new';
Expand Down Expand Up @@ -714,21 +716,34 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
// Special case: There's nothing to work on.
if (existingCallbackNode !== null) {
cancelCallback(existingCallbackNode);
root.callbackNode = null;
root.callbackPriority = NoLanePriority;
}
root.callbackNode = null;
root.callbackPriority = NoLanePriority;
return;
}

// Check if there's an existing task. We may be able to reuse it.
if (existingCallbackNode !== null) {
const existingCallbackPriority = root.callbackPriority;
if (existingCallbackPriority === newCallbackPriority) {
// The priority hasn't changed. We can reuse the existing task. Exit.
return;
const existingCallbackPriority = root.callbackPriority;
if (existingCallbackPriority === newCallbackPriority) {
if (__DEV__) {
// If we're going to re-use an existing task, it needs to exist.
// Assume that discrete update microtasks are non-cancellable and null.
// TODO: Temporary until we confirm this warning is not fired.
if (
existingCallbackNode == null &&
existingCallbackPriority !== InputDiscreteLanePriority
) {
console.error(
'Expected scheduled callback to exist. This error is likely caused by a bug in React. Please file an issue.',
);
}
}
// The priority changed. Cancel the existing callback. We'll schedule a new
// one below.
// The priority hasn't changed. We can reuse the existing task. Exit.
return;
}

if (existingCallbackNode != null) {
// Cancel the existing callback. We'll schedule a new one below.
cancelCallback(existingCallbackNode);
}

Expand All @@ -737,6 +752,8 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
if (newCallbackPriority === SyncLanePriority) {
// Special case: Sync React callbacks are scheduled on a special
// internal queue

// TODO: After enableDiscreteEventMicroTasks lands, we can remove the fake node.
newCallbackNode = scheduleSyncCallback(
performSyncWorkOnRoot.bind(null, root),
);
Expand All @@ -745,6 +762,12 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
ImmediateSchedulerPriority,
performSyncWorkOnRoot.bind(null, root),
);
} else if (
enableDiscreteEventMicroTasks &&
newCallbackPriority === InputDiscreteLanePriority
) {
queueMicrotask(performSyncWorkOnRoot.bind(null, root));
newCallbackNode = null;
} else {
const schedulerPriorityLevel = lanePriorityToSchedulerPriority(
newCallbackPriority,
Expand Down Expand Up @@ -1871,6 +1894,7 @@ function commitRootImpl(root, renderPriorityLevel) {
// commitRoot never returns a continuation; it always finishes synchronously.
// So we can clear these now to allow a new callback to be scheduled.
root.callbackNode = null;
root.callbackPriority = NoLanePriority;

// Update the first and last pending times on this root. The new first
// pending time is whatever is left on the root fiber.
Expand Down
42 changes: 33 additions & 9 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
disableSchedulerTimeoutInWorkLoop,
enableDoubleInvokingEffects,
skipUnmountedBoundaries,
enableDiscreteEventMicroTasks,
} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -92,6 +93,7 @@ import {
warnsIfNotActing,
afterActiveInstanceBlur,
clearContainer,
queueMicrotask,
} from './ReactFiberHostConfig';

import {
Expand Down Expand Up @@ -696,21 +698,34 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
// Special case: There's nothing to work on.
if (existingCallbackNode !== null) {
cancelCallback(existingCallbackNode);
root.callbackNode = null;
root.callbackPriority = NoLanePriority;
}
root.callbackNode = null;
root.callbackPriority = NoLanePriority;
return;
}

// Check if there's an existing task. We may be able to reuse it.
if (existingCallbackNode !== null) {
const existingCallbackPriority = root.callbackPriority;
if (existingCallbackPriority === newCallbackPriority) {
// The priority hasn't changed. We can reuse the existing task. Exit.
return;
const existingCallbackPriority = root.callbackPriority;
if (existingCallbackPriority === newCallbackPriority) {
if (__DEV__) {
// If we're going to re-use an existing task, it needs to exist.
// Assume that discrete update microtasks are non-cancellable and null.
// TODO: Temporary until we confirm this warning is not fired.
if (
existingCallbackNode == null &&
existingCallbackPriority !== InputDiscreteLanePriority
) {
console.error(
'Expected scheduled callback to exist. This error is likely caused by a bug in React. Please file an issue.',
);
}
}
// The priority changed. Cancel the existing callback. We'll schedule a new
// one below.
// The priority hasn't changed. We can reuse the existing task. Exit.
return;
}

if (existingCallbackNode != null) {
// Cancel the existing callback. We'll schedule a new one below.
cancelCallback(existingCallbackNode);
}

Expand All @@ -719,6 +734,8 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
if (newCallbackPriority === SyncLanePriority) {
// Special case: Sync React callbacks are scheduled on a special
// internal queue

// TODO: After enableDiscreteEventMicroTasks lands, we can remove the fake node.
newCallbackNode = scheduleSyncCallback(
performSyncWorkOnRoot.bind(null, root),
);
Expand All @@ -727,6 +744,12 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) {
ImmediateSchedulerPriority,
performSyncWorkOnRoot.bind(null, root),
);
} else if (
enableDiscreteEventMicroTasks &&
newCallbackPriority === InputDiscreteLanePriority
) {
queueMicrotask(performSyncWorkOnRoot.bind(null, root));
newCallbackNode = null;
} else {
const schedulerPriorityLevel = lanePriorityToSchedulerPriority(
newCallbackPriority,
Expand Down Expand Up @@ -1851,6 +1874,7 @@ function commitRootImpl(root, renderPriorityLevel) {
// commitRoot never returns a continuation; it always finishes synchronously.
// So we can clear these now to allow a new callback to be scheduled.
root.callbackNode = null;
root.callbackPriority = NoLanePriority;

// Update the first and last pending times on this root. The new first
// pending time is whatever is left on the root fiber.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3528,6 +3528,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
});

// @gate enableCache
// @gate !enableDiscreteEventMicroTasks
it('regression: empty render at high priority causes update to be dropped', async () => {
// Reproduces a bug where flushDiscreteUpdates starts a new (empty) render
// pass which cancels a scheduled timeout and causes the fallback never to
Expand Down
2 changes: 1 addition & 1 deletion packages/react-test-renderer/src/ReactTestHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export const queueMicrotask =
Promise.resolve(null)
.then(callback)
.catch(handleErrorInNextTick)
: scheduleTimeout;
: scheduleTimeout; // TODO: Determine the best fallback here.

function handleErrorInNextTick(error) {
setTimeout(() => {
Expand Down
2 changes: 2 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,5 @@ export const disableSchedulerTimeoutInWorkLoop = false;

// Experiment to simplify/improve how transitions are scheduled
export const enableTransitionEntanglement = false;

export const enableDiscreteEventMicroTasks = false;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableTransitionEntanglement = false;
export const enableDiscreteEventMicroTasks = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableTransitionEntanglement = false;
export const enableDiscreteEventMicroTasks = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableTransitionEntanglement = false;
export const enableDiscreteEventMicroTasks = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableTransitionEntanglement = false;
export const enableDiscreteEventMicroTasks = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableTransitionEntanglement = false;
export const enableDiscreteEventMicroTasks = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableTransitionEntanglement = false;
export const enableDiscreteEventMicroTasks = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
export const enableRecursiveCommitTraversal = false;
export const disableSchedulerTimeoutInWorkLoop = false;
export const enableTransitionEntanglement = false;
export const enableDiscreteEventMicroTasks = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,4 @@ export const enableUseRefAccessWarning = __VARIANT__;
export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
export const enableTransitionEntanglement = __VARIANT__;
export const enableDiscreteEventMicroTasks = __VARIANT__;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const {
disableNativeComponentFrames,
disableSchedulerTimeoutInWorkLoop,
enableTransitionEntanglement,
enableDiscreteEventMicroTasks,
} = dynamicFeatureFlags;

// On WWW, __EXPERIMENTAL__ is used for a new modern build.
Expand Down

0 comments on commit e51bd6c

Please sign in to comment.