はじめに
「Reactのkeyに配列のindexを使うのは避けるべきだ」
Reactを書いたことのある方なら、なんとなく耳にしたことがあるのではないでしょうか。React公式ドキュメントにも落とし穴として書かれている内容です。
私もなんとなく使用を避けてはいたのですが、先日「indexを使って書いても問題なさそうだな」と思い、あえて使ったことがありました。
レビューで指摘されて修正することになったのですが…先輩に自分の考えを伝えて意見交換した上で、やはり「Reactのkeyに配列のindexを使うのはやめた方がいいだろう」と思ったので、記録しておこうと思います。
問題のコード
以下が、問題となるソースコードです。
ここでerrorMessagesはAPIリクエスト後に返されるstringの配列です。
{errorMessages.map((message, index) => (
<ValidationErrorText key={index} text={message} />
))}
レビューで先輩にコメントいただきました。
配列をループしてリストのようにJSXをレンダーするとき、keyにはindexを使用しない方がいいですよ。
今回であれば、messageがユニークになっているはずなので、messageをkeyに入れるのがいいと思います。
私：「（先輩、わかっているんだけども…いやでも、今回は大丈夫じゃない？）」
先輩に自分の考えを伝えてみることにしました。
先輩とディスカッション
私：「仕様上、errorMessagesはAPIリクエストによって描画されます。一度描画されると、配列の順番が変更されたり削除されることはないです。」
私：「重複することはないとしても、stringのmessageをkeyにするのはなんか気持ち悪い気がするから、indexでも良さそう〜と考えて実装しました」
先輩：「確かに、今の仕様上なら問題は起こらないかもしれない。ただし、初めてこのソースコード読む人からすると、その背景はちょっと考えないとわからないよね。読み手に負荷がかかってしまう」
先輩：「それに、将来は仕様が変わってmessageを消せるようになったりするかもしれない。その場合、今回の実装ではバグが生まれてしまう可能性がある」
先輩：「それなら、やらない方がいいと思うんだけどどうかな？」
私：「た、確かに…」
修正
keyにmessageを入れるよう修正しました。
{errorMessages.map((message) => (
<ValidationErrorText key={message} text={message} />
))}
- ①読み手に負荷がかかってしまうこと
- ②将来バグが発生するリスクがあること
ここまで考慮できていませんでした。仕様上は問題ないとしても、「Reactのkeyに配列のindexを使うのはやめた方がいいだろうと思いました。
終わりに
少し調べてみると、こちらの記事にkeyをindexに仕様しても問題ない場合が書かれていました。
keyをindexに仕様しても問題ない場合
- 配列と要素が静的である - 計算も変更もされない
- 配列内の要素にIDがない
- 配列が決して並べ替えられたりフィルタリングされたりしない
これらすべての条件が満たされている場合、indexをkeyとして使用しても問題ない
今回に当てはめてみると、そもそも条件1を満たしていないですね😇
APIリクエストの結果でerrorMessagesが変わってしまうので、静的とは言えないです。
読み手に優しいコードを書く、将来バグを生むリスクを下げる。
2つの観点からも、たとえ今は問題がないとしてもReactのkeyに配列のindexを使うのはやめた方がいいだろうと思いました。