Reputation: 828
I have some code that seems to work, just not correctly. I have an onChange
handler that sends up a key and value to setState()
of its parent component.
_onAttributeChange(key, value) {
const changes = this.state.changes;
const pushObj = {key: key, value: value};
console.log(pushObj);
if (changes.length === 0) {
this.setState({changes: changes.concat([pushObj])});
} else {
changes.forEach(change => {
if (change.key === key) {
console.log('Update');
} else {
console.log('No update');
this.setState({changes: changes.concat([pushObj])});
}
});
}
}
You can see from this code that I am creating an object based on the key value parameters that will look like {key: 1, value: 'Some Value'}
. The goal is to concat that to the array if the array is of length 0. That part works fine. The forEach is where I'm having issues. My goal is to loop through the state array, check if the change.key coming in matches a key in the array, and if it does I want to perform an update, otherwise I want to concat once again to add that change to the array.
So 2 questions:
Firstly, when the first change comes in, the loop works properly, and will just console log Update
for each consecutive change with the same key (i.e. 1). When another change comes in, it works properly at first and concats it to the array, but then any consecutive change with the key of 2, it will fire the console log of Update AND the else clause.
Secondly, how can I update the change while looping over the array without mutating the state directly?
Upvotes: 1
Views: 1209
Reputation: 59491
The answer should be quite simple really; just work on a temporary copy of the state and once you're done replace your actual state with the copy.
_onAttributeChange(key, value) {
let changes = this.state.changes.slice();
const pushObj = {key: key, value: value};
console.log(pushObj);
if (changes.length === 0) {
changes = changes.concat([pushObj]);
} else {
changes.forEach(change => {
if (change.key === key) {
console.log('Update');
} else {
console.log('No update');
changes = changes.concat([pushObj]);
}
});
}
this.setState({changes: changes});
}
The problem you were having with the consecutive changes is most likely related to the asynchronous nature of setState()
. Not only does each such call result in an unnecessary re-render but it will also, almost certainly, update your state in an unwanted way.
Have a look at what the official React documentation says about setState():
setState()
does not immediately mutatethis.state
but creates a pending state transition. Accessingthis.state
after calling this method can potentially return the existing value. There is no guarantee of synchronous operation of calls to setState and calls may be batched for performance gains.
So basically it is likely returning the "old" state value when you actually want the updated one from the previous iteration in your loop.
Another very important thing to note here is that you need to create a copy of the state property you're working on. Otherwise you'd be making changes to the state directly, which might work, but is discouraged.
So basically always do:
//To copy an array
let arr = this.state.myArray; //wrong. arr still references myArray
let arr = this.state.myArray.slice(); //correct
//To copy an object
let obj = this.state.myObject; //wrong. obj still references myObject
let obj = Object.assign({}, this.state.myObject); //correct
More info on MDN for slice() and Object.assign().
Also, use const
sparingly. Only use it when a variable must be immutable.
TL;DR - Always do all the logic first, then commit your state changes.
Upvotes: 3