Update on "[compiler] Expect components to have hook calls or jsx dir…

…ectly in body"

Summary: We can tighten our criteria for what is a component by requiring that a component or hook contain JSX or hook calls directly within its body, excluding nested functions . Currently, if we see them within the body anywhere -- including nested functions -- we treat it as a component if the other requirements are met. This change makes this stricter.

We also now expect components (but not necessarily hooks) to have return statements, and those returns must be potential React nodes (we can reject functions that return function or object literals, for example).

[ghstack-poisoned]

packages/react-client/src/ReactFlightClient.js

@@ -1136,46 +1136,65 @@ function missingCall() {
   );
 }
 
-export function createResponse(
+function ResponseInstance(
+  this: $FlowFixMe,
   bundlerConfig: SSRModuleMap,
   moduleLoading: ModuleLoading,
   callServer: void | CallServerCallback,
   encodeFormAction: void | EncodeFormActionCallback,
   nonce: void | string,
   temporaryReferences: void | TemporaryReferenceSet,
   findSourceMapURL: void | FindSourceMapURLCallback,
-): Response {
+) {
   const chunks: Map<number, SomeChunk<any>> = new Map();
-  const response: Response = {
-    _bundlerConfig: bundlerConfig,
-    _moduleLoading: moduleLoading,
-    _callServer: callServer !== undefined ? callServer : missingCall,
-    _encodeFormAction: encodeFormAction,
-    _nonce: nonce,
-    _chunks: chunks,
-    _stringDecoder: createStringDecoder(),
-    _fromJSON: (null: any),
-    _rowState: 0,
-    _rowID: 0,
-    _rowTag: 0,
-    _rowLength: 0,
-    _buffer: [],
-    _tempRefs: temporaryReferences,
-  };
+  this._bundlerConfig = bundlerConfig;
+  this._moduleLoading = moduleLoading;
+  this._callServer = callServer !== undefined ? callServer : missingCall;
+  this._encodeFormAction = encodeFormAction;
+  this._nonce = nonce;
+  this._chunks = chunks;
+  this._stringDecoder = createStringDecoder();
+  this._fromJSON = (null: any);
+  this._rowState = 0;
+  this._rowID = 0;
+  this._rowTag = 0;
+  this._rowLength = 0;
+  this._buffer = [];
+  this._tempRefs = temporaryReferences;
   if (supportsCreateTask) {
     // Any stacks that appear on the server need to be rooted somehow on the client
     // so we create a root Task for this response which will be the root owner for any
     // elements created by the server. We use the "use server" string to indicate that
     // this is where we enter the server from the client.
     // TODO: Make this string configurable.
-    response._debugRootTask = (console: any).createTask('"use server"');
+    this._debugRootTask = (console: any).createTask('"use server"');
   }
   if (__DEV__) {
-    response._debugFindSourceMapURL = findSourceMapURL;
+    this._debugFindSourceMapURL = findSourceMapURL;
   }
   // Don't inline this call because it causes closure to outline the call above.
-  response._fromJSON = createFromJSONCallback(response);
-  return response;
+  this._fromJSON = createFromJSONCallback(this);
+}
+
+export function createResponse(
+  bundlerConfig: SSRModuleMap,
+  moduleLoading: ModuleLoading,
+  callServer: void | CallServerCallback,
+  encodeFormAction: void | EncodeFormActionCallback,
+  nonce: void | string,
+  temporaryReferences: void | TemporaryReferenceSet,
+  findSourceMapURL: void | FindSourceMapURLCallback,
+): Response {
+  // $FlowFixMe[invalid-constructor]: the shapes are exact here but Flow doesn't like constructors
+  return new ResponseInstance(
+    bundlerConfig,
+    moduleLoading,
+    callServer,
+    encodeFormAction,
+    nonce,
+    temporaryReferences,
+    findSourceMapURL,
+  );
 }
 
 function resolveModel(

packages/react-client/src/__tests__/ReactFlight-test.js

@@ -964,67 +964,47 @@ describe('ReactFlight', () => {
     const testCases = (
       <>
         <ClientErrorBoundary expectedMessage="This is a real Error.">
-          <div>
-            <Throw value={new TypeError('This is a real Error.')} />
-          </div>
+          <Throw value={new TypeError('This is a real Error.')} />
         </ClientErrorBoundary>
         <ClientErrorBoundary expectedMessage="This is a string error.">
-          <div>
-            <Throw value="This is a string error." />
-          </div>
+          <Throw value="This is a string error." />
         </ClientErrorBoundary>
         <ClientErrorBoundary expectedMessage="{message: ..., extra: ..., nested: ...}">
-          <div>
-            <Throw
-              value={{
-                message: 'This is a long message',
-                extra: 'properties',
-                nested: {more: 'prop'},
-              }}
-            />
-          </div>
+          <Throw
+            value={{
+              message: 'This is a long message',
+              extra: 'properties',
+              nested: {more: 'prop'},
+            }}
+          />
         </ClientErrorBoundary>
         <ClientErrorBoundary
           expectedMessage={'{message: "Short", extra: ..., nested: ...}'}>
-          <div>
-            <Throw
-              value={{
-                message: 'Short',
-                extra: 'properties',
-                nested: {more: 'prop'},
-              }}
-            />
-          </div>
+          <Throw
+            value={{
+              message: 'Short',
+              extra: 'properties',
+              nested: {more: 'prop'},
+            }}
+          />
         </ClientErrorBoundary>
         <ClientErrorBoundary expectedMessage="Symbol(hello)">
-          <div>
-            <Throw value={Symbol('hello')} />
-          </div>
+          <Throw value={Symbol('hello')} />
         </ClientErrorBoundary>
         <ClientErrorBoundary expectedMessage="123">
-          <div>
-            <Throw value={123} />
-          </div>
+          <Throw value={123} />
         </ClientErrorBoundary>
         <ClientErrorBoundary expectedMessage="undefined">
-          <div>
-            <Throw value={undefined} />
-          </div>
+          <Throw value={undefined} />
         </ClientErrorBoundary>
         <ClientErrorBoundary expectedMessage="<div/>">
-          <div>
-            <Throw value={<div />} />
-          </div>
+          <Throw value={<div />} />
         </ClientErrorBoundary>
         <ClientErrorBoundary expectedMessage="function Foo() {}">
-          <div>
-            <Throw value={function Foo() {}} />
-          </div>
+          <Throw value={function Foo() {}} />
         </ClientErrorBoundary>
         <ClientErrorBoundary expectedMessage={'["array"]'}>
-          <div>
-            <Throw value={['array']} />
-          </div>
+          <Throw value={['array']} />
         </ClientErrorBoundary>
         <ClientErrorBoundary
           expectedMessage={
@@ -1034,9 +1014,7 @@ describe('ReactFlight', () => {
             '- A library pre-bundled an old copy of "react" or "react/jsx-runtime".\n' +
             '- A compiler tries to "inline" JSX instead of using the runtime.'
           }>
-          <div>
-            <LazyInlined />
-          </div>
+          <LazyInlined />
         </ClientErrorBoundary>
       </>
     );

packages/react-devtools-shared/src/backend/renderer.js

@@ -268,6 +268,7 @@ export function getInternalReactConstants(version: string): {
       TracingMarkerComponent: 25, // Experimental - This is technically in 18 but we don't
       // want to fork again so we're adding it here instead
       YieldComponent: -1, // Removed
+      Throw: 29,
     };
   } else if (gte(version, '17.0.0-alpha')) {
     ReactTypeOfWork = {
@@ -302,6 +303,7 @@ export function getInternalReactConstants(version: string): {
       SuspenseListComponent: 19, // Experimental
       TracingMarkerComponent: -1, // Doesn't exist yet
       YieldComponent: -1, // Removed
+      Throw: -1, // Doesn't exist yet
     };
   } else if (gte(version, '16.6.0-beta.0')) {
     ReactTypeOfWork = {
@@ -336,6 +338,7 @@ export function getInternalReactConstants(version: string): {
       SuspenseListComponent: 19, // Experimental
       TracingMarkerComponent: -1, // Doesn't exist yet
       YieldComponent: -1, // Removed
+      Throw: -1, // Doesn't exist yet
     };
   } else if (gte(version, '16.4.3-alpha')) {
     ReactTypeOfWork = {
@@ -370,6 +373,7 @@ export function getInternalReactConstants(version: string): {
       SuspenseListComponent: -1, // Doesn't exist yet
       TracingMarkerComponent: -1, // Doesn't exist yet
       YieldComponent: -1, // Removed
+      Throw: -1, // Doesn't exist yet
     };
   } else {
     ReactTypeOfWork = {
@@ -404,6 +408,7 @@ export function getInternalReactConstants(version: string): {
       SuspenseListComponent: -1, // Doesn't exist yet
       TracingMarkerComponent: -1, // Doesn't exist yet
       YieldComponent: 9,
+      Throw: -1, // Doesn't exist yet
     };
   }
   // **********************************************************
@@ -445,6 +450,7 @@ export function getInternalReactConstants(version: string): {
     SuspenseComponent,
     SuspenseListComponent,
     TracingMarkerComponent,
+    Throw,
   } = ReactTypeOfWork;
 
   function resolveFiberType(type: any): $FlowFixMe {
@@ -551,6 +557,9 @@ export function getInternalReactConstants(version: string): {
         return 'Profiler';
       case TracingMarkerComponent:
         return 'TracingMarker';
+      case Throw:
+        // This should really never be visible.
+        return 'Error';
       default:
         const typeSymbol = getTypeSymbol(type);
 
@@ -672,6 +681,7 @@ export function attach(
     SuspenseComponent,
     SuspenseListComponent,
     TracingMarkerComponent,
+    Throw,
   } = ReactTypeOfWork;
   const {
     ImmediatePriority,
@@ -1036,6 +1046,7 @@ export function attach(
       case HostText:
       case LegacyHiddenComponent:
       case OffscreenComponent:
+      case Throw:
         return true;
       case HostRoot:
         // It is never valid to filter the root element.

packages/react-devtools-shared/src/backend/types.js

@@ -72,6 +72,7 @@ export type WorkTagMap = {
   SuspenseListComponent: WorkTag,
   TracingMarkerComponent: WorkTag,
   YieldComponent: WorkTag,
+  Throw: WorkTag,
 };
 
 // TODO: If it's useful for the frontend to know which types of data an Element has

packages/react-reconciler/src/ReactChildFiber.js

@@ -25,6 +25,7 @@ import {
   Forked,
   PlacementDEV,
 } from './ReactFiberFlags';
+import {NoMode, ConcurrentMode} from './ReactTypeOfMode';
 import {
   getIteratorFn,
   ASYNC_ITERATOR,
@@ -46,6 +47,7 @@ import isArray from 'shared/isArray';
 import {
   enableRefAsProp,
   enableAsyncIterableChildren,
+  disableLegacyMode,
 } from 'shared/ReactFeatureFlags';
 
 import {
@@ -55,11 +57,16 @@ import {
   createFiberFromFragment,
   createFiberFromText,
   createFiberFromPortal,
+  createFiberFromThrow,
 } from './ReactFiber';
 import {isCompatibleFamilyForHotReloading} from './ReactFiberHotReloading';
 import {getIsHydrating} from './ReactFiberHydrationContext';
 import {pushTreeFork} from './ReactFiberTreeContext';
-import {createThenableState, trackUsedThenable} from './ReactFiberThenable';
+import {
+  SuspenseException,
+  createThenableState,
+  trackUsedThenable,
+} from './ReactFiberThenable';
 import {readContextDuringReconciliation} from './ReactFiberNewContext';
 import {callLazyInitInDEV} from './ReactFiberCallUserSpace';
 
@@ -1919,20 +1926,45 @@ function createChildReconciler(
     newChild: any,
     lanes: Lanes,
   ): Fiber | null {
-    // This indirection only exists so we can reset `thenableState` at the end.
-    // It should get inlined by Closure.
-    thenableIndexCounter = 0;
-    const firstChildFiber = reconcileChildFibersImpl(
-      returnFiber,
-      currentFirstChild,
-      newChild,
-      lanes,
-      null, // debugInfo
-    );
-    thenableState = null;
-    // Don't bother to reset `thenableIndexCounter` to 0 because it always gets
-    // set at the beginning.
-    return firstChildFiber;
+    try {
+      // This indirection only exists so we can reset `thenableState` at the end.
+      // It should get inlined by Closure.
+      thenableIndexCounter = 0;
+      const firstChildFiber = reconcileChildFibersImpl(
+        returnFiber,
+        currentFirstChild,
+        newChild,
+        lanes,
+        null, // debugInfo
+      );
+      thenableState = null;
+      // Don't bother to reset `thenableIndexCounter` to 0 because it always gets
+      // set at the beginning.
+      return firstChildFiber;
+    } catch (x) {
+      if (
+        x === SuspenseException ||
+        (!disableLegacyMode &&
+          (returnFiber.mode & ConcurrentMode) === NoMode &&
+          typeof x === 'object' &&
+          x !== null &&
+          typeof x.then === 'function')
+      ) {
+        // Suspense exceptions need to read the current suspended state before
+        // yielding and replay it using the same sequence so this trick doesn't
+        // work here.
+        // Suspending in legacy mode actually mounts so if we let the child
+        // mount then we delete its state in an update.
+        throw x;
+      }
+      // Something errored during reconciliation but it's conceptually a child that
+      // errored and not the current component itself so we create a virtual child
+      // that throws in its begin phase. That way the current component can handle
+      // the error or suspending if needed.
+      const throwFiber = createFiberFromThrow(x, returnFiber.mode, lanes);
+      throwFiber.return = returnFiber;
+      return throwFiber;
+    }
   }
 
   return reconcileChildFibers;

packages/react-reconciler/src/ReactFiber.js

@@ -67,6 +67,7 @@ import {
   OffscreenComponent,
   LegacyHiddenComponent,
   TracingMarkerComponent,
+  Throw,
 } from './ReactWorkTags';
 import {OffscreenVisible} from './ReactFiberActivityComponent';
 import {getComponentNameFromOwner} from 'react-reconciler/src/getComponentNameFromFiber';
@@ -879,3 +880,13 @@ export function createFiberFromPortal(
   };
   return fiber;
 }
+
+export function createFiberFromThrow(
+  error: mixed,
+  mode: TypeOfMode,
+  lanes: Lanes,
+): Fiber {
+  const fiber = createFiber(Throw, error, null, mode);
+  fiber.lanes = lanes;
+  return fiber;
+}