4 ms·

React has been quite nice, and very fast to work on. But there's one aspect that's concerning. You have state and props, and the React tree diffing algorithm a

React has been quite nice, and very fast to work on. But there's one aspect that's concerning. You have state and props, and the React tree diffing algorithm attempts to make some minimal set of changes that preserves state. However this algorithm is a heuristic, and so it's possible to hit a case where your component gets a different component's state. For example, say there's a text field which has keyboard focus. Another text field is added above it: the keyboard focus should stay in the bottom text field. But React has no notion of identity and doesn't know whether the top or bottom text field is new. All it sees is a diff from one text field to two, and who knows which one will get focus! React's answer to this is to introduce identity by assigning each component a key, and that works. But this is reactive: I notice a bug, and fix that case by assigning a key, but still most elements do not have a key and it is not reasonable to add one. I'm stuck hoping that the opaque tree-diffing heuristic is correct enough for my case. How do other React users handle this? How do you go from "seems to work" to "I am certain React's tree diff will assign the right state to the right components in all cases?"

dominic_cocch 11y ago
This should really only be an issue when the children of an element are all the same type of element and are at the same depth level. For example, this is very common for li elements in a ul element. We try to keep the structure of the html so that wrappers generally keep different UI components separate from each other. In other cases, like in the example in the blog post, we just use the index from the .map() loop to add a key to every element we know will need one. React should also warn you when this is an issue.
- bsimpson 11y ago
  Moreover, you'll get a warning in the console if there are ambiguous children telling you to add keys.
- ash 11y ago
  > we just use the index from the .map() loop to add a key to every element Doing this could hide weird bugs. Let's say we render a list of posts with "remove" button for each one. The user clicks on "remove" button, we modify the underlying this.state.posts. And on next re-render removed post key would be assigned to the next post! The next post could inherit just-deleted post handlers, for example. It's much better to use unique keys like database ids.
  - dominic_cocch 11y ago
    Great point. This has to be the funniest and most confusing bug when you first encounter it! If your list is totally static, an index will do the job. However, we usually end up appending something like an id from the db since almost nothing on Instacart is static.
- ridiculous_fish 11y ago
  What prevents state cross-talk from distinct components? Do we just hope that their structure is sufficiently different that React doesn't mistakenly infer identity? Even the simplest elements like <p> have state and meaningful identity, like their text selection range.
- bpizzi 11y ago
  As a side note, I use uniqueId() from the venerable underscore.js utility lib (last version is 5,7kb only).
  - tlrobinson 11y ago
    Won't that cause the element to get a different key every time render() is called, causing React to think each is always a new element thus throwing away the state every render?
    - feedjoelpie 11y ago
      You are correct. _.uniqueId() will absolutely wreck your list rendering performance if it is called on every render. 99% of the time, you can find a better unique identifier, but if you absolutely need something like _.uniqueId(), it should never be called in the render loop. It should be attached to the data being iterated over, in advance. But... the other problem with _.uniqueId is that in an isomorphic setup your server will just keep counting up and up while your client will start from 0 on every page load. So the first time you reinflate React on the client side the checksums won't match and will cause a full-page re-render.
      - bpizzi 11y ago
        I only do that on the client, and yes, as a matter of facts, things are rerendered each time. But we are talking of a couple uls with <10 elements, frankly the eye doesn't catch anything at all... Honestly, the real damage of this bad habit of mine show off when I've to explain to my co-workers why it could be bad to do this for every situation.
- andrewingram 11y ago
  Wait, you're using the loop index as the key? This is React's default behaviour, and it's what it warns you about. Assuming you're looping over some kind of list of models, use the model's ID instead. Basically, no matter where in the array a value is, it should always have the same key.
- seivan 11y ago
  NO! Don't use your collections indices as a key, that defeats the whole purpose. Your indices can change without content changing! You want to use item.id or something not as volatile as a collection index!
deleted 11y ago
[deleted]
dceddia 11y ago
I think the correct way to handle this is to always set a key. When I try to render some list items without a key I get this warning in the console: Warning: Each child in an array or iterator should have a unique "key" prop. Check the render method of `YourComponent`. See https://fb.me/react-warning-keys https://fb.me/react-warning-keys for more information.
feedjoelpie 11y ago
I'm seeing a lot of bad solutions to this problem written in response to you, so let me say "Do not ignore the the console warnings about array item keys" and list out some gotchas: * Supply the key to the top level list item component. Do NOT have the list item component define its own key in its render method. That is not how it works * Don't use the array index for the key. Use something that's unique to the VALUE. * Don't use _.uniqueId() or similar in any render loop. It will trigger a re-render every time, even if your underlying data has not changed. * If you use _.uniqueId() outside of a render loop, for example to set a temp ID on a brand new data model (what Backbone does by default), and you have an isomorphic setup, make sure that your client and server counters stay in sync. The server version of _.uniqueId() will just keep counting up forever unless you do something about it. You really need to try to find something unique (or unique enough) about the data itself. In the case of strings, use the string as the key. If the case of objects, if you really HAVE to, if there is nothing else that could possibly work right, you might get desperate and JSON.stringify them to make the key. Hopefully it doesn't come to that but it's still often better than alternatives that force full DOM reconciliation on the whole list.
- ConAntonakos 11y ago
  Shit. I've been using uniqueId from Lodash b/c I wasn't sure what was a better solution. What if it's a static, non-DB-fetched array? Would array indices still not suffice?
  - raquo 11y ago
    If the array does not change over time, using array indices is perfectly fine. You can use anything as a key as long as you guarantee that it stays the same for that specific element. If you don't change or reorder the array, the element's index does give such a guarantee.

