CODEWITHSUNDEEP
reactjsreduxreact-routerreact-redux
xoail
xoail

Reputation: 3064

Redux state mutation

Why does react think I am mutating state when clearly I am passing a new object? Am I missing something? This is my reducer to adding a question on a quiz object within an array of quizzes.

case types.UPDATE_QUESTION_SUCCESS: {
  let quizContext = state.filter(quiz => quiz.id === action.quizId);
  let filteredQuestions = quizContext[0].questions.filter(question => 
    question.questionId !== action.question.questionId
  );
  filteredQuestions.push(action.question);
  quizContext[0].questions = filteredQuestions;
  let quiz = quizContext[0];
  return [...state.filter(quiz => quiz.id !== action.quizId), Object.assign({}, quiz)];
}

Error:

Invariant Violation: A state mutation was detected between dispatches, in the path `quizes.4.questions.1`. This may cause incorrect behavior. (http://redux.js.org/docs/Troubleshooting.html#never-mutate-reducer-arguments)

Upvotes: 1

Views: 5366

Answers (2)

sasidhar
sasidhar

Reputation: 7752

You are mutating the state as per your reducer. When you are using filter function the objects that are passed to the filter are references of the original object, you are not doing a deepCopy here. And this line filterQuestions.push(action.question) is actually mutating the state.

Try this instead: let quizContext = cloneDeep(state.filter(quiz => quiz.id === action.quizId));

Although there might be a more elegant solution out there, this is where you are going wrong. The filtered objects being returned are references into the original array, but not new objects. So you end up mutating the original objects in the state.

--More clarification--

cloneDeep is a lodash function and if you are using Object.assign it is going to do only a shallow merge, but not a deep merge. So when the state is mutated at a deeper level the references are still of the original object there by mutating the original state

--EDIT 2--

as noted by mash the line that is mutating your state is quizContext[0].questions = filteredQuestions; and use Object.assign if you are sure you don't have a nested state in which case more reducers are in order

Upvotes: 0

mash
mash

Reputation: 2526

First of all, since you only care about quizContext[0], filter is overkill. Just do:

const quiz = state.find(quiz => quiz.id === action.quizId);

to achieve the same result.

I don't want to do too much code review here but one more thing. You do use let even though you should use const all the time. const just means you can't reassign the variable you can still mutate it.

Then you filter the questions add one and then add it back to the quiz you just found.

The quizContext[0].questions = is the issue because even though your objects are now in a new array they're still the same objects so if you mutate them you mutate the state.

const quiz = Object.assign({}, state.find(quiz => quiz.id === action.quizId));

So rewriting the complete thing you'd end up with something like this:

case types.UPDATE_QUESTION_SUCCESS:
{
  const quiz = Object.assign({}, state.find(quiz => quiz.id === action.quizId));
  quiz.questions = quiz.questions.filter(question =>
    question.questionId !== action.question.questionId
  );
  quiz.questions.push(action.question);
  return [...state.filter(quiz => quiz.id !== action.quizId), quiz];
}

But keep in mind that most of the time you don't wan't to filter data in your reducer at all. Instead I'd recommend to use reselect so that you can compute derived data from the data in your store.

Upvotes: 2

Related Questions