Throw opaque object to suspend

The old (unstable) mechanism for suspending was to throw a promise. The
purpose of throwing is to interrupt the component's execution, and also
to signal to React that the interruption was caused by Suspense as
opposed to some other error.

A flaw is that throwing is meant to be an implementation details — if
code in userspace catches the promise, it can lead to
unexpected behavior.

With `use`, userspace code does not throw promises directly, but `use`
itself still needs to throw something to interrupt the component and
unwind the stack.

The solution is to throw an opaque object. In development, we can detect
whether the object was caught by a userspace try/catch block and log a
warning — though it's not foolproof, since a clever user could catch the
object and rethrow it later.

I did not yet implement the warning in Flight.

Author: acdlite

packages/react-reconciler/src/ReactFiberHooks.new.js

@@ -137,6 +137,7 @@ import {now} from './Scheduler';
 import {
   prepareThenableState,
   trackUsedThenable,
+  checkIfUseWrappedInTryCatch,
 } from './ReactFiberThenable.new';
 
 const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
@@ -159,8 +160,10 @@ export type UpdateQueue<S, A> = {
 
 let didWarnAboutMismatchedHooksForComponent;
 let didWarnUncachedGetSnapshot;
+let didWarnAboutUseWrappedInTryCatch;
 if (__DEV__) {
   didWarnAboutMismatchedHooksForComponent = new Set();
+  didWarnAboutUseWrappedInTryCatch = new Set();
 }
 
 export type Hook = {
@@ -593,6 +596,22 @@ export function renderWithHooks<Props, SecondArg>(
       }
     }
   }
+
+  if (__DEV__) {
+    if (checkIfUseWrappedInTryCatch()) {
+      const componentName =
+        getComponentNameFromFiber(workInProgress) || 'Unknown';
+      if (!didWarnAboutUseWrappedInTryCatch.has(componentName)) {
+        didWarnAboutUseWrappedInTryCatch.add(componentName);
+        console.error(
+          '`use` was called from inside a try/catch block. This is not allowed ' +
+            'and can lead to unexpected behavior. To handle errors triggered ' +
+            'by `use`, wrap your component in a error boundary.',
+        );
+      }
+    }
+  }
+
   return children;
 }
 

packages/react-reconciler/src/ReactFiberHooks.old.js

@@ -137,6 +137,7 @@ import {now} from './Scheduler';
 import {
   prepareThenableState,
   trackUsedThenable,
+  checkIfUseWrappedInTryCatch,
 } from './ReactFiberThenable.old';
 
 const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
@@ -159,8 +160,10 @@ export type UpdateQueue<S, A> = {
 
 let didWarnAboutMismatchedHooksForComponent;
 let didWarnUncachedGetSnapshot;
+let didWarnAboutUseWrappedInTryCatch;
 if (__DEV__) {
   didWarnAboutMismatchedHooksForComponent = new Set();
+  didWarnAboutUseWrappedInTryCatch = new Set();
 }
 
 export type Hook = {
@@ -593,6 +596,22 @@ export function renderWithHooks<Props, SecondArg>(
       }
     }
   }
+
+  if (__DEV__) {
+    if (checkIfUseWrappedInTryCatch()) {
+      const componentName =
+        getComponentNameFromFiber(workInProgress) || 'Unknown';
+      if (!didWarnAboutUseWrappedInTryCatch.has(componentName)) {
+        didWarnAboutUseWrappedInTryCatch.add(componentName);
+        console.error(
+          '`use` was called from inside a try/catch block. This is not allowed ' +
+            'and can lead to unexpected behavior. To handle errors triggered ' +
+            'by `use`, wrap your component in a error boundary.',
+        );
+      }
+    }
+  }
+
   return children;
 }
 

packages/react-reconciler/src/ReactFiberThenable.new.js

@@ -19,6 +19,15 @@ const {ReactCurrentActQueue} = ReactSharedInternals;
 
 export opaque type ThenableState = Array<Thenable<any>>;
 
