reactjs / reactjs/react.dev

Effect cleanup potential issue in React 17 for mutable value inside effect

Offen
#4,012 0 Kommentare 2 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Vorherrschende Sprache
JavaScript
Sterne
11.8k
Forks
7.9k
Ø Merge
1 T. 11 Std.
Gemergte PRs (30 T.)
11

Beschreibung

In React 17, the effect cleanup function always runs asynchronously — for example, if the component is unmounting, the cleanup runs after the screen has been updated.

In the changelog React team has highlighted a potential issue and solution for this:

Problematic Code:

useEffect(() => {
  someRef.current.someSetupMethod();
  return () => {
    someRef.current.someCleanupMethod();
  };
});

Solution suggested by the React team:

The problem is that someRef.current is mutable, so by the time the cleanup function runs, it may have been set to null. The solution is to capture any mutable values inside the effect.

useEffect(() => {
  const instance = someRef.current;
  instance.someSetupMethod();
  return () => {
    instance.someCleanupMethod();
  };
});

While the above solution works in most cases, I have created a custom hook, where this solution is problematic:

function useMountEffect(funcForMount, funcForUnmount) {
  const funcRef = useRef();
  funcRef.current = { funcForMount, funcForUnmount };
  useEffect(() => {
    funcRef?.current?.funcForMount?.();
    return () => funcRef?.current?.funcForUnmount?.();
  }, []);
}

In the above example, I explicitly don't want to capture the mutable value, otherwise, I'll have to re-run the effect when funcForUnmount changes.

The reason to create the hook in this way is to have the freedom to call the function only on mount / unmount without worrying about stale closure and dependency array.

Update:
I have found one approach for useMountEffect

function useMountEffect(funcForMount, funcForUnmount) {
  const isMounted = useRef();
  useEffect(() => {
    return () => {
      isMounted.current = false;
    };
  }, []);
  useEffect(() => {
    if (!isMounted.current) {
      isMounted.current = true;
      funcForMount?.();
    }
    return () => !isMounted.current && funcForUnmount?.();
  });
}

But the major questions here are:

  • How can the ref.current go null between unmount and cleanup call in React 17?
  • Who is setting the value null? Is it react which can set ref.current to null? Or, is it the user who can change in between?
  • If a user can change in between how is this possible? Why was it not an issue prior to React 17?

The reason to raise this issue for docs is that the line The problem is that someRef.current is mutable, so by the time the cleanup function runs, it may have been set to null. is unclear and raises a lot of questions in the mind.

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Beginne mit dem im Issue verlinkten Abschnitt des React 17 changelog und vergleiche dessen Erklärung mit dem gemeldeten useMountEffect-Beispiel. Kläre, wer ref.current auf null setzen kann, wann das Cleanup ausgeführt wird und warum sich das Timing in React 17 geändert hat. Die Aufgabe ist abgeschlossen, wenn die Dokumentation die aufgeführten Fragen beantwortet, ohne das Verhalten von mutable refs unklar zu lassen.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
javascript, react
Bereich
documentation
Issue-Typ
Dokumentation
Schwierigkeit
3/5
Geschätzter Aufwand
1-2 Tage
Aktivitätsstatus
Veraltet
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
35/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.