1

Both AddToCart and RemoveFromCart actions update num and qty, while both HandleClick and InputQuantity actions update qty.

I have problem with these two lines of code:

num: productsReducer(state.productsReducer.num, AddToCart)

qty: productsReducer(state.productsReducer.qty, AddToCart)

where prop num or qty is only updated by AddToCart action.

The other 3 actions - RemoveFromCart, HandleClick, InputQuantity - do not update prop num or qty. And I have no idea how to allow the above actions update prop num and qty.

How to write productsReducer or my actions such that num and qty are both updated by AddToCart and RemoveFromCart, while qty is updated by HandleClick and InputQuantity?

Do I need a higher-order reducer to handle multiple actions? Please help.

CartContainer

// Both AddToCart and RemoveFromCart actions update num and qty
// Both HandleClick and InputQuantity actions update qty

const mapStateToProps = state => ({
  num: productsReducer(state.productsReducer.num, AddToCart),
  qty: productsReducer(state.productsReducer.qty, AddToCart),
  products: state.productsReducer.products
});

ProductsReducer

import * as actionTypes from '../actions/actions';

const initialState = {
  products: [],
  num: [],
  qty: [],
  totalPrice: 0,
  status: ''
};

const productsReducer = (state = initialState, action) => {
  let updatedNum = state.num;
  let updatedQty = state.qty;
  let index;

  if (action.num !== undefined) {
    index = updatedNum.indexOf(action.num);
  }

  switch (action.type) {
    case actionTypes.FETCH_PRODUCTS_REQUEST:
      return {
        ...state,
        status: action.status
      }

    case actionTypes.FETCH_PRODUCTS_SUCCESS:
      return {
        ...state,
        status: action.status,
        products: action.products
      }

    case actionTypes.ADD_TO_CART:
      // if num array doesn't have that element, insert that element
      if (!state.num.includes(action.num)) {
        updatedNum = state.num.concat(action.num); 
        updatedQty = state.qty.concat(1);

        return {
          ...state,
          num: updatedNum,
          qty: updatedQty
        }
      // if element is present in array already, increase qty of it by 1
      } else {
        updatedQty = state.qty.slice(0);
        updatedQty[index] += 1;

        return {
          ...state,
          qty: updatedQty
        }
      }

      // After updating store, use CalculatePrice action to calculate updated price.
      this.calculateTotalPrice(updatedNum, updatedQty);

    case actionTypes.REMOVE_FROM_CART:
      // remove clicked item with splice, and update state
      updatedNum.splice(index, 1);
      updatedQty.splice(index, 1);

      return {
        ...state,
        num: updatedNum,
        qty: updatedQty
      }

    case actionTypes.HANDLE_CLICK:
      let event = action.eventType;
      console.log(event);

      if (event === 'plus') {
        updatedQty[index] += 1;
      } else if (event === 'minus') {
        updatedQty[index] -= 1;
      }

      return {
        ...state,
        qty: updatedQty 
      }

    case actionTypes.INPUT_QUANTITY:
      const regex = /^[0-9\b]+$/;

      if (regex.test(action.event.target.value)) {
        updatedQty[index] = Number(action.event.target.value);

        return {
          ...state,
          qty: updatedQty 
        }
      }

    case actionTypes.CALCULATE_PRICE:
      let arr = [];
      console.log('updatedNum', action.num);

      action.num.map(num => {
        let index = num - 1;
        arr.push(action.qty[index] * state.products[index].price);
      });

      if (arr.length > 0) {
        let total = arr.reduce((acc, currentVal) => acc + currentVal);
        return { 
          totalPrice: total 
        }
      } 

      return { 
        totalPrice: 0 
      }

    default: 
      return state;
  }
}

export default productsReducer;
jenlky
  • 376
  • 4
  • 17

2 Answers2

1

so I'll give you two answers, first one is mainly explaining the reason why only AddToChart updates the props but not the other actions, and the second one is mainly a set of suggestions that's up to you to implement

