Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Each child in a list should have a unique "key" prop warning when "key" prop is after a prop spread operator/syntax #55642

Closed
1 task done
esseb opened this issue Sep 20, 2023 · 18 comments
Labels
bug Issue was opened via the bug report template. locked

Comments

@esseb
Copy link

esseb commented Sep 20, 2023

I mention this in additional context at the bottom, but just to be clear up top: I can not reproduce this bug using react@18.2.0 itself. The warning only appears when I use next.js (which uses react@18.2.0)

Link to the code that reproduces this issue

https://github.com/esseb/next-react-key-bug

To Reproduce

  1. Create a component with nested mapping where the key prop of the child in the first mapping is after a prop spread operator
    • Example code below, or in the linked repository
  2. Open the "Source" tab in the browser's devtools
  3. Reload the page

Example code

const categories = [
  {
    name: "a",
    subcategories: ["a0", "a1", "a2"]
  },
  {
    name: "b",
    subcategories: ["b0", "b1", "b2"]
  },
  {
    name: "c",
    subcategories: ["c0", "c1", "c2"]
  }
]

export default function Home() {
  return (
    <div>
      {categories.map((category, categoryIndex) => {
        return (
          <button
            // The warning disappears if the key is before the spread operator
            // key={categoryIndex}
            {...{
              className: 'example',
            }}
            key={categoryIndex}
          >
            <span>{category.name}</span>
            <div>
              {category.subcategories.map((subcategory, subcategoryIndex) => (
                <span key={subcategoryIndex}>{subcategory}</span>
              ))}
            </div>
          </button>
        );
      })}
    </div>
  );
}

Current vs. Expected behavior

Expected result

No warning

Actual result

This warning appears

Warning: Each child in a list should have a unique "key" prop. See https://reactjs.org/link/warning-keys for more information.

Further information

The warning disappears if the key prop of the child in the first mapping is before the prop spread operator.

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Before running npm install next@canary

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 22.6.0: Wed Jul  5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000
Binaries:
  Node: 19.0.0
  npm: 9.6.3
  Yarn: 1.22.19
  pnpm: 7.29.0
Relevant Packages:
  next: 13.4.19
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: N/A
Next.js Config:
  output: N/A

After running npm install next@canary

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 22.6.0: Wed Jul  5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000
Binaries:
  Node: 19.0.0
  npm: 9.6.3
  Yarn: 1.22.19
  pnpm: 7.29.0
Relevant Packages:
  next: 13.5.1-canary.1
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: N/A
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Not sure

Additional context

My reproduction repository contains two folders:

  • next
  • create-react-app

Both use react@18.2.0 judging from the respective package-lock.json file. The same code is in both apps but the warning only appears in the next app, so I don't think the bug is in React itself.

@esseb esseb added the bug Issue was opened via the bug report template. label Sep 20, 2023
@icyJoseph
Copy link
Contributor

icyJoseph commented Sep 20, 2023

You can open your project on Stackblitz through: https://stackblitz.com/github/esseb/next-react-key-bug/tree/main/next

@esseb
Copy link
Author

esseb commented Sep 20, 2023

That StackBlitz is using modified code (modified in a way where the warning disappears):
Screenshot 2023-09-20 at 11 43 18

With the original code (as in the repo), the bug is reproducible on StackBlitz also for me:
Screenshot 2023-09-20 at 11 43 39

Screenshot 2023-09-20 at 11 44 52

@icyJoseph
Copy link
Contributor

icyJoseph commented Sep 20, 2023

Mmm 🤔 I thought I checked both. Will redo after lunch!

Ah nvm, I must've cleared the console accidentally.

@icyJoseph
Copy link
Contributor

Odd, the React DevTools shows the keys 🤔 - what is going on....

Screenshot 2023-09-20 at 12 24 11

@icyJoseph
Copy link
Contributor

