Repository: facebook/react
Pull request: Fix Missing `key` Validation in `React.Children`

## Summary

In https://github.com/facebook/react/pull/29088, the validation logic for `React.Children` inspected whether `mappedChild` — the return value of the map callback — has a valid `key`. However, this deviates from existing behavior which only warns if the original `child` is missing a required `key`.

This fixes false positive `key` validation warnings when using `React.Children`, by validating the original `child` instead of `mappedChild`.

This is a more general fix that expands upon my previous fix in https://github.com/facebook/react/pull/29662.

## How did you test this change?

```
$ yarn test ReactChildren-test.js
```

Changed files:
filename: packages/react/src/ReactChildren.js
patch: @@ -229,11 +229,21 @@ function mapIntoArray(
             childKey,
         );
         if (__DEV__) {
-          if (nameSoFar !== '' && mappedChild.key == null) {
-            // We need to validate that this child should have had a key before assigning it one.
-            if (!newChild._store.validated) {
-              // We mark this child as having failed validation but we let the actual renderer
-              // print the warning later.
+          // If `child` was an element without a `key`, we need to validate if
+          // it should have had a `key`, before assigning one to `mappedChild`.
+          // $FlowFixMe[incompatible-type] Flow incorrectly thinks React.Portal doesn't have a key
+          if (
+            nameSoFar !== '' &&
+            child != null &&
+            isValidElement(child) &&
+            child.key == null
+          ) {
+            // We check truthiness of `child._store.validated` instead of being
+            // inequal to `1` to provide a bit of backward compatibility for any
+            // libraries (like `fbt`) which may be hacking this property.
+            if (child._store && !child._store.validated) {
+              // Mark this child as having failed validation, but let the actual
+              // renderer print the warning later.
               newChild._store.validated = 2;
             }
           }

filename: packages/react/src/__tests__/ReactChildren-test.js
patch: @@ -868,7 +868,105 @@ describe('ReactChildren', () => {
     ]);
   });
 
-  it('should warn for flattened children lists', async () => {
+  it('warns for mapped list children without keys', async () => {
+    function ComponentRenderingMappedChildren({children}) {
+      return (
+        <div>
+          {React.Children.map(children, child => (
+            <div />
+          ))}
+        </div>
+      );
+    }
+
+    const container = document.createElement('div');
+    const root = ReactDOMClient.createRoot(container);
+    await expect(async () => {
+      await act(() => {
+        root.render(
+          <ComponentRenderingMappedChildren>
+            {[<div />]}
+          </ComponentRenderingMappedChildren>,
+        );
+      });
+    }).toErrorDev([
+      'Warning: Each child in a list should have a unique "key" prop.',
+    ]);
+  });
+
+  it('does not warn for mapped static children without keys', async () => {
+    function ComponentRenderingMappedChildren({children}) {
+      return (
+        <div>
+          {React.Children.map(children, child => (
+            <div />
+          ))}
+        </div>
+      );
+    }
+
+    const container = document.createElement('div');
+    const root = ReactDOMClient.createRoot(container);
+    await expect(async () => {
+      await act(() => {
+        root.render(
+          <ComponentRenderingMappedChildren>
+            <div />
+            <div />
+          </ComponentRenderingMappedChildren>,
+        );
+      });
+    }).toErrorDev([]);
+  });
+
+  it('warns for cloned list children without keys', async () => {
+    function ComponentRenderingClonedChildren({children}) {
+      return (
+        <div>
+          {React.Children.map(children, child => React.cloneElement(child))}
+        </div>
+      );
+    }
+
+    const container = document.createElement('div');
+    const root = ReactDOMClient.createRoot(container);
+    await expect(async () => {
+      await act(() => {
+        root.render(
+          <ComponentRenderingClonedChildren>
+            {[<div />]}
+          </ComponentRenderingClonedChildren>,
+        );
+      });
+    }).toErrorDev([
+      'Warning: Each child in a list should have a unique "key" prop.',
+    ]);
+  });
+
+  it('does not warn for cloned static children without keys', async () => {
+    function ComponentRenderingClonedChildren({children}) {
+      return (
+        <div>
+          {React.Children.map(children, child => React.cloneElement(child))}
+        </div>
+      );
+    }
+
+    const container = document.createElement('div');
+    const root = ReactDOMClient.createRoot(container);
+    await expect(async () => {
+      await act(() => {
+        root.render(
+          <ComponentRenderingClonedChildren>
+            <div />
+            <div />
+          </ComponentRenderingClonedChildren>,
+        );
+      });
+    }).toErrorDev([]);
+  });
+
+  it('warns for flattened list children without keys', async () => {
     function ComponentRenderingFlattenedChildren({children}) {
       return <div>{React.Children.toArray(children)}</div>;
     }
@@ -888,7 +986,7 @@ describe('ReactChildren', () => {
     ]);
   });
 
-  it('does not warn for flattened positional children', async () => {
+  it('does not warn for flattened static children without keys', async () => {
     function ComponentRenderingFlattenedChildren({children}) {
       return <div>{React.Children.toArray(children)}</div>;
     }