1) Why only AddToChart updates the props (the redux state)

Please find below a quote from Redux's official documentation which also applies to you

Why isn't my component re-rendering, or my mapStateToProps running? Accidentally mutating or modifying your state directly is by far the most common reason why components do not re-render after an action has been dispatched

The important part in this quote is

Accidentally mutating or modifying your state directly

Let's take a look how this applies to you

In your initialState you have the following values

const initialState = {
  num: [],
  qty: []
};

num and qty fields are arrays.

At the beginning of your productReducers, you also have the following line

let updatedNum = state.num;
let updatedQty = state.qty;
let index;

Before going further, let's recall what is the special thing with array in javascript?

Please find a quote from the post https://www.dyn-web.com/javascript/arrays/value-vs-reference.php

Perhaps you have noticed that if you assign an array to another variable and modify that variable's array elements, the original array is also modified. Or perhaps you have passed an array to a function hoping to modify only the local copy of the array, but you find that the original array has also been modified. This happens because arrays are reference types in JavaScript.

That means that if you assign an array to a variable or pass an array to a function, it is the reference to the original array that is copied or passed, not the value of the array.

So basically your updatedNum and updatedQty are not new arrays but rather reference to the original array, therefore, whenever you mutate updatedNum and updatedQty, the original array they are referenced to also updated.

In our case, the original arrays are actually the one that we have in the state, so basically whenever there is an update, the original state is actually mutated and as the first quote suggests,

accidental mutations prevent re-render after an action has been dispatched

In ADD_TO_CART

case actionTypes.ADD_TO_CART:
   ...
   updatedNum = state.num.concat(action.num); 
   updatedQty = state.qty.concat(1);

concat() method is used.

Why does it work for you?

From MDN https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/concat

The concat() method is used to merge two or more arrays. This method does not change the existing arrays, but instead returns a new array.

So there is no accidental mutation, you return a brand new array

In REMOVE_FROM_CHART method

case actionTypes.REMOVE_FROM_CART:
      updatedNum.splice(index, 1);
      updatedQty.splice(index, 1);

splice() method is used.

From MDN https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice

The splice() method changes the contents of an array by removing or replacing existing elements and/or adding new elements.

So there is a mutation because instead of returning new array, you mutate the original array that's referenced to the state

In HANDLE_CLICK and UPDATE_QUANTITY

updatedQty[index] -= 1;

By this way again you mutate the array instead of returning a brand new array, so you mutate the original array that's referenced to the state

In your current case, if you do the following update

let updatedNum = state.num.concat();
let updatedQty = state.qty.concat();
//I would suggest you to use spread operator
let updatedNum = [...state.num]
let updatedQty = [...state.qty]

Most of your initial problems would be solved

2) How to write productsReducer or my actions such that num and qty are both updated

I cannot give you specific step by step guideline for this question, because how to structure your store should be determined by you based on app's current needs and possible future improvements

However, I can make a couple of suggestions

First of all, probably I would get rid of the following section

 let updatedNum = state.num;
  let updatedQty = state.qty;
  let index;

  if (action.num !== undefined) {
    index = updatedNum.indexOf(action.num);
  }

For me, I generally handle this kind of checks and assignments inside of my action handlers in an isolated scope because when the code gets bigger, it's harder for me to go all the way up to see where do I declare those variables and second, you can never be sure whether they are mutated or not and makes things harder to debug. Also, in the future, you might need to update declaration logic of let's say updateNum for some of your action handlers. If you have this kind of declaration, you can break some part of your application without even realizing

Second of all, I believe the usage below is sort of anti-pattern

// After updating store, use CalculatePrice action to calculate updated price.
      this.calculateTotalPrice(updatedNum, updatedQty);

If your intention is to trigger this function inside of an action handler, it wouldn't work because in above lines, you already return a statement.

If your intention is to update the price after ADD_CHART, REMOVE_FROM_CHART

you can put that calculation logic inside of these action handlers, do the calculation and return the new state

For instance

