Skip to content

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

Merged
bvaughn merged 6 commits intoreactjs:masterfrom
ajcumine:docs-no-keys-in-lists
Nov 17, 2017
Merged

[Documentation] Details/Example for Indexes as keys#111
bvaughn merged 6 commits intoreactjs:masterfrom
ajcumine:docs-no-keys-in-lists

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

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.

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.

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