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

This reverts commit 0131d0c.

Author: kassens

packages/react-art/src/ReactARTHostConfig.js

@@ -459,15 +459,10 @@ export function requestPostPaintCallback(callback: (time: number) => void) {
   // noop
 }
 
-export function maySuspendCommit(type, props) {
+export function shouldSuspendCommit(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,15 +1609,10 @@ export function requestPostPaintCallback(callback: (time: number) => void) {
   });
 }
 
-export function maySuspendCommit(type: Type, props: Props): boolean {
+export function shouldSuspendCommit(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,14 +421,10 @@ export function requestPostPaintCallback(callback: (time: number) => void) {
   // noop
 }
 
-export function maySuspendCommit(type: Type, props: Props): boolean {
+export function shouldSuspendCommit(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,15 +522,10 @@ export function requestPostPaintCallback(callback: (time: number) => void) {
   // noop
 }
 
-export function maySuspendCommit(type: Type, props: Props): boolean {
+export function shouldSuspendCommit(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,9 +312,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       if (record === undefined) {
         throw new Error('Could not find record for key.');
       }
-      if (record.status === 'fulfilled') {
-        // Already loaded.
-      } else if (record.status === 'pending') {
+      if (record.status === 'pending') {
         if (suspenseyCommitSubscription === null) {
           suspenseyCommitSubscription = {
             pendingCount: 1,
@@ -323,19 +321,20 @@ 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():
@@ -570,42 +569,38 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       callback(endTime);
     },
 
-    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();
+    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;
+          }
         }
-        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,8 +111,7 @@ import {
   finalizeContainerChildren,
   preparePortalMount,
   prepareScopeUpdate,
-  maySuspendCommit,
-  preloadInstance,
+  shouldSuspendCommit,
 } from './ReactFiberHostConfig';
 import {
   getRootHostContainer,
@@ -435,6 +434,8 @@ 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
@@ -494,6 +495,8 @@ function updateHostComponent(
       recyclableInstance,
     );
 
+    suspendHostCommitIfNeeded(workInProgress, type, newProps, renderLanes);
+
     if (
       finalizeInitialChildren(newInstance, type, newProps, currentHostContext)
     ) {
@@ -516,17 +519,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 preloadInstanceAndSuspendIfNeeded(
+function suspendHostCommitIfNeeded(
   workInProgress: Fiber,
   type: Type,
   props: Props,
   renderLanes: Lanes,
 ) {
   // Ask the renderer if this instance should suspend the commit.
-  if (!maySuspendCommit(type, props)) {
+  if (!shouldSuspendCommit(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 maySuspendCommit to always return true, but
+    // safest thing to do is for shouldSuspendCommit 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;
@@ -549,24 +552,16 @@ function preloadInstanceAndSuspendIfNeeded(
   // 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. 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.
+    // This is an urgent render. Never suspend or trigger a fallback.
   } else {
-    // 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();
-      }
+    // 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();
     }
   }
 }
@@ -1059,17 +1054,6 @@ 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;
       }
     }
@@ -1208,23 +1192,14 @@ 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,11 +31,6 @@ 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
@@ -156,7 +151,7 @@ export function suspendCommit(): void {
   // noopSuspenseyCommitThenable through to throwException.
   // TODO: Factor the thenable check out of throwException
   suspendedThenable = noopSuspenseyCommitThenable;
-  throw SuspenseyCommitException;
+  throw SuspenseException;
 }
 
 // This is used to track the actual thenable that suspended so it can be