Repository navigation
Challenge Solution in "Updating Arrays in State" Page Uses Mutation #5672
Description
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.
Reacted by Ashish Prajapati
Activity
[-]Challenge Solution in "Updating Arrays in State" Uses Mutation[/-][+]Challenge Solution in "Updating Arrays in State" Page Uses Mutation[/+]ferrybig commentedon Mar 17, 2023
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 commentedon May 1, 2023
Pls assign this to me, I would like to work on this if no one is currently working on it
Ashish-simpleCoder commentedon May 7, 2023
We can also try using the Date.now().toString() as value for the id attribute.
It will always be unique and safe.
rickhanlonii commentedon Jan 24, 2024
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 commentedon Jan 24, 2024
If you're interested in improving it, please just submit a PR, we don't assign issues.