4

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

1 Answers1

4

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).

Giorgi Moniava
  • 27,046
  • 9
  • 53
  • 90
  • Thanks, solution #2 worked. About solution#1, when I removed the customMemo function and instead `useCallback` on the `markHabitDone` function (tried both dependency `[habit, setHabits]` and `[]`), all the `Card` components are being rerendered. I think it is because the `habit` state changes and as it is passed to `Card`, the re-rendering is triggered. I did not find a way, to prevent re-rendering whilst also not having a custom memo function. – Swapnil Ingle Mar 14 '21 at 13:18
  • @SwapnilIngle Are you sure all cards re rendered with empty dependency array? Here they don't:https://codesandbox.io/s/intelligent-yonath-v45y7?file=/src/App.js. But if you use empty dependency array you must still rewrite `markHabitDone` function with functional setState. – Giorgi Moniava Mar 14 '21 at 14:10
  • Ok, it is working using the functional setState. Earlier, without functional setState, it was rerendering the same mismatched states from the previous render. Thanks a lot for your help :D – Swapnil Ingle Mar 14 '21 at 14:31