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 2 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
35 changes: 26 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,27 @@ 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__) {
invariant(
existingCallbackNode,
rickhanlonii marked this conversation as resolved.
Show resolved Hide resolved
'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 +745,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 +755,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 +1887,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
35 changes: 26 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,27 @@ 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__) {
invariant(
existingCallbackNode,
'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 +727,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 +737,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 +1867,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
3 changes: 2 additions & 1 deletion scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -372,5 +372,6 @@
"381": "This feature is not supported by ReactSuspenseTestUtils.",
"382": "This query has received more parameters than the last time the same query was used. Always pass the exact number of parameters that the query needs.",
"383": "This query has received fewer parameters than the last time the same query was used. Always pass the exact number of parameters that the query needs.",
"384": "Refreshing the cache is not supported in Server Components."
"384": "Refreshing the cache is not supported in Server Components.",
"385": "Expected scheduled callback to exist. This error is likely caused by a bug in React. Please file an issue."
rickhanlonii marked this conversation as resolved.
Show resolved Hide resolved
}