I modified it like this to see the key presence https://stackblitz.com/edit/github-twhbet?file=pages%2Findex.js - still raises the warning

@icyJoseph
Copy link
Contributor

icyJoseph commented Sep 20, 2023

Doing this, wrapping Button children with a Fragment.

      {categories.map((category, categoryIndex) => {
        return (
          <Button
            // The warning disappears if the key is before the spread operator
            // key={categoryIndex}
            {...{
              className: 'example',
            }}
            key={categoryIndex}
          >
            <>
              <span>{category.name}</span>
              <div>
                {category.subcategories.map((subcategory, subcategoryIndex) => (
                  <span key={subcategoryIndex}>{subcategory}</span>
                ))}
              </div>
            </>
          </Button>
        );
      })}

Stops the warning... 🤔

I had thought that the div nested in a button caused some hydration error, but nope.

Replacing it with whatever else does nothing, and it makes no sense that the fragment would function as workaround anyway

@esseb
Copy link
Author

esseb commented Sep 20, 2023

That seems likely. Changing the button to a div makes the error go away. This explains why I didn't see the issue in the create-next-app version which doesn't do SSR.

Doesn't really explain why moving the key prop before the spread operator makes the warning disappear though, but this seems more likely to be a bug I can report directly to React if I can reproduce using hydration without using next.js.

@icyJoseph
Copy link
Contributor

Yeah, not sure what's up with this one... maybe Next is using React canaries or something like that, during development, and you've been just lucky to see this "issue"?

Super weird. I think I am out of ideas, besides these tests/tries I've made.

@hipdev
Copy link

hipdev commented Sep 21, 2023

Very fun right 😄
Let me explain some things, keys and refs always come first.
Maybe you're worried about the object overwriting the key right?
Test this with your example:

const categories = [
  {
    name: 'a',
    subcategories: ['a0', 'a1', 'a2'],
    key: 'a',
  },
  {
    name: 'b',
    subcategories: ['b0', 'b1', 'b2'],
    key: 'b',
  },
  {
    name: 'c',
    subcategories: ['c0', 'c1', 'c2'],
    key: 'c',
  },
]

And remove the key, then you will see an error like

React keys must be passed directly to JSX without using spread...

Now check this one 👀

// Better a unique id than an index, just saying haha
const categories = [
  { id: 1, name: 'Category 1' },
  { id: 2, name: 'Category 2' },
]

// Render the categories
const CategoryList = () => {
  return (
    <ul>
      {categories.map((category) => (
        <li key={category.id} {...category} className="category-item">
          {category.name}
        </li>
      ))}
    </ul>
  )
}

Here is a more general explanation of why the key prop should go before the spread operator in React.js:

The key prop is used by React to identify elements in a list and to track their position in the list. This allows React to efficiently update the list when elements are added, removed, or rearranged.

The spread operator allows you to spread the properties of an object into another object. This can be useful for creating new objects or for updating existing objects.

If you place the key prop after the spread operator, the key will be used for the entire spread object, which is not what you want. This is because the key should only be used for the individual element that you are rendering (the button in your case).

To ensure that the key prop is used for the individual element, you should place it before the spread operator. This will ensure that the key is used for the correct element, and that React can efficiently update the list.

I know you provide just an example, but it is always better to give a type for buttons; some browsers take them as a type="submit" by default, others like type="button", you know, browsers are not friends 😂
The why not use indexes as keys is well known, but that's for another talk, check multiple posts on Google.

Anyway, I think there should be some kind of help or warning to the developer when he tries to put a key or ref after a spread operator, who knows why the community has not done it🤔.

@oliviertassinari
Copy link
Contributor

So, a regression in Next.js?

@mattdev-sh
Copy link

I am using version 14.0.4 of next and autocomplete of mui and have the same problem. At first I thought the problem was in my code but a instead the error just reports a reading problem within mui.

