useCallback のアンチパターン

2020-12-18

今日現場で見つけたコードが面白かったので備忘録をば。

ざっくり以下のような感じでした。

import React, { FC, useCallback, useEffect, useState } from "react";

const Moge: FC = () => {
  const [hoge, setHoge] = useState(0);
  const fuga = useCallback(
    (moge = hoge) => {
      const piyo = moge + 1;

      setTimeout(() => {
        setHoge(piyo);
      }, 1000);
    },
    [hoge],
  );

  useEffect(() => {
    // fuga(1000);
    fuga();
  }, [fuga]);

  return <div>{hoge}</div>;
};

export default Moge;

ぱっと見では問題なさそうに見えますが、実行して頂けると一目瞭然、無限ループが発生しています。

なぜ無限ループが発生するのか、ざっくりフローを書くと。

  1. useEffect により fuga が実行される
  2. piyo に 1 が格納される
  3. 1 秒後、setHoge が実行される
  4. fuga が再生成される
  5. fuga の再生成に伴い、useEffect により fuga が再度実行される
  6. piyo に 2 が格納される
  7. 以下無限ループ

といった感じになります。

アンチパターンと書くと少し大げさなのですが、「useCallback の第 2 引数に useState によって生成された state を割り当てている一方、useCallback の第 1 引数内の関数で同じ useState によって生成された setState を実行している」のが危険なところです。

ちなみに、コメントアウトのように引数を与えた場合、画面上は変化が起きなくなるためそもそも無限ループに気づくことができない可能性すらあります、めちゃくちゃ危ないですね。

上記の例と同じく、以下の書き方も似たようなアンチパターンとなります。

import React, { FC, useCallback, useEffect, useState } from "react";

const Moge: FC = () => {
  const [hoge, setHoge] = useState(0);
  const fuga = useCallback((moge) => {
    const piyo = moge + 1;

    setTimeout(() => {
      setHoge(piyo);
    }, 1000);
  }, []);

  useEffect(() => {
    fuga(hoge);
  }, [fuga, hoge]);

  return <div>{hoge}</div>;
};

export default Moge;

『こんなコード書くわけねーじゃん』と思われるかもしれないですが、api や form が絡んでくると意外とやりがちな印象があります。

また setTimeout のように遅延実行が噛んでくると、無限ループが発生していること自体なかなか気づけないこともあると思うのでさらに厄介ですね。

今回のプロジェクトでは useCallbackuseEffect が別ファイルに定義されており、加えて非同期処理による遅延実行も絡んでいたので、気づきにくいケースではあったかなーと思います。

条件分岐によってあえてループさせるケースなどもあるとは思いますが、基本的には書かないようにしましょう、というお話でした。