2

I have the following initial state that consists of flat properties and one deep nested property searchStatus:

var searchStatus = Immutable.fromJS({
    requesting: {
        component: {tenants: false, platforms: false},
        tenant: false,
        hdf: false,
        cluster: false
    }
});

const initialState = {
    selectedItem: null,
    searchQuery: '',
    searchStatus: searchStatus
};

I have a reducer that works with this state:

function reducer(state = initialState, action) {
        switch (action.type) {
            case GET_TENANT_TEMPLATES_LISTING_REQUEST:
                var status = state.searchStatus.updateIn(['requesting', 'component', 'tenants'], function () {
                    return true;
                });
                return assign({}, state, {
                    searchStatus: status
                });

Is it OK to return new copy only for searchStatus part of the state and then merge it into the state or should I always return the entire state copy?

UPDATE:

case GET_TENANT_TEMPLATES_LISTING_REQUEST:
    var copy = assign({}, state);
    copy.searchStatus.requesting.component.tenants = true;
    return copy;
Max Koretskyi
  • 85,840
  • 48
  • 270
  • 414

1 Answers1

2

You must copy the state object over every time (note that internal references can be kept, so it's not as inefficient as you may think), which is what you're doing with your assign (which I can only assume is a shorthand for Object.assign())

Your call to assign will copy over all of the properties from state, and then from { searchStatus: status }, to an empty object, effectively copying over your entire state, and then applying changes.

You must not mutate the state passed into the function, and you must also return a complete state object from the function.

Madara's Ghost
  • 158,961
  • 49
  • 244
  • 292
  • thanks, if internal references remain the same, is the approach I specified in my question's update possible? – Max Koretskyi Dec 29 '15 at 08:41
  • No. Because that will mutate the original state object as well, as the references remain the same. Stick with `Object.assign()` – Madara's Ghost Dec 29 '15 at 08:44
  • for some reason I can't see the difference between the two. Can you please provide a bit elaborate explanation? – Max Koretskyi Dec 29 '15 at 09:02
  • `Object.assign()` will replace the entire reference with a different reference, your **UPDATE:** approach will change something within the reference, which is shared with the original object, thus changing the original object. – Madara's Ghost Dec 29 '15 at 09:03
  • Thanks, but my second approach also uses Object.assign, and the mutation is then made in a cloned state. Why is this wrong? In my first approach I also make new copy of searchStatus, is that whats missing in the 2 approach? – Max Koretskyi Dec 29 '15 at 11:09
  • 1
    @Maximus The mutation doesn't happen on a cloned object, it happens in one of the references which are on the cloned object. But the object in that reference is not cloned, it's shared between the clone and the original. So modifying it will change both. See https://jsfiddle.net/pwwttfv6/ – Madara's Ghost Dec 29 '15 at 11:24
  • I think I now understand, thanks! One more question though, for detecting changes it's enough to change a reference to a state object, why is there a requirement to also change references to inner objects so that inner state of an object doesnt change? Does it have something to do with structural sharing mechanism? – Max Koretskyi Dec 29 '15 at 11:33
  • Can you please take a look at my these two questions if you have time: [this](http://stackoverflow.com/questions/34516484/what-is-the-correct-way-to-implement-transactions-with-redux) and [this](http://stackoverflow.com/questions/34516894/why-is-the-requirement-to-always-return-new-object-with-new-internal-references) - both pertaining to `redux` – Max Koretskyi Dec 29 '15 at 18:33