Search code examples
javascriptreactjsreact-statereact-memo

React Memo resets values in component state


What I am trying to do

  1. I'm trying to use an array of objects (habits) and render a "Card" component from each one.
  2. Now for each Card (habit) you can "mark" that habit as done, which will update that Card's state.
  3. I've used React.memo to prevent other Cards from re-rendering.
  4. Whenever the user mark a card as done, the card header gets changed to "Edited"

The Issue I'm facing

Whenever a card is marked as done, the header changes alright, but whenever any other card is marked as done, the first card's state gets reverted back.

I've not been able to find other people facing a similar issue, can somebody please help?

Here is the code:

import React, { useState } from "react";

const initialState = {
  habits: [
    {
      id: "1615649099565",
      name: "Reading",
      description: "",
      startDate: "2021-03-13",
      doneTasksOn: ["2021-03-13"]
    },
    {
      id: "1615649107911",
      name: "Workout",
      description: "",
      startDate: "2021-03-13",
      doneTasksOn: ["2021-03-14"]
    },
    {
      id: "1615649401885",
      name: "Swimming",
      description: "",
      startDate: "2021-03-13",
      doneTasksOn: []
    },
    {
      id: "1615702630514",
      name: "Arts",
      description: "",
      startDate: "2021-03-14",
      doneTasksOn: ["2021-03-14"]
    }
  ]
};

export default function App() {
  const [habits, setHabits] = useState(initialState.habits);

  const markHabitDone = (id) => {
    let newHabits = [...habits];
    let habitToEditIdx = undefined;

    for (let i = 0; i < newHabits.length; i++) {
      if (newHabits[i].id === id) {
        habitToEditIdx = i;
        break;
      }
    }

    let habit = { ...habits[habitToEditIdx], doneTasksOn: [], name: "Edited" };
    newHabits[habitToEditIdx] = habit;
    setHabits(newHabits);
  };

  return (
    <div className="App">
      <section className="test-habit-cards-container">
        {habits.map((habit) => {
          return (
            <MemoizedCard
              markHabitDone={markHabitDone}
              key={habit.id}
              {...habit}
            />
          );
        })}
      </section>
    </div>
  );
}

const Card = ({
  id,
  name,
  description,
  startDate,
  doneTasksOn,
  markHabitDone
}) => {
  console.log(`Rendering ${name}`);
  return (
    <section className="test-card">
      <h2>{name}</h2>
      <small>{description}</small>
      <h3>{startDate}</h3>
      <small>{doneTasksOn}</small>
      <div>
        <button onClick={() => markHabitDone(id, name)}>Mark Done</button>
      </div>
    </section>
  );
};

const areCardEqual = (prevProps, nextProps) => {
  const matched =
    prevProps.id === nextProps.id &&
    prevProps.doneTasksOn === nextProps.doneTasksOn;

  return matched;
};

const MemoizedCard = React.memo(Card, areCardEqual);

Note: This works fine without using React.memo() wrapping on the Card component.

Here is the codesandbox link: https://codesandbox.io/s/winter-water-c2592?file=/src/App.js


Solution

  • Problem is because of your (custom) memoization markHabitDone becomes a stale closure in some components.

    Notice how you pass markHabitDone to components. Now imagine you click one of the cards and mark it as done. Because of your custom memoization function other cards won't be rerendered, hence they will still have an instance of markHabitDone from a previous render. So when you update an item in a new card now:

    let newHabits = [...habits];
    

    the ...habits there is from previous render. So the old items are basically re created this way.

    Using custom functions for comparison in memo like your areCardEqual function can be tricky exactly because you may forget to compare some props and be left with stale closures.

    One of the solutions is to get rid of the custom comparison function in memo and look into using useCallback for the markHabitDone function. If you also use [] for the useCallback then you must rewrite markHabitDone function (using the functional form of setState) such that it doesn't read the habits using a closure like you have in first line of that function (otherwise it will always read old value of habits due to empty array in useCallback).