Is it a bad practice to set property of object, nested in another object by directly setting it on d-CodePudding

I've found an article which states that if i want to change property name in such state:

const [user, setUser] = useState({
    name: 'Cody',
    age: 25,
    education: {
        school: {
          name: 'School of Code'
        }
    }
})

i need to do following:

setUser(prevUser => {
    return {
        ...prevUser,
        education: {
            ...prevUser.education,
            school : {
                ...prevUser.education.school,
                name: 'Layercode Academy'
            }
        }
    }
})

Howewer, they later show that it is possible to make this logic simpler, using immer.js (also changing useState on useImmer), like this:

setUser(draft => {
    draft.education.school.name = 'Layercode Academy';
})

My question is whether i can do this, without using immer.js:

setUser(prevUser => {
    const newUser = {...prevUser}
    newUser.education.school.name = 'Layercode Academy'
    return newUser
})

In every tutorial i've seen (that doesn't use immer.js), they do destructuring. But just assigning value to property of state copy seems simpler and more concise for me in many situations. I am not setting state directly, but rather just modify copy, which is not breaking any "rules" . Are there some hidden pitfalls?

CodePudding user response：

Here is the difference between the copies

SHALLOW COPY

const a = { x: 0, y: { z: 0 } };
const b = {...a}; // or const b = Object.assign({}, a);

b.x = 1; // doesn't update a.x
b.y.z = 1; // also updates a.y.z

DEEP COPY

const a = { x: 0, y: { z: 0 } };
const b = JSON.parse(JSON.stringify(a)); 

b.y.z = 1; // doesn't update a.y.z

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. - Dan

So with this in mind, you don't have to deep clone all things as it can be very expensive and cause unnecessary renders.

so if you have to do something with the data after the render

setUser(prevUser => {
    return {
        ...prevUser,
        education: {
            ...prevUser.education,
            school : {
                ...prevUser.education.school,
                name: 'Layercode Academy'
            }
        }
    }
})

doSomething(user) // setUser merely schedules a state change, so your 'user' state may still have old value
}

Instead,

setUser(prevUser => {
    return {
        ...prevUser,
        education: {
            ...prevUser.education,
            school : {
                ...prevUser.education.school,
                name: 'Layercode Academy'
            }
        }
    }
}, (user) => { doSomething(user})

This way you guarantee the update of the user and don't have to worry about the mutation.

CodePudding user response：

Are there some hidden pitfalls?

Yes, though we can't know if the rest of your code will experience them. So in general just consider it as a blanket "yes".

What you're doing is mutating state. In cases where the entire operation is just performing one state update and re-rendering, you're unlikely to experience a problem. But it's still building upon and relying upon a bad habit.

In cases where more operations are being performed (multiple batched state updates, other logic using current state before state updates are processed, etc.) then you're much more likely to see unexpected (and difficult to trace) bugs and behaviors.

The overall rule of "don't mutate state" is just that, an overall rule. There are indeed cases where one can mutate state without causing any problems. But why? Why rely on bad habits solely because a problem hasn't occurred yet?

A state update should be a wholly new object (or array, or just plain value). For object properties which don't change, it can still be a reference to the previous object property (which deconstructing will do). But for any property which does change, the entire object graph which leads to that property should be new.

CodePudding user response：

Yep, there is something wrong with your last example. You're still modifying the original user's education object:

setUser(prevUser => {
    const newUser = {...prevUser}
    newUser.education.school.name = 'Layercode Academy'

    // true
    prevUser.education.school.name === newUser.education.school.name

    return newUser;
});

You may consider breaking up state to multiple states (that are primitives):

const [userEducationSchoolName, setUserEducationSchoolName] = useState("School of Code");

but this gets out of hand very quickly. If your environment supports it, you may even use structuredClone:

setUser(prevUser => {
    const newUser = structuredClone(prevUser);
    newUser.education.school.name = 'Layercode Academy'
    return newUser
});

but since some browsers don't support structuredClone yet, you would need some other way to deep clone an object. And this brings us back to Immer! The whole point of Immer is to give you a mutable deep copy of the state for you to change.