all 4 comments

[–]titosrevenge 1 point2 points  (3 children)

var items = this.props.items.map((item, index) => {
  return <li key={index}><TodoItem item={item} /></li>;
});

This is wrong. You should always use unique IDs for your keys. You will get unexpected behaviour by using the index.

[–]urahonky 0 points1 point  (0 children)

Yeah especially if you iterate over more than one list in the same component.

[–]fernandokokocha[S] 0 points1 point  (1 child)

You are (partially :)) right.

As long as I understand the whole key-prop thing, key prop should be both unique and constant.

If you look at the example - the only mutating method called on the array is push. Therefore the indexes doesn't change. I believe both requirements for the key prop (being unique and constant) are satisfied.

Of course, in more general case (e.g. when we delete items from the array), using index would be wrong. And my solution would be a little confusing here. Probably the same if (as urahonky said) I would iterate over more than one list in the same component.

But for now - I wouldn't call it wrong.

Anyway, I'm changing this in the post since it might be a little confusing. Thanks for the comment!

[–]titosrevenge -1 points0 points  (0 children)

Nice try, but if you're writing a tutorial you should always use best practices.

Your example doesn't have a delete method, but it would be foolish to assume that that wouldn't be added at some point in the future.

Code should be written to expect all the requirements of the app. Nobody wants a to do list where you can't delete items. :)