Reword duplicate key warning #10148
Reword duplicate key warning #10148
Conversation
**what is the change?:**
- Removes the now-inaccurate description of behavior around duplicate
keys
- Adds link to 'key' docs page
- Changes tone to be more casual and friendly
Alternative wording idea;
'Encountered two children with the same key, ${key}
Child keys must be unique; using duplicate keys is not supported and
will cause unexpected behavior in some versions of React.
See https://fb.me/react-warning-keys for more information on how to
fix this.'
**why make this change?:**
Mainly this change was needed because in React 16, duplicate keys will
not cause omission of items with duplicate keys. All items will be
rendered. It could happen that in future versions of React we will have
different behavior though.
**test plan:**
`yarn test`
**issue:**
Wishlist item on facebook#8854
There was a problem hiding this comment.
Overall, I'd say this is good. Maybe we could refine the message part from:
"If it happens - unlucky! To be honest it happens to all of us"
To be:
"The is a very common source of bugs that you may not realise to begin with. We highly recommend you keep using keys, but be mindful of duplications as they will likely cause inconsistencies in your application."
There was a problem hiding this comment.
Can we workshop the new message a bit before merging? I like that the tone of the message is friendlier, but it reads a bit too informally for my taste.
My suggestion is something along the lines of:
Encountered two children with the same key. Keys should be unique so that components maintain their identity across updates. Duplicate keys can lead to confusing bugs — the behavior is unspecified and could change in a future version. See https://fb.me/react-warning-keys for more information.
To be clear, this is NOT the message you get when you forget to add a key at all. This message is when you have two identical (nonempty) keys. It is a lot rarer. I don't think we should advise adding keys here. It would be confusing. I don't think the website link is helpful as-is. If we weren't planning to revise it, I would drop it. I like Andrew's, or you might like my version which is a little more explicit about the behavior you'll see:
|
|
Great - will update this. It would be nice to fix the docs so that they work for this but not needed atm. |
**what is the change?:** Another tweak to the wording of this error to make it more clear and accurate. **why make this change?:** The previous tweak was too casual in tone and still not clear enough. **test plan:** `yarn test` and `REACT_DOM_JEST_USE_FIBER=1 yarn run test` **issue:** Wishlist item on facebook#8854
| 'how to fix this.%s', | ||
| '`%s`. Keys should be unique so that components maintain their ' + | ||
| 'identity across updates. Non-unique keys may cause children to ' + | ||
| 'be duplicated and/or omitted in — the behavior is ' + |
There was a problem hiding this comment.
sorry, my bad -- this shouldn't have "in"
**what is the change?:** Fixed a typo in the updated message
what is the change?:
keys
Alternative wording idea;
'Encountered two children with the same key, ${key}
Child keys must be unique; using duplicate keys is not supported and
will cause unexpected behavior in some versions of React.
See https://fb.me/react-warning-keys for more information on how to
fix this.'
why make this change?:
Mainly this change was needed because in React 16, duplicate keys will
not cause omission of items with duplicate keys. All items will be
rendered. It could happen that in future versions of React we will have
different behavior though.
test plan:
yarn testissue:
Wishlist item on #8854