...
const totalPrice = Some logic related to updatedNum and updateQty
return {
          ...state,
          num: updatedNum,
          qty: updatedQty,
          totalPrice
        }

I hope my answers will help you to design a path that's more clear for you. There are always a couple of different ways that you can create you actions, action reducers etc.

However the most important thing to remember when it comes to redux, avoid methods or designs that might end up accidental mutations and makes your life hard if you want to debug and find the problem.

So before trying relatively more complex things such as higher-order reducers etc. you can focus on root causes.

Cagri Yardimci
  • 2,821
  • 1
  • 9
  • 13
  • Good points you have made, I have already fixed those. My components were re-rendering and mapStateToProps was running fine before that. To clarify, this.calculateTotalPrice was from a commented out code - my plan was to re-write and schedule a dispatch from within a reducer, https://stackoverflow.com/questions/36730793/can-i-dispatch-an-action-in-reducer, which is not an anti-pattern. Your method is a plausible option too. – jenlky Feb 10 '19 at 14:06
  • I'm still puzzled by how a single CartContainer with a single mapStateToProps can do the following - `qty: productsReducer(state.productsReducer.qty, AddToCart, RemoveFromCart, HandleClick, InputQuantity)` – jenlky Feb 10 '19 at 14:11
0

You are violating a rule on reducers. The rule that says must not mutate its input state argument. What you are doing here:

const productsReducer = (state = initialState, action) => {
  let updatedNum = state.num;
  let updatedQty = state.qty;
  let index;

and even further down the lines of code for that productsReducer. Now the rule is a bit misleading and somewhat wrong and I may get into what I mean, but with that said, keep in mind that we must not mutate that input argument of state.

So for example all of this is bad:

export default (state, action) => {
  state[0] = ‘Dan’
  state.pop()
  state.push()
  state.name = ‘Dan’
  state.age = 30;
};

If your reducer function has state and an = somewhere inside of it. You are violating that reducer rule and as a result having problems with your application.

So I think in order to solve your bug here, we need to understand this rule:

Why isn't my component re-rendering, or my mapStateToProps running? Accidentally mutating or modifying your state directly is by far the most common reason why components do not re-render after an action has been dispatched

The truth is you can mutate your state argument all day and not see any error messages. You can add in properties to objects, you can change properties on objects, all on that state argument and Redux will never give you any error message at all.

The reason why the Redux docs say not to mutate the state argument is because its easier to tell a beginner don't mutate that state ever than to tell them when they can and can't mutate that state.

To be clear, we should still follow Redux official docs of not mutating state argument ever. We absolutely can, and you certainly did, but here is why you should follow what Redux docs is saying.

I want to help you understand the behind-the-scenes stuff that is going on with Redux so you can understand the bug in your application and fix it.

Let's take a look at the source code for the Redux library itself, specifically a snippet that begins on line 162 or so. You can check it out at: https://raw.githubusercontent.com/reduxjs/redux/master/src/combineReducers.js

Its the line that starts with let hasChanged = false

let hasChanged = false
    const nextState = {}
    for (let i = 0; i < finalReducerKeys.length; i++) {
      const key = finalReducerKeys[i]
      const reducer = finalReducers[key]
      const previousStateForKey = state[key]
      const nextStateForKey = reducer(previousStateForKey, action)
      if (typeof nextStateForKey === 'undefined') {
        const errorMessage = getUndefinedStateErrorMessage(key, action)
        throw new Error(errorMessage)
      }
      nextState[key] = nextStateForKey
      hasChanged = hasChanged || nextStateForKey !== previousStateForKey
    }
    return hasChanged ? nextState : state
  }

Study this block of code to really understand what this is saying:

accidental mutations prevent re-render after an action has been dispatched

The code snippet above is the part of Redux that takes an action and anytime it gets dispatched and sends the action around to all the different reducers inside of your application.

So anytime we dispatch an action, the Redux code above is going to be executed.

I will walk you through this code step by step and get a better idea of what happens when you dispatch an action.