+// An opaque object that is thrown (e.g. by `use`) to trigger Suspense. If we
+// detect this is caught by userspace, we'll log a warning in development.
+// TODO: Alternatively, this could be an actual error with a helpful message.
+// The reason not to do that is because someone might be tempted to check the
+// message in order to determine if it's caused by Suspense. Maybe it could be
+// an actual error object in production, but a plain object in development,
+// where it would also be accompanied by a warning.
+export const SuspenseException: mixed = {};
+
 let thenableState: ThenableState | null = null;
 
 export function createThenableState(): ThenableState {
@@ -37,19 +46,9 @@ export function prepareThenableState(prevThenableState: ThenableState | null) {
 export function getThenableStateAfterSuspending(): ThenableState | null {
   // Called by the work loop so it can stash the thenable state. It will use
   // the state to replay the component when the promise resolves.
-  if (
-    thenableState !== null &&
-    // If we only `use`-ed resolved promises, then there is no suspended state
-    // TODO: The only reason we do this is to distinguish between throwing a
-    // promise (old Suspense pattern) versus `use`-ing one. A better solution is
-    // for `use` to throw a special, opaque value instead of a promise.
-    !isThenableStateResolved(thenableState)
-  ) {
-    const state = thenableState;
-    thenableState = null;
-    return state;
-  }
-  return null;
+  const state = thenableState;
+  thenableState = null;
+  return state;
 }
 
 export function isThenableStateResolved(thenables: ThenableState): boolean {
@@ -129,13 +128,54 @@ export function trackUsedThenable<T>(thenable: Thenable<T>, index: number): T {
       }
 
       // Suspend.
-      // TODO: Throwing here is an implementation detail that allows us to
-      // unwind the call stack. But we shouldn't allow it to leak into
-      // userspace. Throw an opaque placeholder value instead of the
-      // actual thenable. If it doesn't get captured by the work loop, log
-      // a warning, because that means something in userspace must have
-      // caught it.
-      throw thenable;
+      //
+      // Throwing here is an implementation detail that allows us to unwind the
+      // call stack. But we shouldn't allow it to leak into userspace. Throw an
+      // opaque placeholder value instead of the actual thenable. If it doesn't
+      // get captured by the work loop, log a warning, because that means
+      // something in userspace must have caught it.
+      suspendedThenable = thenable;
+      if (__DEV__) {
+        needsToResetSuspendedThenableDEV = true;
+      }
+      throw SuspenseException;
+    }
+  }
+}
+
+// This is used to track the actual thenable that suspended so it can be
+// passed to the rest of the Suspense implementation — which, for historical
+// reasons, expects to receive a thenable.
+let suspendedThenable: Thenable<any> | null = null;
+let needsToResetSuspendedThenableDEV = false;
+export function getSuspendedThenable(): Thenable<mixed> {
+  // This is called right after `use` suspends by throwing an exception. `use`
+  // throws an opaque value instead of the thenable itself so that it can't be
+  // caught in userspace. Then the work loop accesses the actual thenable using
+  // this function.
+  if (suspendedThenable === null) {
+    throw new Error(
+      'Expected a suspended thenable. This is a bug in React. Please file ' +
+        'an issue.',
+    );
+  }
+  const thenable = suspendedThenable;
+  suspendedThenable = null;
+  if (__DEV__) {
+    needsToResetSuspendedThenableDEV = false;
+  }
+  return thenable;
+}
+
+export function checkIfUseWrappedInTryCatch(): boolean {
+  if (__DEV__) {
+    // This was set right before SuspenseException was thrown, and it should
+    // have been cleared when the exception was handled. If it wasn't,
+    // it must have been caught by userspace.
+    if (needsToResetSuspendedThenableDEV) {
+      needsToResetSuspendedThenableDEV = false;
+      return true;
     }
   }
+  return false;
 }

packages/react-reconciler/src/ReactFiberThenable.old.js

@@ -19,6 +19,15 @@ const {ReactCurrentActQueue} = ReactSharedInternals;
 
 export opaque type ThenableState = Array<Thenable<any>>;
 
