Why the original data is mutated in this simple task?

Question

I know that data is immutable in redux and always should be, but i am faced with a weird problem may be i am not seeing the big picture.

This simple app just has a select box that change cats images, clicking a cat image should increment number of clicks below it so each cat should have its own number of clicks.

The app works very well but i do not understand something here:

In imageContainer component, logging props.cats is the same with logging the original data (cats). Both returns the modified cats objects with modified clickCount that is incremented after clicking the images.

I can not understand how the original data was affected in this process despite i am returning new data every time by reducers without mutating the state!!

My initial data:

import images from './images';
const cats =  [
  {
    name: 'Lucy',
    src: images.cat1,
    clickCount: 0
  },
  {
    name: 'Julia',
    src: images.cat2,
    clickCount: 0,
  },
  {
    name: 'Kattie',
    src: images.cat3,
    clickCount: 0,
  },
  {
    name: 'Hend',
    src: images.cat4,
    clickCount: 0,
  }
];

 export default cats;

First reducer:

import cats from '../cats'

export default (state = cats, action) => {
  switch(action.type) {
    case 'INCREMENT':
      return cats.map(cat => cat.name === action.name ? {...cat, clickCount: ++cat.clickCount} : cat);

    default:
      return state;
  }
}

Second reducer:

import cats from '../cats'

export default (state = cats[0], action) => {
  switch(action.type) {
    case 'SET_CURRENT_CAT':
      return cats.filter(cat => cat.name === action.name)[0];
    default: 
      return state;
  }
}

My components:

First Component:

import React, { Component } from 'react';
import { connect } from 'react-redux';
import cats from '../cats';

const ImageContainer  = props =>{
  const {currentCat, incrementClicks} = props;
  console.log(props.cats)//cats with modified clickCount
   console.log(cats)     // cats with modified clickCounts
  return(
   <div className="img-cont">
     <img 
       src={currentCat.src}
       className="cat-img"
       onClick={() => incrementClicks(currentCat.name)}
       alt='cat_img'
     />
     <div className="clicks-count">
       <p>{currentCat.clickCount} clicks</p>
     </div>
   </div>
  );

}

const mapStateToProps = (state) => {
 return {
   cats: state.cats,
   currentCat: state.currentCat,
 }
}

const mapDispatchToProps = (dispatch) => {
  return {
    incrementClicks: (name) => dispatch({
      type: 'INCREMENT',
      name
    })
  };
}


export default connect(mapStateToProps, mapDispatchToProps)(ImageContainer);

second component:

import React, { Component } from 'react';
import cats from '../cats';
import { connect } from 'react-redux';

const CatChanger = (props) => {

  return(
    <div className="cat-changer">
      <select onChange={(e) => {props.setCurrentCat(e.target.value)} }>    

        {cats.map((cat,i) => (
          <option value={cat.name} key={i}>{cat.name}</option>
        ))}
      </select>
    </div>
  );

}


const mapDispatchToProps = (dispatch) => {
  return {
    setCurrentCat: (name) => dispatch({
      type: 'SET_CURRENT_CAT',
      name
    })
  };
}

export default connect(null, mapDispatchToProps)(CatChanger);

So why the original data is mutated? or it is not??

Answer 1

In the second reducer, you're getting the data from the cats import, instead of from the state! So your reduce is always returning an object from the import instead of from whatever state is passed in. You should not be importing cats in your reducer code.

Similarly in your first reducer, you are returning data from the cats import.

In both cases, you should be returning data from the state parameter passed in, not from the cats import.

Your first reducer would then look like this:

export default (state = cats, action) => {
  switch(action.type) {
    case 'INCREMENT':
      return state.map(cat => cat.name === action.name ? {...cat, clickCount: cat.clickCount + 1} : {...cat});

    default:
      return state;
  }
}

Second reducer:

export default (state = cats[0], action) => {
  switch(action.type) {
    case 'SET_CURRENT_CAT':
      return {...state.filter(cat => cat.name === action.name)[0]};
    default: 
      return state;
  }
}

Note that the reducer body code references state, not cats. Also note that the reducers now return new objects that are clones of the objects in state (NOT the original objects), using the object spread operator: {...cat} and {...state.filter()[0]}.

It's not necessarily a problem to import cats and use it as the default, though it's not the best design. (The problem is that the code isn't using the state parameter at all, and is instead using the imported object collection.) To avoid using the cats import in the reducers entirely, one option is to pass them in as the default store contents to createStore:

import cats from '../cats';
const defaultState = { cats, currentCatName: 'Lucy' };

const store = createStore(
  rootReducer,
  defaultState
);

Then the defaults for the reducer parameters should return an empty array for the first reducer:

export default (state = [], action) => {

and an empty object for the second reducer:

export default (state = {}, action) => {

Another alternative is to add a LOAD_CATS action that populates your cat collection, perhaps dispatched on componentDidMount for your top level app component.

Another optimization that will help avoid using existing objects is to change SET_CURRENT_CAT to simply change the cat name/id stored in the state:

export default (state, action) => {
      switch(action.type) {
        case 'SET_CURRENT_CAT':
          return action.name;

Then to get the current cat, you can add a selector that gets the cat by name/id from the state returned by the first reducer. (Adding an id field helps you handle the scenario where there are multiple cats with the same name.) The selector might look like this:

function getCatByName(state, name) { return state.cats.filter(cat => cat.name === name) }

Answer 2

You're mutating state when you use the ++ operator on it. Use a simple addition instead

 return cats.map(cat => cat.name === action.name ? {...cat, clickCount: cat.clickCount + 1} : cat);