Search code examples
react-reduxmapstatetoprops

Redux, MapStateToProps/useSelector: Is it safe to mutate those values

Both MapStateToProps and useSelector work with similiar callback store => store.group Is it safe to mutate those values for example in MapStateToProps like this:

const mapStateToProps = store => {
  const { group, level } = store;
  let { group } = store;
  if (level > 50) {
    group = `${group}-admin`;
  }
  return { group };
};

or in useSelector:

const group = useSelector(store => {
  const { group, level } = store;
  let { group } = store;
  if (level > 50) {
    group = `${group}-admin`;
  }
  return { group };
});

And with useSelector it could actually be done inside the component too like this:

let [group, level] = useSelector(store => [store.group, store.level);
if (level > 50) {
  group = `${group}-admin`;
}
...

My co-worker did something like this and I'm not sure if you are supposed to use let in there. I'm just interested if that is acceptable way to deal with this or if that causes problems? I don't need a different solution for it. I know how to do it with using constinstead.

Solution

  • One of the roles of redux is to centralize your state logic:

    From: https://redux.js.org/introduction/getting-started

    The whole state of your app is stored in an object tree inside a single store. The only way to change the state tree is to emit an action, an object describing what happened. To specify how the actions transform the state tree, you write pure reducers.

    So when you do something like this:

    const group = useSelector(store => {
  const { group, level } = store;
  let { group } = store;
  if (level > 50) {
    group = `${group}-admin`;
  }
  return { group };
});

    This should be considered bad practice. The useSelector hook should return a piece of your state without mutating it. You are mutating it inside the useSelector call, so you are returning something that maybe not in the state object.

    From node_modules/react-redux/src/hooks/useSelector.js:

    enter image description here

    See that they even name the result of selectedState = selector(storeState) as selectedState. And you are not selecting a state on your call. You are mutating the result.

    While in your example you are just mutating a string that you read from the state, this is a bad practice because one day you might mutate the state object without mutating it through an action/reducer dispatch. Which would definitely break your code at some point.

    For example:

    const group = useSelector((state) => {
  let { someObject } = state;
  if (someCondition) {
    someObject.someProperty = someValue  // YOU ARE MUTATING STATE OUTSIDE A REDUX REDUCER
  }
  return someObject;
});

    What I think you should do:

    SomeComponent.js

    const { group, level } = useSelector((state) => state);
const newGroup = level > 50 ? `${group}-admin` : group;

    If what you want instead is to update the group state property on Redux, you should instead dispatch an action to be handled by a reducer.