Delete an item from an Array that is inside an object (redux/typescript/react)

Question

I am using a local json file to store some data as the follows:

[
    {
        "name": "Sports",
        "todos": [
            {
                "id": 1,
                "title": "Play Football"
            },
            {
                "id": 2,
                "title": "Play Basketball"
            }
        ]
    },
    {
        "name": "Work",
        "todos": [
            {
                "id": 3,
                "title": "Study TS"
            },
            {
                "id": 4,
                "title": "Work Hard"
            }
        ]
    }
    ]

I made a reducer that takes the two categories above and actually display all these data on the component page.

I am now trying to create another reducer that deletes a certain item from within the todos array that is within each category but I am failing. Here are the actions and the reducers I created so far

// actions

export const getCategories = () => {
    return async (dispatch: Dispatch) => {
        const response = await axios.get<Category[]>(url);

        dispatch({
            type: 'GET_ALL_CATEGORIES',
            payload: response.data
        });
    };
};

export const deleteTodo = (id: number) => {
    return {
        type: 'DELETE_A_TODO',
        payload: { id }
    };
};

// reducers
const categoriesReducer = (state: Category[] = [], action: Action) => {
    switch (action.type) {
        case 'GET_ALL_CATEGORIES':
            return action.payload;
        case 'DELETE_A_TODO':
            state.forEach(category => {
                return category.todos.filter(
                    todo => todo.id !== action.payload.id
                );
            });
        default:
            return state;
    }
};

export const reducers = combineReducers<ReduxStoreState>({
    categories: categoriesReducer
});

Then inside the component I wired up the deleteTodo action successfully but nothing gets deleted, there is no difference in the state.

What am I doing wrong?

in the delete block you dont return anything so you end up with the current state, also forEach doesnt return anything — Asaf Aviv
– Asaf Aviv, Commented Dec 18, 2019 at 2:28
i don't see you return any new state inside the the delete case — plat123456789
– plat123456789, Commented Dec 18, 2019 at 2:29
@plat123456789 when I try to return the new state, it gives me a nasty error that no overload matches this call and that type Todo[][] cannot be assigned to type Category[] how can I return the same array of categories but with the updates? — Ahmed Magdy
– Ahmed Magdy, Commented Dec 18, 2019 at 2:31
@AsafAviv I used map and it doesn't work too. Check the below comment please. — Ahmed Magdy
– Ahmed Magdy, Commented Dec 18, 2019 at 2:31
@Ahmed Magdy, ok, as you mention, the problem now is type of the state don't match between the original state and the new state you try to set, the question above doesn't include the state interface. — plat123456789
– plat123456789, Commented Dec 18, 2019 at 2:37

Pengson · Accepted Answer · 2019-12-18 02:43:35Z

2

Replace

state.forEach(category => {
    return category.todos.filter(
        todo => todo.id !== action.payload.id
    );
});

with:


return state.map(category => {
  return {
    ...category,
    todos: category.todos.filter(todo => todo.id !== action.payload.id)
  }
})

edited Dec 18, 2019 at 2:43

answered Dec 18, 2019 at 2:42

Pengson

8855 silver badges10 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Asaf Aviv Over a year ago

forEach returns undefined

Pengson Over a year ago

@AsafAviv Thanks, never noticed the forEach usage in the old code.

Pengson Over a year ago

@AsafAviv It is required to traversing the category and todos then find the target. The only thing I think could be improved is using immutable data instead of constructing every detail of object each time. Your code does it well since it achieves part of this work. But I still recommend you to use any immutable lib to make sure it's safe enough.

Asaf Aviv Over a year ago

Right now you are traversing every todos array on every update even if it's the first item inside the first category object even though in this case it probably doesn't matter because it's small. your solution is more elegant then my solution in this case but a better solution will be to just pass the category index in the payload then we can just iterate over the todos array that contains the todo instead of searching the todo in every todos array to find it.

Gabor Szekely · Accepted Answer · 2019-12-18 02:45:51Z

1

Calling the forEach method does not actually return a new array in your DELETE case, and you are also not returning a new state object, hence the issue.

You need to map over your state and return new category objects with updated todo arrays. This can be achieved using the following code:

case 'DELETE_A_TODO':
    return state.map(category => {
        return {
            ...category,
            todos: category.todos.filter(
                todo => todo.id !== action.payload.id
            )
        };
    });

Hope that helps.

answered Dec 18, 2019 at 2:45

Gabor Szekely

1,2588 silver badges15 bronze badges

Comments

Asaf Aviv · Accepted Answer · 2019-12-18 05:58:11Z

I would suggest to return the category name or better the category index with the id in the payload of deleteTodo action.

in the current state of your code this is one of the solutions

const categoriesReducer = (state: Category[] = [], action: Action) => {
  switch (action.type) {
    case 'GET_ALL_CATEGORIES':
      return action.payload
    case 'DELETE_A_TODO': {
      const { id } = action.payload
      const categoryIndex = state
        .findIndex(category => category.todos.some(todo => todo.id === id))

      return state.map((category, i) => i === categoryIndex
        ? {
            ...category,
            todos: category.todos.filter(todo => todo.id !== id),
          }
        : category
      )
    }
    default:
      return state
  }
}

Now if you return the category index together with the id, we don't need to iterate through every todos array to find the id

export const deleteTodo = (id: number, categoryIndex: number) => {
  return {
    type: 'DELETE_A_TODO',
    payload: { id, categoryIndex }
  }
}

const categoriesReducer = (state: Category[] = [], action: Action) => {
  switch (action.type) {
    case 'GET_ALL_CATEGORIES':
      return action.payload
    case 'DELETE_A_TODO': {
      const { id, categoryIndex } = action.payload

      return state.map((category, i) => i === categoryIndex
        ? {
            ...category,
            todos: category.todos.filter(todo => todo.id !== id),
          }
        : category
      )
    }
    default:
      return state
  }
}

Collectives™ on Stack Overflow

Delete an item from an Array that is inside an object (redux/typescript/react)

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related