+// An opaque object that is thrown (e.g. by `use`) to trigger Suspense. If we
+// detect this is caught by userspace, we'll log a warning in development.
+// TODO: Alternatively, this could be an actual error with a helpful message.
+// The reason not to do that is because someone might be tempted to check the
+// message in order to determine if it's caused by Suspense. Maybe it could be
+// an actual error object in production, but a plain object in development,
+// where it would also be accompanied by a warning.
+export const SuspenseException: mixed = {};
+
 let thenableState: ThenableState | null = null;
 
 export function createThenableState(): ThenableState {
@@ -37,19 +46,9 @@ export function prepareThenableState(prevThenableState: ThenableState | null) {
 export function getThenableStateAfterSuspending(): ThenableState | null {
   // Called by the work loop so it can stash the thenable state. It will use
   // the state to replay the component when the promise resolves.
-  if (
-    thenableState !== null &&
-    // If we only `use`-ed resolved promises, then there is no suspended state
-    // TODO: The only reason we do this is to distinguish between throwing a
-    // promise (old Suspense pattern) versus `use`-ing one. A better solution is
-    // for `use` to throw a special, opaque value instead of a promise.
-    !isThenableStateResolved(thenableState)
-  ) {
-    const state = thenableState;
-    thenableState = null;
-    return state;
-  }
-  return null;
+  const state = thenableState;
+  thenableState = null;
+  return state;
 }
 
 export function isThenableStateResolved(thenables: ThenableState): boolean {
@@ -129,13 +128,54 @@ export function trackUsedThenable<T>(thenable: Thenable<T>, index: number): T {
       }
 
       // Suspend.
-      // TODO: Throwing here is an implementation detail that allows us to
-      // unwind the call stack. But we shouldn't allow it to leak into
-      // userspace. Throw an opaque placeholder value instead of the
-      // actual thenable. If it doesn't get captured by the work loop, log
-      // a warning, because that means something in userspace must have
-      // caught it.
-      throw thenable;
+      //
+      // Throwing here is an implementation detail that allows us to unwind the
+      // call stack. But we shouldn't allow it to leak into userspace. Throw an
+      // opaque placeholder value instead of the actual thenable. If it doesn't
+      // get captured by the work loop, log a warning, because that means
+      // something in userspace must have caught it.
+      suspendedThenable = thenable;
+      if (__DEV__) {
+        needsToResetSuspendedThenableDEV = true;
+      }
+      throw SuspenseException;
+    }
+  }
+}
+
+// This is used to track the actual thenable that suspended so it can be
+// passed to the rest of the Suspense implementation — which, for historical
+// reasons, expects to receive a thenable.
+let suspendedThenable: Thenable<any> | null = null;
+let needsToResetSuspendedThenableDEV = false;
+export function getSuspendedThenable(): Thenable<mixed> {
+  // This is called right after `use` suspends by throwing an exception. `use`
+  // throws an opaque value instead of the thenable itself so that it can't be
+  // caught in userspace. Then the work loop accesses the actual thenable using
+  // this function.
+  if (suspendedThenable === null) {
+    throw new Error(
+      'Expected a suspended thenable. This is a bug in React. Please file ' +
+        'an issue.',
+    );
+  }
+  const thenable = suspendedThenable;
+  suspendedThenable = null;
+  if (__DEV__) {
+    needsToResetSuspendedThenableDEV = false;
+  }
+  return thenable;
+}
+
+export function checkIfUseWrappedInTryCatch(): boolean {
+  if (__DEV__) {
+    // This was set right before SuspenseException was thrown, and it should
+    // have been cleared when the exception was handled. If it wasn't,
+    // it must have been caught by userspace.
+    if (needsToResetSuspendedThenableDEV) {
+      needsToResetSuspendedThenableDEV = false;
+      return true;
     }
   }
+  return false;
 }

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

@@ -266,6 +266,8 @@ import {
 } from './ReactFiberAct.new';
 import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.new';
 import {
+  SuspenseException,
+  getSuspendedThenable,
   getThenableStateAfterSuspending,
   isThenableStateResolved,
 } from './ReactFiberThenable.new';
