1

I'm working on planning app. The main page has a table with rows being projects, columns being weeks. Each cell is the workload of a person for the corresponding project and week. At the top of the table, there's a row that gives the total remaining availability for each week. It looks like this :

App screenshot

When the user types a workload in an input field, the value is automatically saved to the server after a short delay (debounce). Then a callback is called to trigger the update of the weekly totals at the top of the table.

Here's the hierarchy of components for this page:

WorkPlanPage
 - ProjectWithWorkloads
   - WorkloadInput

There are 2 issues with the code I'll show below :

  1. When the callback is called, the list of workloads held in WorkPlanPage is updated, which triggers the re-rendering of all children components. In doing so, WorkloadInputs are all unmounted and remounted. If one was in the middle of being edited by a (very quick) user, the edit and associated call to the server would be lost (the debounce is cancelled).
  2. I cannot prevent the re-rendering of all cells because the callback is updated every time a workload changes. So memoization won't work because the callback prop is updated at every workload update. This issue of parent-children relationship with callback seems very basic and yet I can't think of a way to avoid the re-rendering of all children, which could be a problem when there are a lot of children (50 projects, 15 weeks displayed, so 750 input fields.

Here's the code for the 3 components, with only relevant bits:

WorkPlanPage


const WorkPlanPage = () => {

  // ...

  const { data: leaves } = useQuery(
     ['leaves', { employeeId: employeeId, start: firstWeek, end: lastWeek }],
     () => requestEmployeeLeaves(employeeId, firstWeek, lastWeek
     { refetchOnWindowFocus: false, initialData: [], enabled: (employeeId > 0) && (currentDate !== undefined) }
   );

  // this callback changes every time allWorkloads changes. The callback changes
  // allWorkloads which in turn triggers the re-rendering of all the children.
  const handleWorkloadChange = useCallback((updated: Workload) => {
    if (allWorkloads === undefined) {
      return;
    }
    // first remove the workload we're about to update
    let newWorkloads = allWorkloads.filter(workload => {
      return (
        workload.project !== updated.project ||
        workload.employee !== updated.employee ||
        workload.date !== updated.date
      );
    });
    newWorkloads.push(updated);

    queryClient.setQueryData(
      ['workloads', { employeeId: employeeId, start: firstWeek, end: lastWeek }],
      newWorkloads);
  }, [allWorkloads, employeeId, firstWeek, lastWeek, queryClient]);

 // ...

  return (
    <div>
      <div>{/* Other parts of the page omitted */}</div>

      <WeeklyAvailabilies
        workloads={allWorkloads ? allWorkloads : []}
        leaves={leaves ? leaves : []}
        weeks={weeks} />

       {displayedProjects?.map((project: Project) => (
         <ProjectWithWorkload
           key={project.id + "-" + employeeId}
           employeeId={employeeId}
           project={project}
           weeks={weeks}
           workloads={workloadsForProjects.get(project.id)}
           onWorkloadChange={handleWorkloadChange}
         />
       ))}
      </div>
    </div>
  );
}

ProjectWithWorkloads (rows)

const ProjectWithWorkload = ({
 employeeId,
 project,
 weeks,
 workloads,
 onWorkloadChange
}) => {

  return (
    <div>
      <div>{/* Removed project header */}</div>
      <div>
        {workloads.map((workload: Workload) => (
          <div key={workload.project + workload.date}>
            <WorkloadInput
              project={project}
              workload={workload}
              onWorkloadChange={onWorkloadChanged}
            />
          </div>
        ))}
      </div>
    </div>
  );
}

WorkloadInput (cell)

const WorkloadInput = ({ project, workload, onWorkloadChange }) => {

  const [effort, setEffort] = useState<string>("");

  // ...

  const handleEffortChange = (event: React.ChangeEvent<HTMLInputElement>) => {
    const valueAsString = event.currentTarget.value;
    const valueAsNumber = parseFloat(valueAsString);

    setEffort(event.currentTarget.value);

    if (valueAsString === "") {
      debouncedSaveWorkload(0);
    } else if (!isNaN(valueAsNumber)) {
      debouncedSaveWorkload(valueAsNumber);
    }
  };

  const debouncedSaveWorkload = useMemo(() => 
    _.debounce(saveWorkload, 500), [saveWorkload]
  );

  // cancel debounce on unmounting
  useEffect(() => {
    return () => {
      debouncedSaveWorkload.cancel();
    }
  }, [debouncedSaveWorkload]);

  const saveWorkload = useCallback((newEffort: number) => {
    updateWorkload({
      ...workload, effort: newEffort 
    }).then((updatedWorkload) => {
      onWorkloadChange(updatedWorkload); // where the callback is called
    });
  };

  return (
    <div>
      <input
        type="text"
        value={workload}
        onChange={handleEffortChange}
      />
    </div>
  );
};

ptrico
  • 1,049
  • 7
  • 22
  • For optimization, you're looking to only re-render a `cell` or entire `row`? And from the look of things, it currently updates all rows, is that correct? – MwamiTovi Sep 19 '21 at 07:33
  • @MwamiTovi I would expect that only the row in which the workload has been updated should re-render. Among that row, I would expect that only the cell that has been modified would re-render. Any re-render of other cells currently triggers the cancel of the debounce function `debounceSaveWorkload`. This loses the upcoming saves to the server. That happens when the user edits a workload in a week and quickly updates the workload on the following before the first network round trip finishes. – ptrico Sep 19 '21 at 08:48
  • @MwamiTovi And by the way, I'm not entirely bothered by the seemingly lack of optimization. I'm more concerned that the full re-render cancels the debounce in all other cells, with the unexpected effect I described. It just happens that if only the modified cell re-renders, I kill two birds with one stone. But that can't happen (afai can see) because the `handleWorkloadChange` callback changes after every update, as well as the array of workload in `WorkPlanPage`, triggering the whole re-render. – ptrico Sep 19 '21 at 08:52
  • Hmmm.... you've explained quite well what your goal is here. Looking more closely into it and will revert with some suggestions. – MwamiTovi Sep 19 '21 at 10:11
  • Wait... If `onWorkloadChange` in the `cell` component is actually `handleWorkloadChange`... why not get the `handleWorkloadChange` to be completely handled within the `cell` component? In that way, the `Parent` and `rows` component should be excluded from any re-renders. Have you tried that already? And look at exlcuding `allWorkloads` (which I imagine must be some sort of `state`, right?) from `handleWorkloadChange` within the `cell` component. So instead you can maintain `allWorkloads` in the parent component but it should only be changed if you need to re-render the `Parent`. – MwamiTovi Sep 19 '21 at 11:11
  • No that won't work in my case. I added a missing piece of code to the return method of `WorkPlanPage` to explain why the callback needs to there, and not in each individual WorkInput component. The `WeeklyAvailabilities` need to be updated when a `WorkloadInput` is changed. That's the component that needs `allWorkloads` to be updated by the callback. – ptrico Sep 19 '21 at 19:26
  • Ahh, I see what you're saying and so for me that looks like a problem that can be solved by using `React.useContext` instead of trying to pass props. And with that, if `allWorkloads` is state, you can then safely move to `handleWorkloadChange` within the cell. Then remove this prop from `WeeklyAvailabilies` and instead consume the `allWorkloads` directly in `WeeklyAvailabilies`. That way only `cell` component and `WeeklyAvailabilies` will be updated when `allWorkloads` is updated. – MwamiTovi Sep 20 '21 at 11:33
  • I was thinking along these lines too. But I created a [more explicit question here](https://stackoverflow.com/questions/69251637/how-to-avoid-unmounting-of-children-components-when-using-jsxs-map). It seems there is an easier answer. Check it out. – ptrico Sep 20 '21 at 11:40
  • Alright, sounds awesome... – MwamiTovi Sep 20 '21 at 14:21

0 Answers0