Skip to content

[Documentation] Details/Example for Indexes as keys #111

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

Merged
merged 6 commits into from
Nov 17, 2017
Merged

[Documentation] Details/Example for Indexes as keys #111

merged 6 commits into from
Nov 17, 2017

Conversation

ajcumine
Copy link
Contributor

This aims to add more information to the documentation and address #79

  1. Added more details on the performance and state issues cause by using indexes as keys in lists.
  2. Added information that React uses indexes as keys if none are given.
  3. Added a link to an example on Codepen to show the issue cause by using indexes as keys in list on state. http://codepen.io/ajcumine/pen/KmVWmQ?editors=0010
  4. Added a link to an example on Codepen to show how not using indexes as keys in a list fixes the previous example's issues with state and reordering. https://codepen.io/ajcumine/pen/ZKQeJM?editors=0010

@reactjs-bot
Copy link

reactjs-bot commented Oct 10, 2017

Deploy preview ready!

Built with commit 383a7cb

https://deploy-preview-111--reactjs.netlify.com

@@ -130,7 +130,7 @@ const todoItems = todos.map((todo, index) =>
);
```

We don't recommend using indexes for keys if the items can reorder, as that would be slow. You may read an [in-depth explanation about why keys are necessary](/docs/reconciliation.html#recursing-on-children) if you're interested.
We don't recommend using indexes for keys if the items can reorder as that would impact performance, and may cause issues with the reordering of components and their respective states. If you choose not to assign a key to your list items then React will use indexes as keys. You may read an [in-depth explanation about why keys are necessary](/react/docs/reconciliation.html#recursing-on-children) if you're interested in more information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /react prefix should not be added to this link.

@@ -138,7 +138,11 @@ In practice, finding a key is usually not hard. The element you are going to dis

When that's not the case, you can add a new ID property to your model or hash some parts of the content to generate a key. The key only has to be unique among its siblings, not globally unique.

As a last resort, you can pass item's index in the array as a key. This can work well if the items are never reordered, but reorders will be slow.
As a last resort, you can pass item's index in the array as a key. This can work well if the items are never reordered. However, there will be a performance impact with reordering as React will naively update components.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually preferred the original wording for this paragraph.


There can also be issues with the state of a component in a list if indexes are used as keys. The state of an item in a list (or any deep state inside of it) will stay attached to the original position of the item, even if the item has “moved” in the data source. This is particularly noticeable with inputs retaining their values in the original positions even when their parent components reorder or are prepended to.

[Here](http://codepen.io/ajcumine/pen/KmVWmQ?editors=0010) is an example of the issues that can be caused by using indexes as keys on CodePen, and [here](https://codepen.io/ajcumine/pen/ZKQeJM?editors=0010) is a updated version of the same example showing how not using indexes as keys will fix these reordering, sorting, and prepending issues.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @gaearon to import these changes into his CodePen profile.

@ajcumine
Copy link
Contributor Author

Thanks for the swift review @bvaughn, I've made the suggested change and fixed the link I had wrong.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 10, 2017

I think these changes look reasonable. I prefer not to merge until we update the Codepen links to point at our standard account (codepen.io/gaearon) so I'll hand this issue to @gaearon for now. 😄

@bvaughn bvaughn requested a review from gaearon October 10, 2017 21:47
@bvaughn
Copy link
Contributor

bvaughn commented Nov 17, 2017

Hi @ajcumine. Sorry for dropping the ball on this PR. I couldn't merge it until we had an answer for the Codepen issue I mentioned previously. Now we do. As of PR #245, we're now able to directly link to code examples stored right here in this project's git repository.

If you'd be willing to update this PR to move the forked examples that you made on Codepen within this project (in the examples directory) then I'll be happy to review and merge it.

@ajcumine
Copy link
Contributor Author

Hi @bvaughn. No worries, I'll update it in the next few days and get everything updated for you to review.

@ajcumine
Copy link
Contributor Author

@bvaughn I found some time to get this done today :) It's all ready for your review, thanks again

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a bunch!

@bvaughn bvaughn merged commit 31ae406 into reactjs:master Nov 17, 2017
@ajcumine ajcumine deleted the docs-no-keys-in-lists branch November 17, 2017 18:42
BetterZxx pushed a commit to BetterZxx/react.dev that referenced this pull request Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants