Use frameAligned for DefaultUpdate

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) {

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 <p ref={ref}>Count: {count}</p>;
+      }
+
+      const root = ReactDOMClient.createRoot(container);
+      act(() => {
+        root.render(<Counter />);
+      });
+      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');
     });
   });

packages/react-reconciler/src/ReactFiberHooks.js

@@ -1442,7 +1442,6 @@ function useMutableSource<Source, Snapshot>(
         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<Source, Snapshot>(
 
         // 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.

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;
     }

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

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;
   });