注文処理は useEffect で行うべきではないと指摘された話

最近よく書いているレビューシリーズです。

今回はコードレビューで「注文処理を useEffect に書くのは無限ループが発生しやすいからやめたほうが良いですよ」と指摘されました。

で、自分が書いていたコードは以下のような感じでした。

import React, { FC } from "react";
import useSWR from "swr";

const Hoge: FC = () => {
  const { data: userInformation } = useSWR("/getUserInformation");
  const [nonce, setNonce] = useState("");
  const handleSubmit = useCallback(() => {
    const {
      data: { nonce: nextNonce },
    } = axios("/getNonce", { userInformation });

    setNonce(nextNonce);
  }, [userInformation]);

  useEffect(() => {
    if (!nonce) {
      return;
    }

    axios("/postOrder", { nonce, userInformation });
  }, [nonce, userInformation]);

  return (
    <form onSubmit={handleSubmit}>
      <button type="submit">注文する</button>
    </form>
  );
};

export default Hoge;

ぱっと見良さそうに見えますが、この状態では正常処理が行われる保証がありません。

理由としては useSWR には Automatic Revalidation の機能がデフォルトでオンになっているため、userInformation が勝手に更新されてしまう可能性が高いです。

そのため postOrder を叩いている最中に userInformation が更新されてしまった場合 postOrder が多重で実行されてしまうことになります。

そのため、上記に対し以下のコードのほうが良いよと提示されたのですが。

import React, { FC } from "react";
import useSWR from "swr";

const Hoge: FC = () => {
  const { data: userInformation } = useSWR("/getUserInformation");
  const handleClick = useCallback(() => {
    axios("/getNonce", { userInformation }).then(({ data: { nonce } }) => {
      axios("/postOrder", { nonce, userInformation });
    });
  }, [userInformation]);

  return (
    <form onSubmit={handleSubmit}>
      <button type="submit">注文する</button>
    </form>
  );
};

export default Hoge;

確かに正常処理が行われそうです。

が、個人的には、そもそも 1 回の onClick に対して getNonce と postOrder という 2 つのロジックを叩こうとしているのが良くないのでは？と思っているのですが、そうでもないのかな？

つまり、本来はこうすべきではないかなーと。

import React, { FC } from "react";
import useSWR from "swr";

const Hoge: FC = () => {
  const { data: userInformation } = useSWR("/getUserInformation");
  const [nonce, setNonce] = useState("");
  const handleSubmitOnGetNonce = useCallback(() => {
    const {
      data: { nonce: nextNonce },
    } = axios("/getNonce", { userInformation });

    setNonce(nextNonce);
  }, [userInformation]);
  const handleSubmitOnPostOrder = useCallback(() => {
    if (!nonce) {
      return;
    }

    axios("/postOrder", { nonce, userInformation });
  }, [nonce, userInformation]);

  return (
    <div>
      <form onSubmit={handleSubmitOnGetNonce}>
        <button type="submit">Nonce を取得する</button>
      </form>
      <form onSubmit={handleSubmitOnPostOrder}>
        <button type="submit">注文する</button>
      </form>
    </div>
  );
};

export default Hoge;

さらに言うならば、「Nonce の取得」と「注文」は別画面であるとさらに開発的にも ux 的に良さそうかなーと思いました。

なので、個人的に 「注文処理を useEffect に書くのは無限ループが発生しやすいからやめたほうが良い」のではなく、「そもそも ux が良くない」というところで腑に落ちました。

hooks 内でチェーンを繋げるのって結構気持ち悪いなーと思っていて。

React ってどれだけチェーンを書かずに済むかを前提に考えられているフレームワークであると勝手に思っています。

hooks 内でチェーンを繋いでいる状態は、何か良くないことを隠蔽しているケースが多いのでは？と少し思った、今日このごろです。