dominic_cocch 11y ago

This should really only be an issue when the children of an element are all the same type of element and are at the same depth level. For example, this is very common for li elements in a ul element. We try to keep the structure of the html so that wrappers generally keep different UI components separate from each other. In other cases, like in the example in the blog post, we just use the index from the .map() loop to add a key to every element we know will need one. React should also warn you when this is an issue.

bsimpson 11y ago
Moreover, you'll get a warning in the console if there are ambiguous children telling you to add keys.
ash 11y ago
> we just use the index from the .map() loop to add a key to every element Doing this could hide weird bugs. Let's say we render a list of posts with "remove" button for each one. The user clicks on "remove" button, we modify the underlying this.state.posts. And on next re-render removed post key would be assigned to the next post! The next post could inherit just-deleted post handlers, for example. It's much better to use unique keys like database ids.
- dominic_cocch 11y ago
  Great point. This has to be the funniest and most confusing bug when you first encounter it! If your list is totally static, an index will do the job. However, we usually end up appending something like an id from the db since almost nothing on Instacart is static.
ridiculous_fish 11y ago
What prevents state cross-talk from distinct components? Do we just hope that their structure is sufficiently different that React doesn't mistakenly infer identity? Even the simplest elements like <p> have state and meaningful identity, like their text selection range.
bpizzi 11y ago
As a side note, I use uniqueId() from the venerable underscore.js utility lib (last version is 5,7kb only).
- tlrobinson 11y ago
  Won't that cause the element to get a different key every time render() is called, causing React to think each is always a new element thus throwing away the state every render?
  - feedjoelpie 11y ago
    You are correct. _.uniqueId() will absolutely wreck your list rendering performance if it is called on every render. 99% of the time, you can find a better unique identifier, but if you absolutely need something like _.uniqueId(), it should never be called in the render loop. It should be attached to the data being iterated over, in advance. But... the other problem with _.uniqueId is that in an isomorphic setup your server will just keep counting up and up while your client will start from 0 on every page load. So the first time you reinflate React on the client side the checksums won't match and will cause a full-page re-render.
    - bpizzi 11y ago
      I only do that on the client, and yes, as a matter of facts, things are rerendered each time. But we are talking of a couple uls with <10 elements, frankly the eye doesn't catch anything at all... Honestly, the real damage of this bad habit of mine show off when I've to explain to my co-workers why it could be bad to do this for every situation.
andrewingram 11y ago
Wait, you're using the loop index as the key? This is React's default behaviour, and it's what it warns you about. Assuming you're looping over some kind of list of models, use the model's ID instead. Basically, no matter where in the array a value is, it should always have the same key.
seivan 11y ago
NO! Don't use your collections indices as a key, that defeats the whole purpose. Your indices can change without content changing! You want to use item.id or something not as volatile as a collection index!

deleted 11y ago

[deleted]

dceddia 11y ago

I think the correct way to handle this is to always set a key. When I try to render some list items without a key I get this warning in the console: Warning: Each child in an array or iterator should have a unique "key" prop. Check the render method of `YourComponent`. See https://fb.me/react-warning-keys https://fb.me/react-warning-keys for more information.

feedjoelpie 11y ago

I'm seeing a lot of bad solutions to this problem written in response to you, so let me say "Do not ignore the the console warnings about array item keys" and list out some gotchas: * Supply the key to the top level list item component. Do NOT have the list item component define its own key in its render method. That is not how it works * Don't use the array index for the key. Use something that's unique to the VALUE. * Don't use _.uniqueId() or similar in any render loop. It will trigger a re-render every time, even if your underlying data has not changed. * If you use _.uniqueId() outside of a render loop, for example to set a temp ID on a brand new data model (what Backbone does by default), and you have an isomorphic setup, make sure that your client and server counters stay in sync. The server version of _.uniqueId() will just keep counting up forever unless you do something about it. You really need to try to find something unique (or unique enough) about the data itself. In the case of strings, use the string as the key. If the case of objects, if you really HAVE to, if there is nothing else that could possibly work right, you might get desperate and JSON.stringify them to make the key. Hopefully it doesn't come to that but it's still often better than alternatives that force full DOM reconciliation on the whole list.

ConAntonakos 11y ago
Shit. I've been using uniqueId from Lodash b/c I wasn't sure what was a better solution. What if it's a static, non-DB-fetched array? Would array indices still not suffice?
- raquo 11y ago
  If the array does not change over time, using array indices is perfectly fine. You can use anything as a key as long as you guarantee that it stays the same for that specific element. If you don't change or reorder the array, the element's index does give such a guarantee.