Check if suspensey instance resolves in immediate task (#26427)

When rendering a suspensey resource that we haven't seen before, it may
have loaded in the background while we were rendering. We should yield
to the main thread to see if the load event fires in an immediate task.

For example, if the resource for a link element has already loaded, its
load event will fire in a task right after React yields to the main
thread. Because the continuation task is not scheduled until right
before React yields, the load event will ping React before it resumes.

If this happens, we can resume rendering without showing a fallback.

I don't think this matters much for images, because the `completed`
property tells us whether the image has loaded, and during a non-urgent
render, we never block the main thread for more than 5ms at a time (for
now — we might increase this in the future). It matters more for
stylesheets because the only way to check if it has loaded is by
listening for the load event.

This is essentially the same trick that `use` does for userspace
promises, but a bit simpler because we don't need to replay the host
component's begin phase; the work-in-progress fiber already completed,
so we can just continue onto the next sibling without any additional
work.

As part of this change, I split the `shouldSuspendCommit` host config
method into separate `maySuspendCommit` and `preloadInstance` methods.
Previously `shouldSuspendCommit` was used for both.

This raised a question of whether we should preload resources during a
synchronous render. My initial instinct was that we shouldn't, because
we're going to synchronously block the main thread until the resource is
inserted into the DOM, anyway. But I wonder if the browser is able to
initiate the preload even while the main thread is blocked. It's
probably a micro-optimization either way because most resources will be
loaded during transitions, not urgent renders.

Author: acdlite

packages/react-art/src/ReactARTHostConfig.js

@@ -459,10 +459,15 @@ export function requestPostPaintCallback(callback: (time: number) => void) {
   // noop
 }
 
-export function shouldSuspendCommit(type, props) {
+export function maySuspendCommit(type, props) {
   return false;
 }
 
+export function preloadInstance(type, props) {
+  // Return true to indicate it's already loaded
+  return true;
+}
+
 export function startSuspendingCommit() {}
 
 export function suspendInstance(type, props) {}

packages/react-dom-bindings/src/client/ReactDOMHostConfig.js

@@ -1609,10 +1609,15 @@ export function requestPostPaintCallback(callback: (time: number) => void) {
   });
 }
 