The first thing that happens is we set up a variable of hasChanged and set it equal to false.

Then we enter a for loop which will iterate through all the different reducers inside of your application.

Inside the body of the for loop there is a variable called previousStateForKey

That variable will be assigned the last state value that your reducer returned.

Every single time that a reducer gets called the first argument will be the state that it returned the last time it ran.

So the previousStateForKey is a reference to the previous state value that a particular reducer that its iterating over returned. The next line is where the reducer gets invoked.

The first argument is the state your reducer returned the last time it can, previousStateForKey and then the second argument is the action object.

Your reducer is going to run and then eventually turn some new state value and that will be assigned to nextStateForKey. So we have hasChanged, previousStateForKey and nextStateForKey, which is our new state value.

Immediately after your reducer runs and assigns that new state value to nextStateForKey, Redux will check to see if your reducer just returned a value of undefined which is what will happen when our reducers are first initialized and Redux checks this with the if statement here:

if (typeof nextStateForKey === 'undefined') {
  const errorMessage = getUndefinedStateErrorMessage(key, action)
  throw new Error(errorMessage)
}

That's one of the big rules with reducers, a reducer can never return a value of undefined. So with this if statement, Redux asks, did they return undefined? If so throw this error message.

Assuming you get past that check, things start to get even more interesting at this line of code:

hasChanged = hasChanged || nextStateForKey !== previousStateForKey

hasChanged is going to take the value of a direct comparison between nextStateForKey and your previousStateForKey.

So that rule of not mutating your state argument comes down to that line of code above.

What that comparison does is to check to see if nextStateForKey and previousStateForKey are the exact same array or object in memory.

So if you just returned some array and its the exact same array in-memory from the last time the reducer ran then hasChanged will be a value of false, otherwise if your reducers returned a brand new array created in your reducer and the array is totally different from the array the last time your reducer ran, then hasChanged will be equal to true.

So this hasChanged variable is saying has any of the state returned by this reducer changed?

So the for loop will iterate over all of your reducers and hasChanged is going to be either true or false considering all of those different reducers that you passed to it.

Notice how there is only one hasChanged variable? There is not one for every reducer. If any reducer has changed a value or returned a different value, array or object or a different value for an integer, number or string, hasChanged will be equal to true.

After the for loop, things still get more interesting. We look at the value of hasChanged and if it has been changed to be true, the result of the entire function is to return the new state object. The entire new state object assembled by all your different reducers, otherwise, if hasChanged is equal to false, we instead return state and state is a reference to all the state that your reducers returned the last time they ran.

In summary, that snippet of code from Redux checks to see if after running your reducers, did any of your reducers return a brand new array or object or number or string, if they did, then Redux will return a brand new result from all your reducers, otherwise, if your reducers returned no new value, it will return the old state form your reducers.

What's the relevance here?

The reason its relevant to your issue is if Redux returns the old state value then Redux will not notify the rest of your application that data has changed.

If you do have a new state, if you have mutated state from one of your reducers and you return nextState, Redux will look at that object and say oh we have some new state form all these different reducers and it will notify the rest of your application, including your React application that you have a new state available and that will cause your React application to re-render.

In summary, the reason you must not mutate your state argument in a reducer is not because you can't mutate it, the reason is that if you accidentally return the same value that is pumped into your reducer, if you say return state; in your reducer and its still the same object or array, modified or not, Redux will say no difference here, its the same object or array in-memory, nothing has changed and so we have done no update to the data inside the application and the React app does not need to render itself and thats why you will not see any updated counter appear on the screen. Thats whats going on here.

Thats why the Redux docs tell you not to mutate state argument in a reducer. If you return a state argument making changes to it or not, if you return the same value in the form of return state; then no change will be made to your application.

That is what the Redux docs is trying to convey with that rule there, do not mutate state argument.

Its a lot easier to tell you not to mutate the state argument such as it does in the Redux docs than to go through the huge answer I just gave you.

Daniel
  • 9,020
  • 8
  • 63
  • 101