JS spread operator workflow on React

Question

React suggests not to mutate state. I have an array of objects which I am manipulating based on some events. My question is, is it okay to write it like this:

const makeCopy = (arr) => arr.map((item) => ({ ...item }));

function SomeComponenet() {
    const [filters, setFilters] = useState(aemFilterData);

    const handleFilterClick = (filter, c) => {
        let copiedFilters = makeCopy(filters);

        /**
         * Apply toggle on the parent as well
         */
        if (!("parentId" in filter)) {
            copiedFilters[filter.id].open = !copiedFilters[filter.id].open;
        }
        setFilters(copiedFilters);
    }
}

Am I mutating the original object by doing like above? Or does it make a difference if written like this:

const makeCopy = (arr) => arr.map((item) => ({ ...item }));

function SomeComponent() {
    const [filters, setFilters] = useState(aemFilterData);

    const handleFilterClick = (filter, c) => {
        let copiedFilters = makeCopy(filters);

        /**
         * Apply toggle on the parent as well
         */
        if (!("parentId" in filter)) {
            copiedFilters = copiedFilters.map((f) => {
            if (filter.id === f.id) {
              return {
                ...f,
                open: !f.open,
              };
            } else {
              return { ...f };
            }
          });
        }
        setFilters(copiedFilters);
    }
}

What's the preferred way to do this? Spread operators are getting a lot verbose and I am not liking it, but I prefer it if that's how I need to do it here. immutable.js and immer or not an option right now.

Use JSON for deep copy --> https://stackoverflow.com/a/47690512/12509535 — Siddharth Seth, Dec 07 '21 at 05:47
Would you still recommend the same for the not-so-nested array? Check this gist: https://gist.github.com/thomsebastin/ddbeb878d959cf614ce520a85d25d233 — Thomas Sebastian, Dec 07 '21 at 05:57
@AbdennourTOUMI So is it safe to assume that the first way is okay? — Thomas Sebastian, Dec 07 '21 at 06:06
The childIds array won't be cloned using spread operator and would be susceptible to changes from both objects as both would share the same...https://jsfiddle.net/aejgupLs/ — Siddharth Seth, Dec 07 '21 at 06:10
@ThomasSebastian yes, 1st approach is enough. i put detailed answer below. Good luck! — Abdennour TOUMI, Dec 07 '21 at 06:16
@SiddharthS. Luckily, the "childIds" is read-only throughout the lifecycle of this component since there is no removal/reordering of children anywhere in the requirement. But deep cloning might help keep things clean, so it is indeed an option worth considering if things get out of hand. — Thomas Sebastian, Dec 07 '21 at 06:16
@ThomasSebastian React does not require you to make a deep clone with the new `useState` hook. The only thing that needs to change when you update the state is the object identity of the state object itself. Because React only compares that with the previous one to decide if it must re-render. So a shallow `const newFilters = [...filters];` is enough for most use cases. However, if you take out inner parts of the state to feed a dependency array somewhere you would need a fresh identity: `useEffect(some, [filters[0]])` would require a fresh identity. Does this clear things up? — Martin, Dec 07 '21 at 07:12
The old class rules for `setState()` don't apply for `useState()`. — Martin, Dec 07 '21 at 07:13
@Martin So, as long as I am not using some nested item within the state object as a dependency, it won't be an issue? Is my understanding correct? Does this mean I can safely consider the first approach above? — Thomas Sebastian, Dec 07 '21 at 08:35
`as long as I am not using some nested item within the state object as a dependency, it won't be an issue?` Yes, correct, it won't be an issue. `Is my understanding correct?` Seems to be the case. `Does this mean I can safely consider the first approach above?` Yes, actually your first approach is already an overkill. You even cloned all your items in the array. It would be enough to just clone the array, hence doing this `const copiedFilters = [...filters]` is sufficient. No need for your `makeCopy` function. — Martin, Dec 07 '21 at 09:00
[Here](https://stackoverflow.com/questions/56296041/react-updating-state-array-of-objects-do-i-need-to-deep-copy-the-array) is a question that covers your use-case. See the accepted answer. — Martin, Dec 07 '21 at 09:03

vatz88 · Accepted Answer · 2021-12-07T06:48:32.540

1

const makeCopy = (arr) => arr.map((item) => item );

With above code, it's mutating on the original object reference because we're not creating a deep clone.

copiedFilters[filter.id].open = !copiedFilters[filter.id].open;

Here reference of copiedFilters[filter.id] and filters[filter.id] is same.

With spread operator

const makeCopy = (arr) => arr.map((item) => ({ ...item }));

Here we create a new copy of the inner object too. So copiedFilters[filter.id] and filters[filter.id] will have different reference.

This is same as your second approach.

So either you use spread operator while making a copy or you can skip making a copy in the second approach and directly map on filters since you're using spread operator there. This looks better because, why run loop twice - first to create copy and then to update open.

// let copiedFilters = makeCopy(filters); Not needed in second approach
copiedFilters = copiedFilters.map((f) => {
  if (filter.id === f.id) {
    return {
      ...f,
      open: !f.open,
    };
  } else {
    return { ...f };
  }
});

You can create a deep clone when you copy but that would be waste of computation and memory, I don't think it's needed here. Deep clone is helpful when you have further nesting in the object.

edited Dec 07 '21 at 06:48

answered Dec 07 '21 at 06:07

vatz88

2,422
2
14
25

I think I will stick to the second approach based on your advice then; Considering, I have events in other places and don't want to deep clone everywhere if that is less performant. – Thomas Sebastian Dec 07 '21 at 06:14
My code is full of such map and spread operations. I was wondering if I could deep-clone and simply modify them directly. Will there be a huge impact on performance or is it an okay trade-off to make considering readability will improve a lot. – Thomas Sebastian Dec 07 '21 at 06:23
1

You'll have to measure the performance impact. In most cases you'll not see any difference but theoretically deepclone should be expensive. Libraries like ImmutableJS might do a better job at it though. – vatz88 Dec 07 '21 at 06:26
In your answer, you said the reference of `copiedFilters[filter.id] and filters[filter.id] is same`. Could you check this fiddle, because I noticed that they are different? Since there is no further nesting, I feel it works. https://jsfiddle.net/vcnd7woj/ – Thomas Sebastian Dec 07 '21 at 06:33
@ThomasSebastian In the fiddle you're using spread operator but instead if you had `arr.map((item) => item );` then the ref would be same – vatz88 Dec 07 '21 at 06:39
My bad, in your code, in first approach you're using spread operator to it is creating new reference but if you go further deep in the object level, it'd be same references – vatz88 Dec 07 '21 at 06:41
@ThomasSebastian updated the answer – vatz88 Dec 07 '21 at 06:48
Thanks and appreciate your effort. I will keep things as is now. But good to have an understanding of all these gotchas. I'll probably end up using immer/immutable.js etc... – Thomas Sebastian Dec 07 '21 at 07:03

JS spread operator workflow on React

1 Answers1