-
Notifications
You must be signed in to change notification settings - Fork 7.7k
[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
Conversation
Deploy preview ready! Built with commit 383a7cb |
content/docs/lists-and-keys.md
Outdated
@@ -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. |
There was a problem hiding this comment.
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.
content/docs/reconciliation.md
Outdated
@@ -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. |
There was a problem hiding this comment.
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.
content/docs/reconciliation.md
Outdated
|
||
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. |
There was a problem hiding this comment.
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.
Thanks for the swift review @bvaughn, I've made the suggested change and fixed the link I had wrong. |
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. 😄 |
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 |
Hi @bvaughn. No worries, I'll update it in the next few days and get everything updated for you to review. |
@bvaughn I found some time to get this done today :) It's all ready for your review, thanks again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a bunch!
This aims to add more information to the documentation and address #79