Suspensily committing a prerendered tree (#26434)

Prerendering a tree (i.e. with Offscreen) should not suspend the commit
phase, because the content is not yet visible. However, when revealing a
prerendered tree, we should suspend the commit phase if resources in the
prerendered tree haven't finished loading yet.

To do this properly, we need to visit all the visible nodes in the tree
that might possibly suspend. This includes nodes in the current tree,
because even though they were already "mounted", the resources might not
have loaded yet, because we didn't suspend when it was prerendered.

We will need to add this capability to the Offscreen component's
"manual" mode, too. Something like a `ready()` method that returns a
promise that resolves when the tree has fully loaded.

Also includes some fixes to #26450. See PR for details.

Author: acdlite

Diff for: packages/react-dom-bindings/src/client/ReactDOMHostConfig.js

@@ -1788,7 +1788,7 @@ type StyleTagResource = TResource<'style', null>;
 type StyleResource = StyleTagResource | StylesheetResource;
 type ScriptResource = TResource<'script', null>;
 type VoidResource = TResource<'void', null>;
-type Resource = StyleResource | ScriptResource | VoidResource;
+export type Resource = StyleResource | ScriptResource | VoidResource;
 
 type LoadingState = number;
 const NotLoaded = /*       */ 0b000;
@@ -2170,6 +2170,7 @@ function preinit(href: string, options: PreinitOptions) {
             state.loading |= Errored;
           });
 
+          state.loading |= Inserted;
           insertStylesheet(instance, precedence, resourceRoot);
         }
 
@@ -2518,6 +2519,11 @@ export function acquireResource(
 
         markNodeAsHoistable(instance);
         setInitialProperties(instance, 'style', styleProps);
+
+        // TODO: `style` does not have loading state for tracking insertions. I
+        // guess because these aren't suspensey? Not sure whether this is a
+        // factoring smell.
+        // resource.state.loading |= Inserted;
         insertStylesheet(instance, qualifiedProps.precedence, hoistableRoot);
         resource.instance = instance;
 
@@ -2556,6 +2562,7 @@ export function acquireResource(
           linkInstance.onerror = reject;
         });
         setInitialProperties(instance, 'link', stylesheetProps);
+        resource.state.loading |= Inserted;
         insertStylesheet(instance, qualifiedProps.precedence, hoistableRoot);
         resource.instance = instance;
 
@@ -2604,6 +2611,28 @@ export function acquireResource(
         );
       }
     }
+  } else {
+    // In the case of stylesheets, they might have already been assigned an
+    // instance during `suspendResource`. But that doesn't mean they were
+    // inserted, because the commit might have been interrupted. So we need to
+    // check now.
+    //
+    // The other resource types are unaffected because they are not
+    // yet suspensey.
+    //
+    // TODO: This is a bit of a code smell. Consider refactoring how
+    // `suspendResource` and `acquireResource` work together. The idea is that
+    // `suspendResource` does all the same stuff as `acquireResource` except
+    // for the insertion.
+    if (
+      resource.type === 'stylesheet' &&
+      (resource.state.loading & Inserted) === NotLoaded
+    ) {
+      const qualifiedProps: StylesheetQualifyingProps = props;
+      const instance: Instance = resource.instance;
+      resource.state.loading |= Inserted;
+      insertStylesheet(instance, qualifiedProps.precedence, hoistableRoot);
+    }
   }
   return resource.instance;
 }
@@ -2613,7 +2642,7 @@ export function releaseResource(resource: Resource): void {
 }
 
 function insertStylesheet(
-  instance: HTMLElement,
+  instance: Element,
   precedence: string,
   root: HoistableRoot,
 ): void {

Diff for: packages/react-dom/src/__tests__/ReactDOMFloat-test.js

@@ -2939,7 +2939,6 @@ body {
     );
   });
 
