Munsterberg
Munsterberg

Reputation: 828

Loop over state and check if object exists

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:

Upvotes: 1

Views: 1209

Answers (1)

Chris
Chris

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 mutate this.state but creates a pending state transition. Accessing this.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

Related Questions