-export function shouldSuspendCommit(type: Type, props: Props): boolean {
+export function maySuspendCommit(type: Type, props: Props): boolean {
   return false;
 }
 
+export function preloadInstance(type: Type, props: Props): boolean {
+  // Return true to indicate it's already loaded
+  return true;
+}
+
 export function startSuspendingCommit(): void {}
 
 export function suspendInstance(type: Type, props: Props): void {}

packages/react-native-renderer/src/ReactFabricHostConfig.js

@@ -421,10 +421,14 @@ export function requestPostPaintCallback(callback: (time: number) => void) {
   // noop
 }
 
-export function shouldSuspendCommit(type: Type, props: Props): boolean {
+export function maySuspendCommit(type: Type, props: Props): boolean {
   return false;
 }
 
+export function preloadInstance(type: Type, props: Props): boolean {
+  return true;
+}
+
 export function startSuspendingCommit(): void {}
 
 export function suspendInstance(type: Type, props: Props): void {}

packages/react-native-renderer/src/ReactNativeHostConfig.js

@@ -522,10 +522,15 @@ export function requestPostPaintCallback(callback: (time: number) => void) {
   // noop
 }
 
-export function shouldSuspendCommit(type: Type, props: Props): boolean {
+export function maySuspendCommit(type: Type, props: Props): boolean {
   return false;
 }
 
+export function preloadInstance(type: Type, props: Props): boolean {
+  // Return true to indicate it's already loaded
+  return true;
+}
+
 export function startSuspendingCommit(): void {}
 
 export function suspendInstance(type: Type, props: Props): void {}

packages/react-noop-renderer/src/createReactNoop.js

@@ -312,7 +312,9 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       if (record === undefined) {
         throw new Error('Could not find record for key.');
       }
-      if (record.status === 'pending') {
+      if (record.status === 'fulfilled') {
+        // Already loaded.
+      } else if (record.status === 'pending') {
         if (suspenseyCommitSubscription === null) {
           suspenseyCommitSubscription = {
             pendingCount: 1,
@@ -321,20 +323,19 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
         } else {
           suspenseyCommitSubscription.pendingCount++;
         }
+        // Stash the subscription on the record. In `resolveSuspenseyThing`,
+        // we'll use this fire the commit once all the things have loaded.
+        if (record.subscriptions === null) {
+          record.subscriptions = [];
+        }
+        record.subscriptions.push(suspenseyCommitSubscription);
       }
-      // Stash the subscription on the record. In `resolveSuspenseyThing`,
-      // we'll use this fire the commit once all the things have loaded.
-      if (record.subscriptions === null) {
-        record.subscriptions = [];
-      }
-      record.subscriptions.push(suspenseyCommitSubscription);
     } else {
       throw new Error(
         'Did not expect this host component to be visited when suspending ' +
           'the commit. Did you check the SuspendCommit flag?',
       );
     }
-    return suspenseyCommitSubscription;
   }
 
   function waitForCommitToBeReady():
@@ -569,38 +570,42 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       callback(endTime);
     },
 
-    shouldSuspendCommit(type: string, props: Props): boolean {
-      if (type === 'suspensey-thing' && typeof props.src === 'string') {
-        if (suspenseyThingCache === null) {
-          suspenseyThingCache = new Map();
-        }
-        const record = suspenseyThingCache.get(props.src);
-        if (record === undefined) {
-          const newRecord: SuspenseyThingRecord = {
-            status: 'pending',
-            subscriptions: null,
-          };
-          suspenseyThingCache.set(props.src, newRecord);
-          const onLoadStart = props.onLoadStart;
-          if (typeof onLoadStart === 'function') {
-            onLoadStart();
-          }
-          return props.src;
-        } else {
-          if (record.status === 'pending') {
-            // The resource was already requested, but it hasn't finished
-            // loading yet.
-            return true;
-          } else {
-            // The resource has already loaded. If the renderer is confident that
-            // the resource will still be cached by the time the render commits,
-            // then it can return false, like we do here.
-            return false;
-          }
+    maySuspendCommit(type: string, props: Props): boolean {
+      // Asks whether it's possible for this combination of type and props
+      // to ever need to suspend. This is different from asking whether it's
+      // currently ready because even if it's ready now, it might get purged
+      // from the cache later.
+      return type === 'suspensey-thing' && typeof props.src === 'string';
+    },
+
+    preloadInstance(type: string, props: Props): boolean {
+      if (type !== 'suspensey-thing' || typeof props.src !== 'string') {
+        throw new Error('Attempted to preload unexpected instance: ' + type);
+      }
+
+      // In addition to preloading an instance, this method asks whether the
+      // instance is ready to be committed. If it's not, React may yield to the
+      // main thread and ask again. It's possible a load event will fire in
+      // between, in which case we can avoid showing a fallback.
+      if (suspenseyThingCache === null) {
+        suspenseyThingCache = new Map();
+      }
+      const record = suspenseyThingCache.get(props.src);
+      if (record === undefined) {
+        const newRecord: SuspenseyThingRecord = {
+          status: 'pending',
+          subscriptions: null,
+        };
+        suspenseyThingCache.set(props.src, newRecord);
+        const onLoadStart = props.onLoadStart;
+        if (typeof onLoadStart === 'function') {
+          onLoadStart();
         }
+        return false;
+      } else {
+        // If this is false, React will trigger a fallback, if needed.
+        return record.status === 'fulfilled';
       }
-      // Don't need to suspend.
-      return false;
     },
 
     startSuspendingCommit,

packages/react-reconciler/src/ReactFiberCompleteWork.js

@@ -111,7 +111,8 @@ import {
   finalizeContainerChildren,
   preparePortalMount,
   prepareScopeUpdate,
-  shouldSuspendCommit,
+  maySuspendCommit,
+  preloadInstance,
 } from './ReactFiberHostConfig';
 import {
   getRootHostContainer,
@@ -434,8 +435,6 @@ function updateHostComponent(
     // Even better would be if children weren't special cased at all tho.
     const instance: Instance = workInProgress.stateNode;
 
-    suspendHostCommitIfNeeded(workInProgress, type, newProps, renderLanes);
-
     const currentHostContext = getHostContext();
     // TODO: Experiencing an error where oldProps is null. Suggests a host
     // component is hitting the resume path. Figure out why. Possibly
@@ -495,8 +494,6 @@ function updateHostComponent(
       recyclableInstance,
     );
 
-    suspendHostCommitIfNeeded(workInProgress, type, newProps, renderLanes);
-
     if (
       finalizeInitialChildren(newInstance, type, newProps, currentHostContext)
     ) {
@@ -519,17 +516,17 @@ function updateHostComponent(
 // not created until the complete phase. For our existing use cases, host nodes
 // that suspend don't have children, so it doesn't matter. But that might not
 // always be true in the future.
-function suspendHostCommitIfNeeded(
+function preloadInstanceAndSuspendIfNeeded(
   workInProgress: Fiber,
   type: Type,
   props: Props,
   renderLanes: Lanes,
 ) {
   // Ask the renderer if this instance should suspend the commit.
-  if (!shouldSuspendCommit(type, props)) {
+  if (!maySuspendCommit(type, props)) {
     // If this flag was set previously, we can remove it. The flag represents
     // whether this particular set of props might ever need to suspend. The
-    // safest thing to do is for shouldSuspendCommit to always return true, but
+    // safest thing to do is for maySuspendCommit to always return true, but
     // if the renderer is reasonably confident that the underlying resource
     // won't be evicted, it can return false as a performance optimization.
     workInProgress.flags &= ~SuspenseyCommit;
@@ -552,16 +549,24 @@ function suspendHostCommitIfNeeded(
   // TODO: We may decide to expose a way to force a fallback even during a
   // sync update.
   if (!includesOnlyNonUrgentLanes(renderLanes)) {
-    // This is an urgent render. Never suspend or trigger a fallback.
+    // This is an urgent render. Don't suspend or show a fallback. Also,
+    // there's no need to preload, because we're going to commit this
+    // synchronously anyway.
+    // TODO: Could there be benefit to preloading even during a synchronous
+    // render? The main thread will be blocked until the commit phase, but
+    // maybe the browser would be able to start loading off thread anyway?
+    // Likely a micro-optimization either way because typically new content
+    // is loaded during a transition, not an urgent render.
   } else {
-    // Need to decide whether to activate the nearest fallback or to continue
-    // rendering and suspend right before the commit phase.
-    if (shouldRemainOnPreviousScreen()) {
-      // It's OK to block the commit. Don't show a fallback.
-    } else {
-      // We shouldn't block the commit. Activate a fallback at the nearest
-      // Suspense boundary.
-      suspendCommit();
+    // Preload the instance
+    const isReady = preloadInstance(type, props);
+    if (!isReady) {
+      if (shouldRemainOnPreviousScreen()) {
+        // It's OK to suspend. Continue rendering.
+      } else {
+        // Trigger a fallback rather than block the render.
+        suspendCommit();
+      }
     }
   }
 }
@@ -1054,6 +1059,17 @@ function completeWork(
           );
         }
         bubbleProperties(workInProgress);
+
+        // This must come at the very end of the complete phase, because it might
+        // throw to suspend, and if the resource immediately loads, the work loop
+        // will resume rendering as if the work-in-progress completed. So it must
+        // fully complete.
+        preloadInstanceAndSuspendIfNeeded(
+          workInProgress,
+          workInProgress.type,
+          workInProgress.pendingProps,
+          renderLanes,
+        );
         return null;
       }
     }
@@ -1192,14 +1208,23 @@ function completeWork(
           }
         }
 
-        suspendHostCommitIfNeeded(workInProgress, type, newProps, renderLanes);
-
         if (workInProgress.ref !== null) {
           // If there is a ref on a host node we need to schedule a callback
           markRef(workInProgress);
         }
       }
       bubbleProperties(workInProgress);
+
+      // This must come at the very end of the complete phase, because it might
+      // throw to suspend, and if the resource immediately loads, the work loop
+      // will resume rendering as if the work-in-progress completed. So it must
+      // fully complete.
+      preloadInstanceAndSuspendIfNeeded(
+        workInProgress,
+        type,
+        newProps,
+        renderLanes,
+      );
       return null;
     }
     case HostText: {

packages/react-reconciler/src/ReactFiberThenable.js

@@ -31,6 +31,11 @@ export const SuspenseException: mixed = new Error(
     "call the promise's `.catch` method and pass the result to `use`",
 );
 
+export const SuspenseyCommitException: mixed = new Error(
+  'Suspense Exception: This is not a real error, and should not leak into ' +
+    "userspace. If you're seeing this, it's likely a bug in React.",
+);
+
 // This is a noop thenable that we use to trigger a fallback in throwException.
 // TODO: It would be better to refactor throwException into multiple functions
 // so we can trigger a fallback directly without having to check the type. But
@@ -151,7 +156,7 @@ export function suspendCommit(): void {
   // noopSuspenseyCommitThenable through to throwException.
   // TODO: Factor the thenable check out of throwException
   suspendedThenable = noopSuspenseyCommitThenable;
-  throw SuspenseException;
+  throw SuspenseyCommitException;
 }
 
 // This is used to track the actual thenable that suspended so it can be