reactjs / reactjs/react.dev

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

オープン
#4,012 コメント 0 件 リアクション 2 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

主要言語
JavaScript
スター
11.8k
フォーク
7.9k
平均マージ
1日 11時間
マージ済み PR(30日)
11

説明

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.

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

Issue にリンクされている React 17 changelog のセクションから始め、その説明を報告されている useMountEffect の例と比較します。誰が ref.current を null に設定できるのか、cleanup がいつ実行されるのか、そして React 17 でタイミングが変わった理由を明確にします。列挙された質問にドキュメントが回答し、mutable ref の動作が曖昧なままになっていなければ完了です。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
javascript, react
領域
documentation
issue の種類
ドキュメント
難易度
3/5
見積もり時間
1〜2日
活発さ
停滞
明瞭さ
おおむね明確
初心者へのやさしさ
35/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。