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

Queue discrete events in microtask #20669

Merged
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
rickhanlonii marked this conversation as resolved.
Show resolved Hide resolved
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 (
rickhanlonii marked this conversation as resolved.
Show resolved Hide resolved
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