@@ -1722,13 +1724,26 @@ function handleThrow(root, thrownValue): void {
   // separate issue. Write a regression test using string refs.
   ReactCurrentOwner.current = null;
 
+  if (thrownValue === SuspenseException) {
+    // This is a special type of exception used for Suspense. For historical
+    // reasons, the rest of the Suspense implementation expects the thrown value
+    // to be a thenable, because before `use` existed that was the (unstable)
+    // API for suspending. This implementation detail can change later, once we
+    // deprecate the old API in favor of `use`.
+    thrownValue = getSuspendedThenable();
+    workInProgressSuspendedThenableState = getThenableStateAfterSuspending();
+  } else {
+    // This is a regular error. If something earlier in the component already
+    // suspended, we must clear the thenable state to unblock the work loop.
+    workInProgressSuspendedThenableState = null;
+  }
+
   // Setting this to `true` tells the work loop to unwind the stack instead
   // of entering the begin phase. It's called "suspended" because it usually
   // happens because of Suspense, but it also applies to errors. Think of it
   // as suspending the execution of the work loop.
   workInProgressIsSuspended = true;
   workInProgressThrownValue = thrownValue;
-  workInProgressSuspendedThenableState = getThenableStateAfterSuspending();
 
   const erroredWork = workInProgress;
   if (erroredWork === null) {
@@ -1750,6 +1765,7 @@ function handleThrow(root, thrownValue): void {
     if (
       thrownValue !== null &&
       typeof thrownValue === 'object' &&
+      // $FlowFixMe[method-unbinding]
       typeof thrownValue.then === 'function'
     ) {
       const wakeable: Wakeable = (thrownValue: any);
@@ -3503,6 +3519,7 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
     } catch (originalError) {
       if (
         didSuspendOrErrorWhileHydratingDEV() ||
+        originalError === SuspenseException ||
         (originalError !== null &&
           typeof originalError === 'object' &&
           typeof originalError.then === 'function')

packages/react-reconciler/src/ReactFiberWorkLoop.old.js

@@ -266,6 +266,8 @@ import {
 } from './ReactFiberAct.old';
 import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.old';
 import {
+  SuspenseException,
+  getSuspendedThenable,
   getThenableStateAfterSuspending,
   isThenableStateResolved,
 } from './ReactFiberThenable.old';
@@ -1722,13 +1724,26 @@ function handleThrow(root, thrownValue): void {
   // separate issue. Write a regression test using string refs.
   ReactCurrentOwner.current = null;
 
+  if (thrownValue === SuspenseException) {
+    // This is a special type of exception used for Suspense. For historical
+    // reasons, the rest of the Suspense implementation expects the thrown value
+    // to be a thenable, because before `use` existed that was the (unstable)
+    // API for suspending. This implementation detail can change later, once we
+    // deprecate the old API in favor of `use`.
+    thrownValue = getSuspendedThenable();
+    workInProgressSuspendedThenableState = getThenableStateAfterSuspending();
+  } else {
+    // This is a regular error. If something earlier in the component already
+    // suspended, we must clear the thenable state to unblock the work loop.
+    workInProgressSuspendedThenableState = null;
+  }
+
   // Setting this to `true` tells the work loop to unwind the stack instead
   // of entering the begin phase. It's called "suspended" because it usually
   // happens because of Suspense, but it also applies to errors. Think of it
   // as suspending the execution of the work loop.
   workInProgressIsSuspended = true;
   workInProgressThrownValue = thrownValue;
-  workInProgressSuspendedThenableState = getThenableStateAfterSuspending();
 
   const erroredWork = workInProgress;
   if (erroredWork === null) {
@@ -1750,6 +1765,7 @@ function handleThrow(root, thrownValue): void {
     if (
       thrownValue !== null &&
       typeof thrownValue === 'object' &&
+      // $FlowFixMe[method-unbinding]
       typeof thrownValue.then === 'function'
     ) {
       const wakeable: Wakeable = (thrownValue: any);
@@ -3503,6 +3519,7 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
     } catch (originalError) {
       if (
         didSuspendOrErrorWhileHydratingDEV() ||
+        originalError === SuspenseException ||
         (originalError !== null &&
           typeof originalError === 'object' &&
           typeof originalError.then === 'function')