diff --git a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js index c75b7a6734..9793d43d90 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js @@ -81,7 +81,7 @@ import { } from 'react-reconciler/src/ReactWorkTags'; import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem'; -import {UnknownEventPriority} from 'react-reconciler/src/ReactEventPriorities'; +import {DefaultEventPriority} from 'react-reconciler/src/ReactEventPriorities'; // TODO: Remove this deep import when we delete the legacy root API import {ConcurrentMode, NoMode} from 'react-reconciler/src/ReactTypeOfMode'; @@ -384,7 +384,7 @@ export function createTextInstance( export function getCurrentEventPriority(): EventPriority { const currentEvent = window.event; if (currentEvent === undefined) { - return UnknownEventPriority; + return DefaultEventPriority; } return getEventPriority(currentEvent.type); } @@ -451,37 +451,58 @@ export const scheduleMicrotask: any = .catch(handleErrorInNextTick) : scheduleTimeout; // TODO: Determine the best fallback here. -// ------------------- -// requestAnimationFrame -// ------------------- -type FrameAlignedTask = { - frameNode: any, - callbackNode: any, -}; - -// TODO: Fix these types export const supportsFrameAlignedTask = true; -export function scheduleFrameAlignedTask(task: any): FrameAlignedTask { - // Schedule both tasks, we'll race them and use the first to fire. - const raf: any = localRequestAnimationFrame; - return { - frameNode: raf(task), - callbackNode: Scheduler.unstable_scheduleCallback( - Scheduler.unstable_NormalPriority, - task, - ), - }; +type FrameAlignedTask = {| + rafNode: AnimationFrameID, + schedulerNode: number | null, + task: function, +|}; + +let currentTask: FrameAlignedTask | null = null; +function performFrameAlignedWork() { + if (currentTask != null) { + const task = currentTask.task; + localCancelAnimationFrame(currentTask.rafNode); + Scheduler.unstable_cancelCallback(currentTask.schedulerNode); + currentTask = null; + if (task != null) { + task(); + } + } } -export function cancelFrameAlignedTask(task: any) { - const caf: any = localCancelAnimationFrame; - if (task.frameNode != null) { - caf(task.frameNode); + +export function scheduleFrameAlignedTask(task: any): any { + if (currentTask === null) { + const rafNode = localRequestAnimationFrame(performFrameAlignedWork); + + const schedulerNode = Scheduler.unstable_scheduleCallback( + Scheduler.unstable_NormalPriority, + performFrameAlignedWork, + ); + + currentTask = { + rafNode, + schedulerNode, + task, + }; + } else { + currentTask.task = task; + currentTask.schedulerNode = Scheduler.unstable_scheduleCallback( + Scheduler.unstable_NormalPriority, + performFrameAlignedWork, + ); } - if (task.callbackNode != null) { - Scheduler.unstable_cancelCallback(task.callbackNode); - } + return currentTask; +} + +export function cancelFrameAlignedTask(task: any) { + Scheduler.unstable_cancelCallback(task.schedulerNode); + task.schedulerNode = null; + // We don't cancel the rAF in case it gets re-used later. + // But clear the task so if it fires and shouldn't run, it won't. + task.task = null; } function handleErrorInNextTick(error) { diff --git a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js index e77b09fb52..a2ba9e5768 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.js @@ -633,6 +633,44 @@ describe('ReactDOMFiberAsync', () => { expect(counterRef.current.textContent).toBe('Count: 2'); }); + // @gate enableFrameEndScheduling + it('Should re-use scheduled rAF, not cancel and schedule anew', () => { + let setState = null; + let counterRef = null; + function Counter() { + const [count, setCount] = React.useState(0); + const ref = React.useRef(); + setState = setCount; + counterRef = ref; + Scheduler.unstable_yieldValue('Count: ' + count); + return

Count: {count}

; + } + + const root = ReactDOMClient.createRoot(container); + act(() => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['Count: 0']); + + window.event = undefined; + setState(1); + // Unknown updates should schedule a rAF. + expect(global.requestAnimationFrameQueue.length).toBe(1); + const firstRaf = global.requestAnimationFrameQueue[0]; + + setState(2); + // Default updates after unknown should re-use the scheduled rAF. + expect(global.requestAnimationFrameQueue.length).toBe(1); + const secondRaf = global.requestAnimationFrameQueue[0]; + expect(firstRaf).toBe(secondRaf); + + expect(Scheduler).toHaveYielded([]); + expect(counterRef.current.textContent).toBe('Count: 0'); + global.flushRequestAnimationFrameQueue(); + expect(Scheduler).toHaveYielded(['Count: 2']); + expect(counterRef.current.textContent).toBe('Count: 2'); + }); + // @gate enableFrameEndScheduling it('Default update followed by an unknown update is batched, scheduled in a rAF', () => { let setState = null; @@ -655,8 +693,8 @@ describe('ReactDOMFiberAsync', () => { window.event = 'test'; setState(1); - // We should not schedule a rAF for default updates only. - expect(global.requestAnimationFrameQueue).toBe(null); + // We should schedule a rAF for default updates. + expect(global.requestAnimationFrameQueue.length).toBe(1); window.event = undefined; setState(2); @@ -692,8 +730,8 @@ describe('ReactDOMFiberAsync', () => { window.event = 'test'; setState(1); - // We should not schedule a rAF for default updates only. - expect(global.requestAnimationFrameQueue).toBe(null); + // We should schedule a rAF for default updates. + expect(global.requestAnimationFrameQueue.length).toBe(1); window.event = undefined; setState(2); @@ -892,28 +930,25 @@ describe('ReactDOMFiberAsync', () => { window.event = undefined; setState(1); + expect(global.requestAnimationFrameQueue.length).toBe(1); global.flushRequestAnimationFrameQueue(); expect(Scheduler).toHaveYielded(['Count: 1']); setState(2); setThrowing(true); - + expect(global.requestAnimationFrameQueue.length).toBe(1); global.flushRequestAnimationFrameQueue(); expect(Scheduler).toHaveYielded(['Count: 2', 'suspending']); expect(counterRef.current.textContent).toBe('Count: 1'); unsuspend(); - setThrowing(false); - - // Should not be scheduled in a rAF. + // Default update should be scheduled in a rAF. window.event = 'test'; + setThrowing(false); setState(2); - // TODO: This should not yield - // global.flushRequestAnimationFrameQueue(); - // expect(Scheduler).toHaveYielded([]); - - expect(Scheduler).toFlushAndYield(['Count: 2']); + global.flushRequestAnimationFrameQueue(); + expect(Scheduler).toHaveYielded(['Count: 2']); expect(counterRef.current.textContent).toBe('Count: 2'); }); }); diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 322379b4b5..fa11fc0c55 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -1442,7 +1442,6 @@ function useMutableSource( setSnapshot(maybeNewSnapshot); const lane = requestUpdateLane(fiber); - // TODO: What to do about isUnknownEventPriority markRootMutableRead(root, lane); } // If the source mutated between render and now, @@ -1463,7 +1462,6 @@ function useMutableSource( // Record a pending mutable source update with the same expiration time. const lane = requestUpdateLane(fiber); - // TODO: What to do about isUnknownEventPriority markRootMutableRead(root, lane); } catch (error) { // A selector might throw after a source mutation. diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 1d9800d7e4..d546abd910 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -500,11 +500,7 @@ export function includesBlockingLane(root: FiberRoot, lanes: Lanes): boolean { allowConcurrentByDefault && (root.current.mode & ConcurrentUpdatesByDefaultMode) !== NoMode ) { - if ( - enableFrameEndScheduling && - (lanes & DefaultLane) !== NoLanes && - root.hasUnknownUpdates - ) { + if (enableFrameEndScheduling && (lanes & DefaultLane) !== NoLanes) { // Unknown updates should flush synchronously, even in concurrent by default. return true; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 187006cff9..1b3c2dcf7f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -928,10 +928,10 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { if ( enableFrameEndScheduling && - newCallbackPriority === DefaultLane && - root.hasUnknownUpdates + supportsFrameAlignedTask && + newCallbackPriority === DefaultLane ) { - // Do nothing, we need to cancel the existing default task and schedule a rAF. + // Do nothing, we need to schedule a new rAF. } else { // The priority hasn't changed. We can reuse the existing task. Exit. return; @@ -943,9 +943,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { if ( enableFrameEndScheduling && supportsFrameAlignedTask && - existingCallbackNode != null && - // TODO: is there a better check for callbackNode type? - existingCallbackNode.frameNode != null + existingCallbackPriority === DefaultLane ) { cancelFrameAlignedTask(existingCallbackNode); } else { @@ -997,8 +995,7 @@ function ensureRootIsScheduled(root: FiberRoot, currentTime: number) { } else if ( enableFrameEndScheduling && supportsFrameAlignedTask && - newCallbackPriority === DefaultLane && - root.hasUnknownUpdates + newCallbackPriority === DefaultLane ) { if (__DEV__ && ReactCurrentActQueue.current !== null) { // Inside `act`, use our internal `act` queue so that these get flushed diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index 76c4fd1cc0..ed5634d200 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -64,9 +64,11 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { if (error) { throw error; } + if (global.requestAnimationFrameQueue != null) { + console.warn('requestAnimationFrameQueue has not been flushed.'); + } }); env.beforeEach(() => { - // TODO: warn if this has not flushed. global.requestAnimationFrameQueue = null; });