Challenge Solution in "Updating Arrays in State" Page Uses Mutation

[edzhuang]

For the solution to the third challenge in the Updating Arrays in State page, it uses the following function:

function handleAddTodo(title) {
  setTodos([
    ...todos,
    {
      id: nextId++,
      title: title,
      done: false
    }
  ]);
}

This changes nextId, a variable outside the component's scope, which I believe is a mutation. It should instead use the length of todos to set the next id.

[ferrybig]

Using the length of the array becomes unsafe if you are also going to implement deletion.

You could go to the last items and add 1 to the id, but this also breaks when you add a sorting function

In react, event handlers are allowed to have side effects, so the side effect of mutating the global counter is safe

[jaisaichand]

Pls assign this to me, I would like to work on this if no one is currently working on it

[Ashish-simpleCoder]

We can also try using the Date.now().toString() as value for the id attribute. It will always be unique and safe.

[rickhanlonii]

Yeah, the mutation is ok because it's mutating a module level variable in a click handler. But since this is teaching you to not mutated, it would be good to include a note in the solution that explains why mutating the nextId is OK.

[rickhanlonii]

If you're interested in improving it, please just submit a PR, we don't assign issues.