Validate key in Flight

Elements that render Server Components are validated inside Flight.
Others pass the validated flag to the client.

Author: sebmarkbage

packages/react-client/src/ReactFlightClient.js

@@ -565,6 +565,7 @@ function createElement(
   props: mixed,
   owner: null | ReactComponentInfo, // DEV-only
   stack: null | string, // DEV-only
+  validated: number, // DEV-only
 ): React$Element<any> {
   let element: any;
   if (__DEV__ && enableRefAsProp) {
@@ -616,7 +617,7 @@ function createElement(
       configurable: false,
       enumerable: false,
       writable: true,
-      value: 1, // This element has already been validated on the server.
+      value: enableOwnerStacks ? validated : 1, // Whether the element has already been validated on the server.
     });
     // debugInfo contains Server Component debug information.
     Object.defineProperty(element, '_debugInfo', {
@@ -630,7 +631,7 @@ function createElement(
         configurable: false,
         enumerable: false,
         writable: true,
-        value: {stack: stack},
+        value: stack,
       });
       Object.defineProperty(element, '_debugTask', {
         configurable: false,
@@ -1034,6 +1035,7 @@ function parseModelTuple(
       tuple[3],
       __DEV__ ? (tuple: any)[4] : null,
       __DEV__ && enableOwnerStacks ? (tuple: any)[5] : null,
+      __DEV__ && enableOwnerStacks ? (tuple: any)[6] : 0,
     );
   }
   return value;

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

@@ -1389,19 +1389,40 @@ describe('ReactFlight', () => {
     ReactNoopFlightClient.read(transport);
   });
 
-  it('should warn in DEV a child is missing keys', () => {
+  it('should warn in DEV a child is missing keys on server component', () => {
+    function NoKey({children}) {
+      return <div key="this has a key but parent doesn't" />;
+    }
+    expect(() => {
+      const transport = ReactNoopFlightServer.render(
+        <div>{Array(6).fill(<NoKey />)}</div>,
+      );
+      ReactNoopFlightClient.read(transport);
+    }).toErrorDev('Each child in a list should have a unique "key" prop.', {
+      withoutStack: gate(flags => flags.enableOwnerStacks),
+    });
+  });
+
+  it('should warn in DEV a child is missing keys in client component', async () => {
     function ParentClient({children}) {
       return children;
     }
     const Parent = clientReference(ParentClient);
-    expect(() => {
+    await expect(async () => {
       const transport = ReactNoopFlightServer.render(
         <Parent>{Array(6).fill(<div>no key</div>)}</Parent>,
       );
       ReactNoopFlightClient.read(transport);
+      await act(async () => {
+        ReactNoop.render(await ReactNoopFlightClient.read(transport));
+      });
     }).toErrorDev(
-      'Each child in a list should have a unique "key" prop. ' +
-        'See https://react.dev/link/warning-keys for more information.',
+      gate(flags => flags.enableOwnerStacks)
+        ? 'Each child in a list should have a unique "key" prop.' +
+            '\n\nCheck the top-level render call using <ParentClient>. ' +
+            'See https://react.dev/link/warning-keys for more information.'
+        : 'Each child in a list should have a unique "key" prop. ' +
+            'See https://react.dev/link/warning-keys for more information.',
     );
   });
 
@@ -2309,7 +2330,7 @@ describe('ReactFlight', () => {
     }
 
     function ThirdPartyFragmentComponent() {
-      return [<span>Who</span>, ' ', <span>dis?</span>];
+      return [<span key="1">Who</span>, ' ', <span key="2">dis?</span>];
     }
 
     function ServerComponent({transport}) {
@@ -2321,7 +2342,7 @@ describe('ReactFlight', () => {
     const promiseComponent = Promise.resolve(<ThirdPartyComponent />);
 
     const thirdPartyTransport = ReactNoopFlightServer.render(
-      [promiseComponent, lazy, <ThirdPartyFragmentComponent />],
+      [promiseComponent, lazy, <ThirdPartyFragmentComponent key="3" />],
       {
         environmentName: 'third-party',
       },
@@ -2413,8 +2434,8 @@ describe('ReactFlight', () => {
     const iteratorPromise = new Promise(r => (resolve = r));
 
     async function* ThirdPartyAsyncIterableComponent({item, initial}) {
-      yield <span>Who</span>;
-      yield <span>dis?</span>;
+      yield <span key="1">Who</span>;
+      yield <span key="2">dis?</span>;
       resolve();
     }
 

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js

@@ -596,6 +596,10 @@ describe('ReactFlightDOMBrowser', () => {
     }
     const Parent = clientExports(ParentClient);
     const ParentModule = clientExports({Parent: ParentClient});
+
+    const container = document.createElement('div');
+    const root = ReactDOMClient.createRoot(container);
+
     await expect(async () => {
       const stream = ReactServerDOMServer.renderToReadableStream(
         <>
@@ -606,11 +610,12 @@ describe('ReactFlightDOMBrowser', () => {
         </>,
         webpackMap,
       );
-      await ReactServerDOMClient.createFromReadableStream(stream);
-    }).toErrorDev(
-      'Each child in a list should have a unique "key" prop. ' +
-        'See https://react.dev/link/warning-keys for more information.',
-    );
+      const result =
+        await ReactServerDOMClient.createFromReadableStream(stream);
+      await act(() => {
+        root.render(result);
+      });
+    }).toErrorDev('Each child in a list should have a unique "key" prop.');
   });
 
   it('basic use(promise)', async () => {

packages/react-server/src/ReactFlightServer.js

@@ -399,6 +399,7 @@ export type Request = {
   onPostpone: (reason: string) => void,
   // DEV-only
   environmentName: string,
+  didWarnForKey: null | WeakSet<ReactComponentInfo>,
 };
 
 const {
@@ -500,6 +501,7 @@ export function createRequest(
   if (__DEV__) {
     request.environmentName =
       environmentName === undefined ? 'Server' : environmentName;
+    request.didWarnForKey = null;
   }
   const rootTask = createTask(request, model, null, false, abortSet);
   pingedTasks.push(rootTask);
@@ -965,6 +967,7 @@ function renderFunctionComponent<Props>(
   props: Props,
   owner: null | ReactComponentInfo, // DEV-only
   stack: null | string, // DEV-only
+  validated: number, // DEV-only
 ): ReactJSONValue {
   // Reset the task's thenable state before continuing, so that if a later
   // component suspends we can reuse the same task object. If the same
@@ -1005,6 +1008,10 @@ function renderFunctionComponent<Props>(
       // being no references to this as an owner.
       outlineModel(request, componentDebugInfo);
       emitDebugChunk(request, componentDebugID, componentDebugInfo);
+
+      if (enableOwnerStacks) {
+        warnForMissingKey(request, key, validated, componentDebugInfo);
+      }
     }
     prepareToUseHooksForComponent(prevThenableState, componentDebugInfo);
     result = callComponentInDEV(Component, props, componentDebugInfo);
@@ -1102,6 +1109,8 @@ function renderFunctionComponent<Props>(
       if (__DEV__) {
         (result: any)._debugInfo = iterableChild._debugInfo;
       }
+    } else if (__DEV__ && (result: any).$$typeof === REACT_ELEMENT_TYPE) {
+      (result: any)._store.validated = 1;
     }
   }
   // Track this element's key on the Server Component on the keyPath context..
@@ -1124,11 +1133,68 @@ function renderFunctionComponent<Props>(
   return json;
 }
 
+function warnForMissingKey(
+  request: Request,
+  key: null | string,
+  validated: number,
+  componentDebugInfo: ReactComponentInfo,
+): void {
+  if (__DEV__) {
+    if (validated !== 2) {
+      return;
+    }
+
+    let didWarnForKey = request.didWarnForKey;
+    if (didWarnForKey == null) {
+      didWarnForKey = request.didWarnForKey = new WeakSet();
+    }
+    const parentOwner = componentDebugInfo.owner;
+    if (parentOwner != null) {
+      if (didWarnForKey.has(parentOwner)) {
+        // We already warned for other children in this parent.
+        return;
+      }
+      didWarnForKey.add(parentOwner);
+    }
+
+    // Call with the server component as the currently rendering component
+    // for context.
+    callComponentInDEV(
+      () => {
+        console.error(
+          'Each child in a list should have a unique "key" prop.' +
+            '%s%s See https://react.dev/link/warning-keys for more information.',
+          '',
+          '',
+        );
+      },
+      null,
+      componentDebugInfo,
+    );
+  }
+}
+
 function renderFragment(
   request: Request,
   task: Task,
   children: $ReadOnlyArray<ReactClientValue>,
 ): ReactJSONValue {
+  if (__DEV__) {
+    for (let i = 0; i < children.length; i++) {
+      const child = children[i];
+      if (
+        child !== null &&
+        typeof child === 'object' &&
+        child.$$typeof === REACT_ELEMENT_TYPE
+      ) {
+        const element: ReactElement = (child: any);
+        if (element.key === null && !element._store.validated) {
+          element._store.validated = 2;
+        }
+      }
+    }
+  }
+
   if (task.keyPath !== null) {
     // We have a Server Component that specifies a key but we're now splitting
     // the tree using a fragment.
@@ -1231,6 +1297,7 @@ function renderClientElement(
   props: any,
   owner: null | ReactComponentInfo, // DEV-only
   stack: null | string, // DEV-only
+  validated: number, // DEV-only
 ): ReactJSONValue {
   // We prepend the terminal client element that actually gets serialized with
   // the keys of any Server Components which are not serialized.
@@ -1242,7 +1309,7 @@ function renderClientElement(
   }
   const element = __DEV__
     ? enableOwnerStacks
-      ? [REACT_ELEMENT_TYPE, type, key, props, owner, stack]
+      ? [REACT_ELEMENT_TYPE, type, key, props, owner, stack, validated]
       : [REACT_ELEMENT_TYPE, type, key, props, owner]
     : [REACT_ELEMENT_TYPE, type, key, props];
   if (task.implicitSlot && key !== null) {
@@ -1292,6 +1359,7 @@ function renderElement(
   props: any,
   owner: null | ReactComponentInfo, // DEV only
   stack: null | string, // DEV only
+  validated: number, // DEV only
 ): ReactJSONValue {
   if (ref !== null && ref !== undefined) {
     // When the ref moves to the regular props object this will implicitly
@@ -1312,7 +1380,15 @@ function renderElement(
   if (typeof type === 'function') {
     if (isClientReference(type) || isOpaqueTemporaryReference(type)) {
       // This is a reference to a Client Component.
-      return renderClientElement(task, type, key, props, owner, stack);
+      return renderClientElement(
+        task,
+        type,
+        key,
+        props,
+        owner,
+        stack,
+        validated,
+      );
     }
     // This is a Server Component.
     return renderFunctionComponent(
@@ -1323,10 +1399,11 @@ function renderElement(
       props,
       owner,
       stack,
+      validated,
     );
   } else if (typeof type === 'string') {
     // This is a host element. E.g. HTML.
-    return renderClientElement(task, type, key, props, owner, stack);
+    return renderClientElement(task, type, key, props, owner, stack, validated);
   } else if (typeof type === 'symbol') {
     if (type === REACT_FRAGMENT_TYPE && key === null) {
       // For key-less fragments, we add a small optimization to avoid serializing
@@ -1347,11 +1424,19 @@ function renderElement(
     }
     // This might be a built-in React component. We'll let the client decide.
     // Any built-in works as long as its props are serializable.
-    return renderClientElement(task, type, key, props, owner, stack);
+    return renderClientElement(task, type, key, props, owner, stack, validated);
   } else if (type != null && typeof type === 'object') {
     if (isClientReference(type)) {
       // This is a reference to a Client Component.
-      return renderClientElement(task, type, key, props, owner, stack);
+      return renderClientElement(
+        task,
+        type,
+        key,
+        props,
+        owner,
+        stack,
+        validated,
+      );
     }
     switch (type.$$typeof) {
       case REACT_LAZY_TYPE: {
@@ -1372,6 +1457,7 @@ function renderElement(
           props,
           owner,
           stack,
+          validated,
         );
       }
       case REACT_FORWARD_REF_TYPE: {
@@ -1383,6 +1469,7 @@ function renderElement(
           props,
           owner,
           stack,
+          validated,
         );
       }
       case REACT_MEMO_TYPE: {
@@ -1395,6 +1482,7 @@ function renderElement(
           props,
           owner,
           stack,
+          validated,
         );
       }
     }
@@ -1963,8 +2051,11 @@ function renderModelDestructive(
           props,
           __DEV__ ? element._owner : null,
           __DEV__ && enableOwnerStacks
-            ? filterDebugStack(element._debugStack)
+            ? !element._debugStack || typeof element._debugStack === 'string'
+              ? element._debugStack
+              : filterDebugStack(element._debugStack)
             : null,
+          __DEV__ && enableOwnerStacks ? element._store.validated : 0,
         );
       }
       case REACT_LAZY_TYPE: {