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

2021-01-18

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

今回はコードレビューで「注文処理を 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 に対して getNoncepostOrder という 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 内でチェーンを繋いでいる状態は、何か良くないことを隠蔽しているケースが多いのでは?と少し思った、今日このごろです。

© 2018 kk-web