CODEWITHSUNDEEP
javascriptreactjsredux
David
David

Reputation: 1237

Is this Redux reducer example from the Pro React 16 book kosher?

I'm learning React and Redux, and I'm currently reading Pro React 16 by Adam Freeman. Consider this reducer example from Chapter 5. This reducer handles actions that modify a shopping cart. Here's part of the reducer:

import { ActionTypes } from "./Types";

export const CartReducer = (storeData, action) => {
    let newStore = { cart: [], cartItems: 0, cartPrice: 0, ...storeData }
    switch(action.type) {
        case ActionTypes.CART_ADD: 
            const p = action.payload.product;
            const q = action.payload.quantity;

            let existing = newStore.cart.find(item => item.product.id === p.id);
            if (existing) {
                existing.quantity += q;
            } else {
                newStore.cart = [...newStore.cart, action.payload];
            }
            newStore.cartItems += q;
            newStore.cartPrice += p.price * q;            
            return newStore;

My understanding is that reducers must be pure functions, but this one seems to modify the store argument storeData when the product already exists in the cart array. Specifically, it updates the quantity property of the existing cart item, which comes from a shallow copy of storeData's cart array. Thus, storeData will be modified as a side effect.

Am I correct?

Upvotes: 2

Views: 188

Answers (3)

Mselmi Ali
Mselmi Ali

Reputation: 1257

I think the right way of creating a reducer and to return the new state is like below:

export default CartReducer = (storeData = [], action) => {
    switch(action.type) {
       case ActionTypes.CART_ADD: 
           return [..state, { ..action.cart }]
       default:
           return storeData;
    }
}

This way you create a new copy of your state and simply return the new state, you need also keep your reducer simple and avoid doing extra stuff like the logic of adding/updating the shopping cart in the reducer code.

You need also to create a default action and return untouched state.

Upvotes: 0

xadm
xadm

Reputation: 8418

This is not a side effect. I would call it a mutation of element used for new state creation.

This reducer is pure is not pure but IMHO is good enough (when you know pros/cons) :

  • from the app perspective it does the job
  • returns the same output for the same input (replayable action sequence)

Reducer role is to create a new state from current state and action. The old state will no longer be used - this mutation doesn't matter. There is no requirement/restriction to leave the old state untouched.

CONS: it can affect redux dev tools, undoing action can work incorectly (replay actions only in one direction - use redux-undo for real undo)

PROS: simplicity, smaller memory usage

There is a shopping cart example in redux - different (more flat) data structure.

Upvotes: 0

DannyMoshe
DannyMoshe

Reputation: 6255

I think you are correct.

Not only are you mutating the state (as you pointed out), but you are also returning this mutated object.

I believe you should only use the 'set' method from Immutable JS to update state in a reducer.

Upvotes: 2

Related Questions