4

So I've got an array chosenIds[] which will essentially hold a list of ids (numbers). But I'm having trouble accessing the state in my reducer to check whether the ID I parsed to my action is in the array.

  const initialState = {
  'shouldReload': false,
  'chosenIds': [],
};

export default function filter(state = initialState, action) {
  switch (action.type) {


 case ADD_TYPE:
      console.log(state.chosenIds, "Returns undefined???!!!");

      // Check if NUMBER parsed is in state
      let i = state.chosenIds.indexOf(action.chosenId);

      //If in state then remove it
      if(i) {
        state.chosenIds.splice(i, 1);
        return {
          ...state.chosenIds,
          ...state.chosenIds
        }
      }
      // If number not in state then add it 
      else {
        state.chosenIds.push(action.chosenId)
        return { ...state.chosenIds, ...state.chosenIds }
      }

I'm not to sure what's going on...But when I log state.chosenIds, it returns undefined? It doesn't even return the initial empty array [] .

Basically what this function is suppose to do is check to see if the action.chosenId is in the state.chosenIds, If it is, then remove the action.chosenId value, if it's not, then add the action.chosenId to the state.

James111
  • 15,378
  • 15
  • 78
  • 121

2 Answers2

20

I'm seeing a few different issues here.

First, you're using splice() and push() on the array that's already in the state. That's direct mutation, which breaks Redux. You need to make a copy of the array, and modify that copy instead.

Second, the object spread usage doesn't look right. You're using it as if "chosenIds" was an object, but it's an array. Also, you're duplicating the spreads. That's causing the returned state to no longer have a field named "chosenIds".

Third, Array.indexOf() returns -1 if not found, which actually counts as "truthy" because it's not 0. So, the current if/else won't do as you expect.

I would rewrite your reducer to look like this:

export default function reducer(state = initialState, action) {
    switch(action.type) {
        case ADD_TYPE:
            let idAlreadyExists = state.chosenIds.indexOf(action.chosenId) > -1;
            // make a copy of the existing array
            let chosenIds = state.chosenIds.slice();

            if(idAlreadyExists) {
                chosenIds = chosenIds.filter(id => id != action.chosenId);                
            }     
            else {
                // modify the COPY, not the original
                chosenIds.push(action.chosenId);            
            }      

            return {
                // "spread" the original state object
                ...state,
                // but replace the "chosenIds" field
                chosenIds
            };
        default:
            return state;
    }    
}
markerikson
  • 63,178
  • 10
  • 141
  • 157
  • Ahhh ofcourse...I totally forgot. I don't get why you can't just update the state directly? Probably just a redux rule role – James111 Mar 04 '16 at 07:27
  • 4
    Because objects and arrays are pass-by-reference in JavaScript. When you mutate your state, you change the previous state and make the previous state identical to the new state. Thus react-redux can no longer detect a change between the old and the new, so it doesn't update. – David L. Walsh Mar 04 '16 at 09:14
  • Beyond that, Redux has at least two places it does shallow equality checks: at the end of a reduce call looking at the top of your state tree, and in React-Redux where it compares your return values for mapStateToProps to see if they've changed. In both places, if the contents of old value vs new value haven't changed, it assumes nothing deeper changed and that it doesn't need to actually do anything more (namely, notifying subscribers or re-rendering the wrapped component). – markerikson Mar 25 '16 at 16:53
0

another aproach with a standalone function:

export default function reducer(state = initialState, action) {
    switch(action.type) {
        case ADD_TYPE:
             function upsert(array, item) {
    // (1)
    // make a copy of the existing array
    let comments = array.slice();
    const i = comments.findIndex(_item => _item._id === item._id);
    if (i > -1) {
      comments[i] = item;

      return comments;
    }
    // (2)
    else {
      // make a copy of the existing array
      let comments = array.slice();
      comments.push(item);

      return comments;
    }
  }

            return {
     ...state,
    comments: upsert(state.comments, action.payload),
            };
        default:
            return state;
    }    
}
Mohamed Daher
  • 609
  • 1
  • 10
  • 23