I found only one other topic in the mui repo, but with react it works, so the problem seems to be in next. How did you continue to work this bug? Have you changed libraries?

It seems strange to me that this problem has only come out now with mui.

        <Autocomplete
            multiple
            id="checkboxes-tags-demo"
            options={top100Films}
            disableCloseOnSelect
            getOptionLabel={(option) => option.title}
            renderOption={(props, option, { selected }) => (
                <li key={props.key}  {...props}>
                    <Checkbox
                        style={{ marginRight: 8 }}
                        checked={selected}
                    />
                    {option.title}
                </li>
            )}
            style={{width: 500}}
            renderInput={(params) => (
                <TextField {...params} label="Checkboxes" placeholder="Favorites"/>
            )}
        />
    );```

@esseb
Copy link
Author

esseb commented Jan 17, 2024

I haven't had time/desire to investigate the bug further yet. I just moved all my keys before the spread operators in my project.

Your code example already has the key before the spread operator though, so I'm not sure you're seeing the same issue.

@mattdev-sh
Copy link

I haven't had time/desire to investigate the bug further yet. I just moved all my keys before the spread operators in my project.

Your code example already has the key before the spread operator though, so I'm not sure you're seeing the same issue.

That's what I thought too, so I simply put a key before the props, but still I get the same error when I go to click in the options.

 Warning: A props object containing a "key" prop is being spread into JSX:
  let props = {key: someKey, tabIndex: ..., role: ..., id: ..., onMouseMove: ..., onClick: ..., onTouchStart: ..., data-option-index: ..., aria-disabled: ..., aria-selected: ..., className: ..., children: ...};
  <li {...props} />
React keys must be passed directly to JSX without using spread:
  let props = {tabIndex: ..., role: ..., id: ..., onMouseMove: ..., onClick: ..., onTouchStart: ..., data-option-index: ..., aria-disabled: ..., aria-selected: ..., className: ..., children: ...};
  <li key={someKey} {...props} />
    at Autocomplete (webpack-internal:///(app-pages-browser)/./node_modules/@mui/material/Autocomplete/Autocomplete.js:506:85)

I have tried this way as well, but no luck.

renderOption={(props, option, index) => {
                const key = `listItem-${index}-${option.id}`;
                return (
                    <li  key={key} {...props} >
                        {option.title}
                    </li>

                );
            }}

I have already used autocomplete with react, starting with that and creating a component with other different functions.
I also tried the demo of mui on codesandbox and indeed, being done only with react, it works perfectly.
I have no other ideas at the moment.

@MrOxMasTer
Copy link

I also get an error, and I don't understand where the problem is. I hope in the future they will show which component the problem will be in.
image

@artemis-prime
Copy link

+1 Not even using a spread op

@chrisdemetriad
Copy link

Same issue. I am using Autocomplete from MUI with Next.js.

<Autocomplete
multiple
filterSelectedOptions
options={teamMembers}
defaultValue={[teamMembers[0]]}
getOptionLabel={(option) => ${option.name} (${option.role})}
renderInput={(params) => <TextField {...params} label="Team" placeholder="Search team members" />}
/>

Error:

  let props = {key: someKey, label: ..., size: ..., className: ..., disabled: ..., data-tag-index: ..., tabIndex: ..., onDelete: ...};
  <ForwardRef(Chip) {...props} />

@esseb
Copy link
Author

esseb commented Mar 20, 2024

Closing this. My original issue was about a very specific issue with a key prop after a spread operator which I don't think is an issue in next.js itself. I haven't had time to debug this further and report this in the correct place yet though.

I don't think anyone else but me who's commented in this issue sees the specific issue I saw where the issue only occurred with a key prop after a spread operator.

@esseb esseb closed this as completed Mar 20, 2024
Copy link
Contributor

github-actions bot commented Apr 4, 2024

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot added the locked label Apr 4, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. locked
Projects
None yet
Development

No branches or pull requests

9 participants