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

Mention that key is not a prop in the docs. #5740

Closed
wants to merge 1 commit into from

Conversation

dallonf
Copy link

@dallonf dallonf commented Dec 29, 2015

Just had to help my friend with this. Figured I'd try to help the rest of the world, too!

New paragraph, for reference:

Note that key is not a prop; trying to access this.props.key from inside ListItemWrapper.render wil be undefined. If you need to access the same value within the child component, you should pass it as a different prop (ex: <ListItemWrapper key={result.id} id={result.id} />) While this may seem redundant, it's important to separate app logic from reconciling hints.

Just had to help my friend with this. Figured I'd try to help the rest of the world, too!
@jimfb
Copy link
Contributor

jimfb commented Dec 29, 2015

I worry that we are adding a ton of stuff to docs, often covering edge cases that most users never hit. It creates a lot of clutter, and makes the docs harder to understand, since they are covering all the manusha in a page that is trying to communicate a basic high-level understanding. I suspect that most users wouldn't try accessing the key, since it is a React reconciliation hint rather than a prop on the element. And even if they do, I wonder how many of those people would have read+remembered this little note buried deep in the page on "multiple components".

I think it might be better to use Object.defineProperty (in dev mode) to warn if props.key is accessed, and point the user to a note/discussion somewhere. That way, users who actually run into the issue will always get a timely message that sends them directly to the relevant explanation/discussion. I created an issue to track that idea: #5742

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@jimfb
Copy link
Contributor

jimfb commented Feb 2, 2016

Closing in favor of something more like #5744

@jimfb jimfb closed this Feb 2, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants