Hacker News Reader

Comment by dominic_cocch | Hacker News Reader

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.

View on HN

> 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.

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.

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.

Moreover, you'll get a warning in the console if there are ambiguous children telling you to add keys.

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.

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!

As a side note, I use uniqueId() from the venerable underscore.js utility lib (last version is 5,7kb only).

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?

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.

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.

Reply on news.ycombinator.com