-  // @gate TODO
   it('can interrupt a suspended commit with a new update', async () => {
     function App({children}) {
       return (
@@ -2949,9 +2948,13 @@ body {
       );
     }
     const root = ReactDOMClient.createRoot(document);
+
+    // Do an initial render. This means subsequent insertions will suspend,
+    // unless they are wrapped inside a fresh Suspense boundary.
     root.render(<App />);
     await waitForAll([]);
 
+    // Insert a stylesheet. This will suspend because it's a transition.
     React.startTransition(() => {
       root.render(
         <App>
@@ -2961,6 +2964,7 @@ body {
       );
     });
     await waitForAll([]);
+    // Although the commit suspended, a preload was inserted.
     expect(getMeaningfulChildren(document)).toEqual(
       <html>
         <head>
@@ -2970,6 +2974,9 @@ body {
       </html>,
     );
 
+    // Before the stylesheet has loaded, do an urgent update. This will insert a
+    // different stylesheet, and cancel the first one. This stylesheet will not
+    // suspend, even though it hasn't loaded, because it's an urgent update.
     root.render(
       <App>
         hello2
@@ -2978,6 +2985,9 @@ body {
       </App>,
     );
     await waitForAll([]);
+
+    // The bar stylesheet was inserted. There's still a "foo" preload, even
+    // though that update was superseded.
     expect(getMeaningfulChildren(document)).toEqual(
       <html>
         <head>
@@ -2989,9 +2999,10 @@ body {
       </html>,
     );
 
-    // Even though foo was preloaded we don't see the stylesheet insert because the commit was cancelled.
-    // If we do a followup render that tries to recommit that resource it will insert right away because
-    // the preload is already loaded
+    // When "foo" finishes loading, nothing happens, because "foo" was not
+    // included in the last root update. However, if we insert "foo" again
+    // later, it should immediately commit without suspending, because it's
+    // been preloaded.
     loadPreloads(['foo']);
     assertLog(['load preload: foo']);
     expect(getMeaningfulChildren(document)).toEqual(
@@ -3005,6 +3016,7 @@ body {
       </html>,
     );
 
+    // Now insert "foo" again.
     React.startTransition(() => {
       root.render(
         <App>
@@ -3015,6 +3027,7 @@ body {
       );
     });
     await waitForAll([]);
+    // Commits without suspending because "foo" was preloaded.
     expect(getMeaningfulChildren(document)).toEqual(
       <html>
         <head>
@@ -3023,7 +3036,7 @@ body {
           <link rel="preload" href="foo" as="style" />
           <link rel="preload" href="bar" as="style" />
         </head>
-        <body>hello2</body>
+        <body>hello3</body>
       </html>,
     );
 

Diff for: packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -94,7 +94,8 @@ import {
   LayoutMask,
   PassiveMask,
   Visibility,
-  SuspenseyCommit,
+  ShouldSuspendCommit,
+  MaySuspendCommit,
 } from './ReactFiberFlags';
 import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
 import {
@@ -4065,12 +4066,23 @@ export function commitPassiveUnmountEffects(finishedWork: Fiber): void {
   resetCurrentDebugFiberInDEV();
 }
 
+// If we're inside a brand new tree, or a tree that was already visible, then we
+// should only suspend host components that have a ShouldSuspendCommit flag.
+// Components without it haven't changed since the last commit, so we can skip
+// over those.
+//
+// When we enter a tree that is being revealed (going from hidden -> visible),
+// we need to suspend _any_ component that _may_ suspend. Even if they're
+// already in the "current" tree. Because their visibility has changed, the
+// browser may not have prerendered them yet. So we check the MaySuspendCommit
+// flag instead.
+let suspenseyCommitFlag = ShouldSuspendCommit;
 export function accumulateSuspenseyCommit(finishedWork: Fiber): void {
   accumulateSuspenseyCommitOnFiber(finishedWork);
 }
 
 function recursivelyAccumulateSuspenseyCommit(parentFiber: Fiber): void {
-  if (parentFiber.subtreeFlags & SuspenseyCommit) {
+  if (parentFiber.subtreeFlags & suspenseyCommitFlag) {
     let child = parentFiber.child;
     while (child !== null) {
       accumulateSuspenseyCommitOnFiber(child);
@@ -4083,7 +4095,7 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber) {
   switch (fiber.tag) {
     case HostHoistable: {
       recursivelyAccumulateSuspenseyCommit(fiber);
-      if (fiber.flags & SuspenseyCommit) {
+      if (fiber.flags & suspenseyCommitFlag) {
         if (fiber.memoizedState !== null) {
           suspendResource(
             // This should always be set by visiting HostRoot first
@@ -4101,7 +4113,7 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber) {
     }
     case HostComponent: {
       recursivelyAccumulateSuspenseyCommit(fiber);
-      if (fiber.flags & SuspenseyCommit) {
+      if (fiber.flags & suspenseyCommitFlag) {
         const type = fiber.type;
         const props = fiber.memoizedProps;
         suspendInstance(type, props);
@@ -4117,10 +4129,33 @@ function accumulateSuspenseyCommitOnFiber(fiber: Fiber) {
 
         recursivelyAccumulateSuspenseyCommit(fiber);
         currentHoistableRoot = previousHoistableRoot;
-        break;
+      } else {
+        recursivelyAccumulateSuspenseyCommit(fiber);
       }
+      break;
+    }
+    case OffscreenComponent: {
+      const isHidden = (fiber.memoizedState: OffscreenState | null) !== null;
+      if (isHidden) {
+        // Don't suspend in hidden trees
+      } else {
+        const current = fiber.alternate;
+        const wasHidden =
+          current !== null &&
+          (current.memoizedState: OffscreenState | null) !== null;
+        if (wasHidden) {
+          // This tree is being revealed. Visit all newly visible suspensey
+          // instances, even if they're in the current tree.
+          const prevFlags = suspenseyCommitFlag;
+          suspenseyCommitFlag = MaySuspendCommit;
+          recursivelyAccumulateSuspenseyCommit(fiber);
+          suspenseyCommitFlag = prevFlags;
+        } else {
+          recursivelyAccumulateSuspenseyCommit(fiber);
+        }
+      }
+      break;
     }
-    // eslint-disable-next-line-no-fallthrough
     default: {
       recursivelyAccumulateSuspenseyCommit(fiber);
     }