背景
AIにコードを書かせている時、動作はしているが可読性が高くないコードに出会った。
一部を抜粋したので、未定義定数があるのには目を瞑ってほしい。
useEffect(() => {
setFailed(false);
if (!src) return;
if (src.startsWith('blob:')) {
setObjectUrl(src);
return;
}
let cancelled = false;
let blobUrl: string;
(async () => {
const token = await getAccessTokenSilently({
authorizationParams: { audience: Configure.auth0Audience },
});
const res = await fetch(src, {
headers: {
Authorization: `Bearer ${token}`,
'oooo-Key': Configure.xxxx,
},
});
if (cancelled) return;
if (!res.ok) {
setFailed(true);
return;
}
const blob = await res.blob();
if (cancelled) return;
blobUrl = URL.createObjectURL(blob);
setObjectUrl(blobUrl);
onObjectUrlRef.current?.(blobUrl);
})();
return () => {
cancelled = true;
if (blobUrl) {
URL.revokeObjectURL(blobUrl);
onObjectUrlRef.current?.(undefined);
}
};
}, [src, getAccessTokenSilently]);
なぜ可読性が低いのか?を分析した
-
ifでのreturnが多い。読んでいるときに条件を複数追加されると、現在何を省いた状態のデータが残っているのか、ということを記憶し続ける状態になり、思考のノイズになる。紙に書いて状態を整理することはあまりしたくない。それよりもそもそもの条件分岐を最低限にするべき
-
cancelledは何を管理するフラグなのか明確でない。最初に読んだ時、何をキャンセルするんだろう?と思った -
letでの変数宣言がある。constでの表現に切り替えられないか?
-
そもそも
useEffectを使って行う処理なのか?useEffectは基本使わない方針でいけないだろうか
などを考えた。
調べると、非同期通信を用いた状態管理で有効な手段としてuseQueryという方法があった。初学なので言語化してまとめる。
useQueryとは
参考:https://tanstack.com/query/v5/docs/framework/react/reference/useQuery
非同期通信を行う際、通信によって得られた成果物と通信の状態を返してくれるhook。useEffectで行っていたライフサイクル管理を隠蔽できる。
使用に必要な条件は基本2つだが、今回のリファクタリングでは3つ使った。
queryKey
- キャッシュの識別に必要。そのためにはキーとして一意に定まるものとなる必要がある。本実装ではキーに
srcが含まれるため、一意となることが保証されている - 同じ
srcを持つコンポーネント間でのキャッシュが共有される - 配列構造とすることで、データの階層化が実現できる。例えば、上位の階層をのキーを指定することで、下位のキーの違いを問わずデータを取得するという強み(
invalidateQueries)が保てる
queryFn
- 行いたい処理を含んだ関数を定義する
- 関数は必ず
Promiseで返す必要がある。基本的に必須 - 今回の場合、
async()が必ずPromiseを返すので条件を満たしている
enabled
-
queryFnを実行する条件を記述する。必須ではない -
enabledの条件設定について、元コードから確認してみる。条件として、returnするパターンを抽出する
`if (!src)` : srcが存在していないこと
`if (src.startsWith('blob:'))` : 'blob:'の文字列でsrcが始まっていること
`if (cancelled)` : 画面に表示されていないとき
`if (!res.ok)` : 非同期通信が失敗したとき
queryFnが発火しないための条件は上記のうち1番目、2番目である。3番目はアンマウント時のステート更新フラグだが、useQueryが内部でアンマウント時のキャンセル処理を自動で行ってくれるためフラグ管理不要、4番目はuseQueryの返り値で回収することができる。よって、以下のようにenabledを定める
enabled: !!src && !src.startWith('blob:')
返り値
- useQueryの返り値にはdata, isErrorを設定した
結果
リファクタリングした結果、次のように変換できた
const { data: fetchedUrl, isError } = useQuery({
queryKey: ['ooo-image', src],
queryFn: async () => {
const token = await getAccessTokenSilently({
authorizationParams: { audience: Configure.auth0Audience },
});
const res = await fetch(src!, {
headers: {
Authorization: `Bearer ${token}`,
'oooo-Key': Configure.xxxx,
},
});
if (!res.ok) throw new Error();
const blob = await res.blob();
return URL.createObjectURL(blob);
},
enabled: !!src && !src.startsWith('blob:'),
});
fetch処理が働くための条件をenabledの1つでまとめられた。また、不要なフラグが消えコード全体としてすっきりした見た目となった。
わかったこと
- 非同期通信でが
useQueryを使うことで、複雑なフラグ処理が一任できる - if文は便利だが、コードの可読性は低下する。こういうコードが積み重なる場合は宣言的に記述できないか考えてみる
- 一方で、
useQuery自体を知らない場合、このコードをレビューに回してもなんのこっちゃ?となる。AIばかりにコードを描かせているが、技術の勉強は欠かさないようにしたい