CODEWITHSUNDEEP
reactjsreduxreact-redux
SuperSimplePimpleDimple
SuperSimplePimpleDimple

Reputation: 566

Which is better practice for setting state in react redux

I am learning Redux. I am kind of new so I dont want to develop a bad habit at the start which is why i am writing this.

The doc i am reading says that in the reducers we need to always set the state where we change it - in other words do the following:

const someReducer = (state=initState, {type, payload}) => {
    switch(type) {
        case 'CHANGE_VALUE_ONE' : {
            state = {...state, valueOne: payload }
            break
        }
        case 'CHANGE_VALUE_TWO' : {
            state = {...state, valueTwo: payload }
            break
        }
        default:
            state = state
    }
    return state
}

My approach was creating a newState constant which is identical to the state we receive, make change to the dummy and return it - or in code:

const userReducer = (state= {name: null, age: null}, {type, payload}) => {
    const newState = {...state}
    switch(type) {
        case 'CHANGE_VALUE_ONE' : {
            newState.valueOne= payload
            break
        }

        case 'CHANGE_VALUE_TWO' : {
            newState.valueTwo= payload
            break
        }
    }

    return newState
}

I think the way i do it is cleaner - if i have a complicated state i can easily change what i need to change without mutating the original state.

Upvotes: 3

Views: 239

Answers (4)

Mateus Stanki
Mateus Stanki

Reputation: 98

Your solution isn't immutable, the doc's one is. In more complex cases, I recommend to you use immer, the lib approach is similar to yours, but you will mutate a copy of the state and then immer will merge this copy with the actual state to create a new one. Helps you to work with nested data and bring immutability to reducers. Your case, with immer: 

const userReducer = produce(state, draft => {
  switch(type) {
    case 'CHANGE_VALUE_ONE' : {
      draft.valueOne= payload
      break
    }

    case 'CHANGE_VALUE_TWO' : {
      draft.valueTwo= payload
      break
    }
  }
},{name: null, age: null})

Upvotes: 1

Anthony Liu
Anthony Liu

Reputation: 322

Always keep the state immutable, use pure function. And there is a performance issue for your approach.

Your approach is immutable. But remember, any dispatched action will go through all reducers. That means even the action is not related to your reducer, you still create a new shallow copy of it. It's not necessary. Don't do this

// This newState will be created by every action
    const newState = {...state}
    switch(type) {
        case 'CHANGE_VALUE_ONE' : {
            newState.valueOne= payload
            break
        }

        case 'CHANGE_VALUE_TWO' : {
            newState.valueTwo= payload
            break
        }
    }

    return newState

Upvotes: 1

Mayank Shukla
Mayank Shukla

Reputation: 104539

With first snippet, you are violating the principle of pure reducer. That means instead of updating the value directly in state, create a new copy of it and do the changes in that.

As per Doc:

Reducers are just pure functions that take the previous state and an action, and return the next state. Remember to return new state objects, instead of mutating the previous state.

Which you are clearly violating by assigning new value to state, here:

state = {...state, valueOne: payload }

Note: Don't forget const newState = {...state}, will only create the shallow copy, so if you have nested state, this will also violate the principle.

In case of nested state, update it in this way:

var obj = { car: { model: '', num: '' } };

function update() {
  return {
    ...obj,
    car: {
      ...obj.car,
      model: 'abc'
    }
  }
}

console.log('new obj = ', update());

console.log('old obj = ', obj);

Upvotes: 8

user7143559
user7143559

Reputation:

I think you overcomplicate yourself. Here's a reducer that I use (as an example):

import * as types from '../actions/types';
import { Enums } from '../helpers/enums';

const initialState = {
  testState: Enums.command.STOP
};

export default function(state = initialState, action) {
  switch (action.type) {
    case types.SET_CREEP_TEST_STATE:
      return {...state, testState: action.testState};
    case types.SET_CREEP_TEST_TIME:
      return {...state, testime: action.testime};
    default: return state;
  }
};

It shouldn't be any more complex than that.

Upvotes: 2

Related Questions