Idle updates should not be blocked by hidden work

packages/react-reconciler/src/ReactFiberExpirationTime.js

@@ -21,9 +21,16 @@ import {
 export type ExpirationTime = number;
 
 export const NoWork = 0;
-// TODO: Think of a better name for Never.
+// TODO: Think of a better name for Never. The key difference with Idle is that
+// Never work can be committed in an inconsistent state without tearing the UI.
+// The main example is offscreen content, like a hidden subtree. So one possible
+// name is Offscreen. However, it also includes dehydrated Suspense boundaries,
+// which are inconsistent in the sense that they haven't finished yet, but
+// aren't visibly inconsistent because the server rendered HTML matches what the
+// hydrated tree would look like.
 export const Never = 1;
-// TODO: Use the Idle expiration time for idle state updates
+// Idle is slightly higher priority than Never. It must completely finish in
+// order to be consistent.
 export const Idle = 2;
 export const Sync = MAX_SIGNED_31_BIT_INT;
 export const Batched = Sync - 1;
@@ -115,7 +122,7 @@ export function inferPriorityFromExpirationTime(
   if (expirationTime === Sync) {
     return ImmediatePriority;
   }
-  if (expirationTime === Never) {
+  if (expirationTime === Never || expirationTime === Idle) {
     return IdlePriority;
   }
   const msUntil =

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -321,6 +321,7 @@ export function computeExpirationForFiber(
 
   if ((executionContext & RenderContext) !== NoContext) {
     // Use whatever time we're already rendering
+    // TODO: Should there be a way to opt out, like with `runWithPriority`?
     return renderExpirationTime;
   }
 
@@ -347,7 +348,7 @@ export function computeExpirationForFiber(
         expirationTime = computeAsyncExpiration(currentTime);
         break;
       case IdlePriority:
-        expirationTime = Never;
+        expirationTime = Idle;
         break;
       default:
         invariant(false, 'Expected a valid priority level');
@@ -1406,14 +1407,14 @@ export function markRenderEventTimeAndConfig(
 ): void {
   if (
     expirationTime < workInProgressRootLatestProcessedExpirationTime &&
-    expirationTime > Never
+    expirationTime > Idle
   ) {
     workInProgressRootLatestProcessedExpirationTime = expirationTime;
   }
   if (suspenseConfig !== null) {
     if (
       expirationTime < workInProgressRootLatestSuspenseTimeout &&
-      expirationTime > Never
+      expirationTime > Idle
     ) {
       workInProgressRootLatestSuspenseTimeout = expirationTime;
       // Most of the time we only have one config and getting wrong is not bad.
@@ -2203,24 +2204,25 @@ function commitLayoutEffects(
 }
 
 export function flushPassiveEffects() {
+  if (pendingPassiveEffectsRenderPriority !== NoPriority) {
+    const priorityLevel =
+      pendingPassiveEffectsRenderPriority > NormalPriority
+        ? NormalPriority
+        : pendingPassiveEffectsRenderPriority;
+    pendingPassiveEffectsRenderPriority = NoPriority;
+    return runWithPriority(priorityLevel, flushPassiveEffectsImpl);
+  }
+}
+
+function flushPassiveEffectsImpl() {
   if (rootWithPendingPassiveEffects === null) {
     return false;
   }
   const root = rootWithPendingPassiveEffects;
   const expirationTime = pendingPassiveEffectsExpirationTime;
-  const renderPriorityLevel = pendingPassiveEffectsRenderPriority;
   rootWithPendingPassiveEffects = null;
   pendingPassiveEffectsExpirationTime = NoWork;
-  pendingPassiveEffectsRenderPriority = NoPriority;
-  const priorityLevel =
-    renderPriorityLevel > NormalPriority ? NormalPriority : renderPriorityLevel;
-  return runWithPriority(
-    priorityLevel,
-    flushPassiveEffectsImpl.bind(null, root, expirationTime),
-  );
-}
 
-function flushPassiveEffectsImpl(root, expirationTime) {
   invariant(
     (executionContext & (RenderContext | CommitContext)) === NoContext,
     'Cannot flush passive effects while already rendering.',
@@ -2263,6 +2265,7 @@ function flushPassiveEffectsImpl(root, expirationTime) {
   }
 
   executionContext = prevExecutionContext;
+
   flushSyncCallbackQueue();
 
   // If additional passive effects were scheduled, increment a counter. If this

packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js

@@ -352,4 +352,60 @@ describe('ReactSchedulerIntegration', () => {
     Scheduler.unstable_flushUntilNextPaint();
     expect(Scheduler).toHaveYielded(['A', 'B', 'C']);
   });
+
+  it('idle updates are not blocked by offscreen work', async () => {
+    function Text({text}) {
+      Scheduler.unstable_yieldValue(text);
+      return text;
+    }
+
+    function App({label}) {
+      return (
+        <>
+          <Text text={`Visible: ` + label} />
+          <div hidden={true}>
+            <Text text={`Hidden: ` + label} />
+          </div>
+        </>
+      );
+    }
+
+    const root = ReactNoop.createRoot();
+    await ReactNoop.act(async () => {
+      root.render(<App label="A" />);
+
+      // Commit the visible content
+      expect(Scheduler).toFlushUntilNextPaint(['Visible: A']);
+      expect(root).toMatchRenderedOutput(
+        <>
+          Visible: A
+          <div hidden={true} />
+        </>,
+      );
+
+      // Before the hidden content has a chance to render, schedule an
+      // idle update
+      runWithPriority(IdlePriority, () => {
+        root.render(<App label="B" />);
+      });
+
+      // The next commit should only include the visible content
+      expect(Scheduler).toFlushUntilNextPaint(['Visible: B']);
+      expect(root).toMatchRenderedOutput(
+        <>
+          Visible: B
+          <div hidden={true} />
+        </>,
+      );
+    });
+
+    // The hidden content commits later
+    expect(Scheduler).toHaveYielded(['Hidden: B']);
+    expect(root).toMatchRenderedOutput(
+      <>
+        Visible: B
+        <div hidden={true}>Hidden: B</div>
+      </>,
+    );
+  });
 });

packages/react/src/__tests__/ReactDOMTracing-test.internal.js

@@ -134,7 +134,12 @@ describe('ReactDOMTracing', () => {
         expect(
           onInteractionScheduledWorkCompleted,
         ).toHaveBeenLastNotifiedOfInteraction(interaction);
-        expect(onRender).toHaveBeenCalledTimes(3);
+        // TODO: This is 4 instead of 3 because this update was scheduled at
+        // idle priority, and idle updates are slightly higher priority than
+        // offscreen work. So it takes two render passes to finish it. Profiler
+        // calls `onRender` for the first render even though everything
+        // bails out.
+        expect(onRender).toHaveBeenCalledTimes(4);
         expect(onRender).toHaveLastRenderedWithInteractions(
           new Set([interaction]),
         );
@@ -281,7 +286,12 @@ describe('ReactDOMTracing', () => {
         expect(
           onInteractionScheduledWorkCompleted,
         ).toHaveBeenLastNotifiedOfInteraction(interaction);
-        expect(onRender).toHaveBeenCalledTimes(3);
+        // TODO: This is 4 instead of 3 because this update was scheduled at
+        // idle priority, and idle updates are slightly higher priority than
+        // offscreen work. So it takes two render passes to finish it. Profiler
+        // calls `onRender` for the first render even though everything
+        // bails out.
+        expect(onRender).toHaveBeenCalledTimes(4);
         expect(onRender).toHaveLastRenderedWithInteractions(
           new Set([interaction]),
         );

scripts/jest/matchers/schedulerTestMatchers.js

@@ -44,6 +44,15 @@ function toFlushAndYieldThrough(Scheduler, expectedYields) {
   });
 }
 
+function toFlushUntilNextPaint(Scheduler, expectedYields) {
+  assertYieldsWereCleared(Scheduler);
+  Scheduler.unstable_flushUntilNextPaint();
+  const actualYields = Scheduler.unstable_clearYields();
+  return captureAssertion(() => {
+    expect(actualYields).toEqual(expectedYields);
+  });
+}
+
 function toFlushWithoutYielding(Scheduler) {
   return toFlushAndYield(Scheduler, []);
 }
@@ -76,6 +85,7 @@ function toFlushAndThrow(Scheduler, ...rest) {
 module.exports = {
   toFlushAndYield,
   toFlushAndYieldThrough,
+  toFlushUntilNextPaint,
   toFlushWithoutYielding,
   toFlushExpired,
   toHaveYielded,