ga-wdi-exercises / project4

[project]
https://github.com/ga-wdi-exercises/project4
2 stars 8 forks source link

adjusting states of objects #543

Closed perryf closed 7 years ago

perryf commented 7 years ago

If I have a state that holds objects, how would I go about adjusting the inner objects--and even objects inside those? setState doesn't seem to like the nested objects.

    this.state = {
    synth: new Tone.Synth({
      oscillator: {
        type: 'fmsquare',
        modulationType: 'sawtooth',
        modulationIndex: 3,
        harmonicity: 3.4
      },
      envelope: {
        attack: 0.001,
        decay: 0.1,
        sustain: 0.1,
        release: 0.1
      }
    }).toMaster()
    }
  }

It seems like there are some work arounds but none of them seem that efficient. Am i going about this the wrong way?

jsm13 commented 7 years ago

I think the problem you may be running into is that using setState and passing it an object will perform a shallow merge between the state and the object you passed so:

// state was { car: { make: 'GMC', model: 'Suburban'}, foo: 1 }
this.setState({car: {make: 'Chevy'}, bar: 2 }
// In the next render...
this.state // => { car: { make: 'Chevy' }, foo: 1, bar: 2 }

A shallow merge goes through each top level property and assign its value to the state object. That means that since the object passed to setState didn't have a property foo, that property in state doesn't change. New properties get added as shown by bar. Nested objects however are not merged but simply over written. What you want in this case is to pass setState a function instead. This function is called with state and props as arguments and its return value will be shallow merged with state. We don't care about props so we won't add a param for it:

// state was { car: { make: 'GMC', model: 'Suburban'}, foo: 1 }
this.setState(state => {
  return {
    car: Object.assign({}, state.car, { make: 'Chevy' }),
    bar: 2
  }
})
// In the next render...
this.state // => { car: { make: 'Chevy', model: 'Suburban'}, foo: 1, bar: 2}
perryf commented 7 years ago

Hey, thanks so much. That makes sense about the shallow merge. The way I ended up fixing the problem late last night was whenever I wanted to setState, I would create a copy var equal to the original state object, change the copy, and then setState save the original to the updated copy. I don't know if its that most concise way but it seems like dealing with large objects in react gets a little messy either way.

changePitch(slider) {
    let synth = this.state.synth
    let newPitch = slider.target.value * 55 + 40
    console.log(newPitch)
    synth.frequency.value = newPitch
    this.setState({
      synth: synth
    })
  }

This is an example of how I'm changing pitch. Think this approach is ok?

jsm13 commented 7 years ago

This still qualifies as mutating state. I'm not sure this will cause a problem but it is not best practice. The reason this is still mutating state is because in JS, collections are passed by reference so the line let synth = this.state.synth is rather than copying state.synth just pointing to it (ie this.state.synth === synth, they are the same object).

jsm13 commented 7 years ago

Below, a nested object is assigned to a variable, nestedObj. On the third line nestedObj is mutated (has a new value assigned to its property nestedProperty). On the fourth line when we log the original object, we see it has changed too

let objWithObjProperty = { topLevelProperty: { nestedProperty: 'foo' } }
let nestedObj = objWithObjProperty.topLevelProperty
nestedObj.nestedProperty = 'bar'
console.log(objWithObjProperty) // => { topLevelProperty: { nestedProperty: 'bar' } }
jsm13 commented 7 years ago

Really good article on the topic here (with some less contrived examples)

perryf commented 7 years ago

oh crud...yeah I see what you mean. My way works but it goes against the React way and immutable states. I'll try changing over to the Object.assigns method. Two questions if you don't mind... -What are the benefits to using states with React if we don't use Redux? I feel like I'm spending a lot of time making this synth object play nice with the React States.
-Will Object.assign method still work ok for deeply nested properties?

// state was { car: { make: 'GMC', model: 'Suburban', colors: {outside: 'yellow', inside: 'tan'}}, foo: 1 }
this.setState(state => {
  return {
    car: Object.assign({}, state.car, { make: 'Chevy' }, { outside: 'blue' }),
    bar: 2
  }
})
// In the next render...
this.state // => { car: { make: 'Chevy', model: 'Suburban', color: {outside: 'blue', inside: 'tan'}}, foo: 1, bar: 2} ???

Lastly, why does bar become part of the car object?

perryf commented 7 years ago

oops, ok will check out the article, thank you!

jsm13 commented 7 years ago

I'll follow up on the first question but the second and third are quick so I'm going to answers those first.

  1. Object.assign also does a shallow merge, merging nested objects will need to be done explicitly.

  2. bar is a sibling property to car. State after the set state is:

{
  car: {
    make: 'Chevy',
    model: 'Suburban'
  },
  foo: 1,
  bar: 2
}

Foo and bar are just examples of props that don't need special treatment when setting state

jsm13 commented 7 years ago

The reason we'd want to keep state in react is so that our components are declarative. Everything about how our component renders is dependent exclusively on the components state, props and render method. This is one of the common themes of front end frameworks, we want to keep state in exactly one place so that we don't have to worry about how things will work when the DOM is in various states (as we do with jQuery)

perryf commented 7 years ago

ooooh ok. I misinterpreted the car example. I think that all makes sense now. Long story short, I'll use the Object.assign method instead of the way I was doing it before-and set the deeply nested properties explicitly. Thank you so much for taking the time to answer everything!

jsm13 commented 7 years ago

Sounds good and sure thing! I'm going to go ahead and close this issue