reselect modifying redux state

[reggieofarrell]

I'm using reselect with redux and react (with hooks). Having some strange behavior where a selector I'm creating with createSelector is actually modifying my redux state.

reselect: 4.0.0 redux: 4.0.5 react-redux: 7.0.2 react: 16.13.1

This code is in a file outside of the react component...

import { createSelector } from 'reselect';
import { keyBy } from 'lodash';

export const breakoutsSessionSelector = (state) => state.firestore.data.breakouts || {};

export const getRoomMap = createSelector(
  [breakoutsSessionSelector],
  (breakouts) => {
    if (!breakouts?.participantRoomAssignments.length) return null;

    const breakoutRooms = keyBy(breakouts.rooms, 'id');

    breakouts.participantRoomAssignments.forEach(rmAss => {
      if (!breakoutRooms[rmAss.roomId].participants) {
        breakoutRooms[rmAss.roomId].participants = [];
      }
      breakoutRooms[rmAss.roomId].participants.push(rmAss.participantId);
    });

    return breakoutRooms;
  }
);

The data structure of state.firestore.data.breakouts in redux is...

breakouts: {
  breakoutStarted: true,
  id: 'YGUoUP8eMpnswO5T9MQH',
  participantRoomAssignments: [
    {
      participantId: 'NqZ5FqvAq7XZrpmfZb9uinzSSwE3',
      roomId: 1594222672714
    },
    {
      participantId: 'yfm0FZLIZTeZ86cMJrHyiXZHJ5H2',
      roomId: 1594421903409
    },
    ...
  ],
  rooms: [
    {
      description: '',
      id: 1594222672714,
      title: 'Breakout 1',
    },
    {
      description: '',
      id: 1594327531991,
      title: 'Breakout 2',
    },
    ...
  ],
  topic: {
    description: '',
    name: 'the name'
  }
}

state.firestore.data.breakouts stores data about what room meeting participants are in. The getRoomMap selector's purpose is to create a map of the breakout rooms with the participants in an array inside of the rooms like so...

{
  "1594222672714": {
    "description": "",
    "id": 1594222672714,
    "title": "Breakout 1",
    "participants": [
      "NqZ5FqvAq7XZrpmfZb9uinzSSwE3",
      "ywVEHre55JMsbzrC84ah2hB5Y3P2",
      "dcc9o6GOSKR3ZoY7HvCCLTDDbHz1"
    ]
  },
  ...
}

This works just fine. The odd thing that I can't figure out is why when I call const roomMap = useSelector(getRoomMap) inside one of my react components, it actually modifies state.firestore.data.breakouts such that the rooms array also has the array of participant ids in it like so...

rooms: [
    {
      description: '',
      id: 1594222672714,
      title: 'Breakout 1',
      participants: [
        'NqZ5FqvAq7XZrpmfZb9uinzSSwE3',
        'ywVEHre55JMsbzrC84ah2hB5Y3P2',
        'dcc9o6GOSKR3ZoY7HvCCLTDDbHz1'
      ]
    },
    {
      description: '',
      id: 1594327531991,
      title: 'Breakout 2',
      participants: [
        'a2sKe1ixOOYbrwBKOmFHGocPI7U2',
        'X0M5TEBTgPMvTCVRbCCGPlDFgIA2',
        'dxoJEByX94hl3zSD7yabsZPfR7J3'
      ]
    },
    ...
  ],

Is this normal? It's not breaking anything, but it is very odd. Why is creating this selector modifying my redux state?

[markerikson]

Reselect isn't doing anything - your code is mutating the existing data:

breakouts.participantRoomAssignments.forEach(rmAss => {
      if (!breakoutRooms[rmAss.roomId].participants) {
        breakoutRooms[rmAss.roomId].participants = [];
      }
      breakoutRooms[rmAss.roomId].participants.push(rmAss.participantId);
    });

Those objects are still the original reference, not copies.

Don't do that :) You'll probably want to make copies here and return those instead.

[reggieofarrell]

Thanks for the quick reply @markerikson. I actually assumed that was the case initially, but I tried this and I get the same results...

export const getRoomMap = createSelector(
  [breakoutsSessionSelector],
  (breakouts) => {
    const breakoutsCopy = {...breakouts};

    if (!breakoutsCopy?.participantRoomAssignments.length) return null;

    const breakoutRooms = keyBy(breakoutsCopy.rooms, 'id');

    breakoutsCopy.participantRoomAssignments.forEach(rmAss => {
      if (!breakoutRooms[rmAss.roomId].participants) {
        breakoutRooms[rmAss.roomId].participants = [];
      }
      breakoutRooms[rmAss.roomId].participants.push(rmAss.participantId);
    });

    return breakoutRooms;
  }
);

...which is why I was scratching my head. But I just remembered that you can't deep clone objects with the spread operator 🤦🏻‍♂️. lodash's cloneDeep to the rescue. It's working properly now. Thanks! Working code...

import { keyBy, cloneDeep } from 'lodash';

export const breakoutsSessionSelector = (state) => state.firestore.data.breakouts || {};

export const getRoomMap = createSelector(
  [breakoutsSessionSelector],
  (breakouts) => {
    const breakoutsCopy = cloneDeep(breakouts);

    if (!breakoutsCopy?.participantRoomAssignments.length) return null;

    const breakoutRooms = keyBy(breakoutsCopy.rooms, 'id');

    breakoutsCopy.participantRoomAssignments.forEach(rmAss => {
      if (!breakoutRooms[rmAss.roomId].participants) {
        breakoutRooms[rmAss.roomId].participants = [];
      }
      breakoutRooms[rmAss.roomId].participants.push(rmAss.participantId);
    });

    return breakoutRooms;
  }
); 

[markerikson]

Yep, the spread operator is a shallow clone, which is a common mistake:

https://redux.js.org/recipes/structuring-reducers/immutable-update-patterns#common-mistake-2-only-making-a-shallow-copy-of-one-level

You might want to consider using https://immerjs.github.io/immer/docs/introduction to let you write "mutating" logic that get safely turned into immutable updates.

Note that we also use Immer internally in our official Redux Toolkit package:

https://redux-toolkit.js.org