[FlightReply] Performance improvements when decoding (#37090)

Security Patches included in 19.2.8

Co-authored-by: Sebastian Sebbie Silbermann <[email protected]>

packages/react-server-dom-parcel/src/server/ReactFlightDOMServerEdge.js

@@ -21,6 +21,7 @@ import {
   type ServerReferenceId,
 } from '../client/ReactFlightClientConfigBundlerParcel';
 
+import noop from 'shared/noop';
 import {ASYNC_ITERATOR} from 'shared/ReactSymbols';
 
 import {
@@ -314,7 +315,7 @@ export function decodeReplyFromAsyncIterable<T>(
     if (typeof (iterator as any).throw === 'function') {
       // The iterator protocol doesn't necessarily include this but a generator do.
       // $FlowFixMe[prop-missing] should be able to pass mixed
-      iterator.throw(reason).then(error, error);
+      iterator.throw(reason).then(noop, noop);
     }
   }
 

packages/react-server-dom-parcel/src/server/ReactFlightDOMServerNode.js

@@ -24,6 +24,7 @@ import type {Duplex} from 'stream';
 
 import {Readable} from 'stream';
 
+import noop from 'shared/noop';
 import {ASYNC_ITERATOR} from 'shared/ReactSymbols';
 
 import {
@@ -772,7 +773,7 @@ export function decodeReplyFromAsyncIterable<T>(
     if (typeof (iterator as any).throw === 'function') {
       // The iterator protocol doesn't necessarily include this but a generator do.
       // $FlowFixMe[prop-missing] should be able to pass mixed
-      iterator.throw(reason).then(error, error);
+      iterator.throw(reason).then(noop, noop);
     }
   }
 

packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js

@@ -15,6 +15,7 @@ import type {Thenable} from 'shared/ReactTypes';
 import type {ClientManifest} from './ReactFlightServerConfigTurbopackBundler';
 import type {ServerManifest} from 'react-client/src/ReactFlightClientConfig';
 
+import noop from 'shared/noop';
 import {ASYNC_ITERATOR} from 'shared/ReactSymbols';
 
 import {
@@ -313,7 +314,7 @@ function decodeReplyFromAsyncIterable<T>(
     if (typeof (iterator as any).throw === 'function') {
       // The iterator protocol doesn't necessarily include this but a generator do.
       // $FlowFixMe[prop-missing] should be able to pass mixed
-      iterator.throw(reason).then(error, error);
+      iterator.throw(reason).then(noop, noop);
     }
   }
 

packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js

@@ -22,6 +22,7 @@ import type {Duplex} from 'stream';
 
 import {Readable} from 'stream';
 
+import noop from 'shared/noop';
 import {ASYNC_ITERATOR} from 'shared/ReactSymbols';
 
 import {
@@ -766,7 +767,7 @@ function decodeReplyFromAsyncIterable<T>(
     if (typeof (iterator as any).throw === 'function') {
       // The iterator protocol doesn't necessarily include this but a generator do.
       // $FlowFixMe[prop-missing] should be able to pass mixed
-      iterator.throw(reason).then(error, error);
+      iterator.throw(reason).then(noop, noop);
     }
   }
 

packages/react-server-dom-unbundled/src/server/ReactFlightDOMServerNode.js

@@ -22,6 +22,7 @@ import type {Duplex} from 'stream';
 
 import {Readable} from 'stream';
 
+import noop from 'shared/noop';
 import {ASYNC_ITERATOR} from 'shared/ReactSymbols';
 
 import {
@@ -766,7 +767,7 @@ function decodeReplyFromAsyncIterable<T>(
     if (typeof (iterator as any).throw === 'function') {
       // The iterator protocol doesn't necessarily include this but a generator do.
       // $FlowFixMe[prop-missing] should be able to pass mixed
-      iterator.throw(reason).then(error, error);
+      iterator.throw(reason).then(noop, noop);
     }
   }
 

packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js

@@ -15,6 +15,7 @@ import type {Thenable} from 'shared/ReactTypes';
 import type {ClientManifest} from './ReactFlightServerConfigWebpackBundler';
 import type {ServerManifest} from 'react-client/src/ReactFlightClientConfig';
 
+import noop from 'shared/noop';
 import {ASYNC_ITERATOR} from 'shared/ReactSymbols';
 
 import {
@@ -313,7 +314,7 @@ function decodeReplyFromAsyncIterable<T>(
     if (typeof (iterator as any).throw === 'function') {
       // The iterator protocol doesn't necessarily include this but a generator do.
       // $FlowFixMe[prop-missing] should be able to pass mixed
-      iterator.throw(reason).then(error, error);
+      iterator.throw(reason).then(noop, noop);
     }
   }
 

packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js

@@ -22,6 +22,7 @@ import type {Duplex} from 'stream';
 
 import {Readable} from 'stream';
 
+import noop from 'shared/noop';
 import {ASYNC_ITERATOR} from 'shared/ReactSymbols';
 
 import {
@@ -767,7 +768,7 @@ function decodeReplyFromAsyncIterable<T>(
     if (typeof (iterator as any).throw === 'function') {
       // The iterator protocol doesn't necessarily include this but a generator do.
       // $FlowFixMe[prop-missing] should be able to pass mixed
-      iterator.throw(reason).then(error, error);
+      iterator.throw(reason).then(noop, noop);
     }
   }
 

packages/react-server/src/ReactFlightActionServer.js

@@ -112,53 +112,50 @@ export function decodeAction<T>(
   // the implementation details of the action data.
   const formData = new FormData();
 
-  let action: Promise<(formData: FormData) => T> | null = null;
-  const seenActions = new Set<string>();
+  let maybeActionKey: null | string = null;
 
   // $FlowFixMe[prop-missing]
   body.forEach((value: string | File, key: string) => {
     if (!key.startsWith('$ACTION_')) {
       // $FlowFixMe[incompatible-type]
       formData.append(key, value);
-      return;
-    }
-    // Later actions may override earlier actions if a button is used to
-    // override the default form action. However, we don't expect the same
-    // action ref field to be sent multiple times in legitimate form data.
-    if (key.startsWith('$ACTION_REF_')) {
-      if (seenActions.has(key)) {
-        return;
-      }
-      seenActions.add(key);
-      const formFieldPrefix = '$ACTION_' + key.slice(12) + ':';
-      const metaData = decodeBoundActionMetaData(
-        body,
-        serverManifest,
-        formFieldPrefix,
-      );
-      action = loadServerReference(serverManifest, metaData);
-      return;
-    }
-    // A simple action with no bound arguments may appear twice in the form data
-    // if a button specifies the same action as the default form action. We only
-    // load the first one, as they're guaranteed to be identical.
-    if (key.startsWith('$ACTION_ID_')) {
-      if (seenActions.has(key)) {
-        return;
-      }
-      seenActions.add(key);
-      const id = key.slice(11);
-      action = loadServerReference(serverManifest, {
-        id,
-        bound: null,
-      });
-      return;
+    } else if (key.startsWith('$ACTION_REF_')) {
+      // Later actions may override earlier actions if a button is used to
+      // override the default form action. However, we don't expect the same
+      // action ref field to be sent multiple times in legitimate form data.
+      maybeActionKey = key;
+    } else if (key.startsWith('$ACTION_ID_')) {
+      // A simple action with no bound arguments may appear twice in the form data
+      // if a button specifies the same action as the default form action.
+      maybeActionKey = key;
     }
   });
 
-  if (action === null) {
+  if (maybeActionKey === null) {
     return null;
   }
+  const actionKey = maybeActionKey;
+
+  let action: Promise<(formData: FormData) => T> | null = null;
+  if (actionKey.startsWith('$ACTION_REF_')) {
+    const formFieldPrefix =
+      '$ACTION_' + actionKey.slice('$ACTION_REF_'.length) + ':';
+    const metaData = decodeBoundActionMetaData(
+      body,
+      serverManifest,
+      formFieldPrefix,
+    );
+    action = loadServerReference(serverManifest, metaData);
+  } else if (actionKey.startsWith('$ACTION_ID_')) {
+    const id = actionKey.slice('$ACTION_ID_'.length);
+    action = loadServerReference(serverManifest, {
+      id,
+      bound: null,
+    });
+  } else {
+    throw new Error('Cannot handle action key. This is a bug in React.');
+  }
+
   // Return the action with the remaining FormData bound to the first argument.
   return action.then(fn => fn.bind(null, formData));
 }
@@ -175,24 +172,28 @@ export function decodeFormState<S>(
   }
   // Search through the form data object to get the reference id and the number
   // of bound arguments. This repeats some of the work done in decodeAction.
-  let metaData = null;
+  let actionKey: null | string = null;
   // $FlowFixMe[prop-missing]
   body.forEach((value: string | File, key: string) => {
     if (key.startsWith('$ACTION_REF_')) {
-      const formFieldPrefix = '$ACTION_' + key.slice(12) + ':';
-      metaData = decodeBoundActionMetaData(
-        body,
-        serverManifest,
-        formFieldPrefix,
-      );
+      actionKey = key;
     }
     // We don't check for the simple $ACTION_ID_ case because form state actions
     // are always bound to the state argument.
   });
-  if (metaData === null) {
+  if (actionKey === null) {
     // Should be unreachable.
     return Promise.resolve(null);
   }
+
+  const formFieldPrefix =
+    '$ACTION_' + actionKey.slice('$ACTION_REF_'.length) + ':';
+  const metaData = decodeBoundActionMetaData(
+    body,
+    serverManifest,
+    formFieldPrefix,
+  );
+
   const referenceId = metaData.id;
   return Promise.resolve(metaData.bound).then(bound => {
     if (bound === null) {

packages/react-server/src/ReactFlightServer.js

@@ -1404,7 +1404,7 @@ function serializeAsyncIterable(
     if (typeof (iterator as any).throw === 'function') {
       // The iterator protocol doesn't necessarily include this but a generator do.
       // $FlowFixMe[prop-missing] should be able to pass mixed
-      iterator.throw(reason).then(error, error);
+      iterator.throw(reason).then(noop, noop);
     }
   }
   function abortIterable() {
@@ -1424,9 +1424,11 @@ function serializeAsyncIterable(
       enqueueFlush(request);
     }
     if (typeof (iterator as any).throw === 'function') {
+      // TODO: Premature exits should call return() on the iterator if it exists
+      // to allow cleanup. See https://tc39.es/ecma262/multipage/control-abstraction-objects.html#table-async-iterator-optional
       // The iterator protocol doesn't necessarily include this but a generator do.
       // $FlowFixMe[prop-missing] should be able to pass mixed
-      iterator.throw(reason).then(error, error);
+      iterator.throw(reason).then(noop, noop);
     }
   }
   request.cacheController.signal.addEventListener('abort', abortIterable);

scripts/error-codes/codes.json

@@ -586,5 +586,6 @@
   "598": "Maximum update depth exceeded. This could be an infinite loop. This can happen when a component repeatedly calls setState during render phase or inside useLayoutEffect, causing infinite render loop. React limits the number of nested updates to prevent infinite loops.",
   "599": "Expected an initialized chunk but got an initialized stream chunk instead. This payload may have been submitted by an older version of React.",
   "600": "A rejected Promise was passed to React without a `reason` property. React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`.",
-  "601": "A chunk pair is incomplete. This is a bug in React."
+  "601": "A chunk pair is incomplete. This is a bug in React.",
+  "602": "Cannot handle action key. This is a bug in React."
 }