useEffectでStateを同期して無限ループ。filteredItemsをDerived Stateに直すまで

ProductListコンポーネントでフィルタとソートを実装してたんですけど、検索クエリとカテゴリ変更の同期で挙動がおかしくなったんです。最終的にはfilteredItemsをstateで持つのをやめて、レンダリングのたびに算出する形にしました。そこまでの試行錯誤を順にまとめていきます。

ProductListでfilteredItemsをstateに持っていた

最初の実装はこんな感じでした。

const ProductList = ({ items }: { items: Item[] }) => {
  const [query, setQuery] = useState('');
  const [filteredItems, setFilteredItems] = useState<Item[]>([]);
  const [activeCategory, setActiveCategory] = useState<string>('all');

  useEffect(() => {
    const result = items.filter(item => {
      const matchQuery = item.name.toLowerCase().includes(query.toLowerCase());
      const matchCategory = activeCategory === 'all' || item.category === activeCategory;
      return matchQuery && matchCategory;
    });
    setFilteredItems(result);
  }, [items, query, activeCategory]);

もともとこのコンポーネントはサーバーサイドフィルタ前提だったんです。/api/items?q=&cat=&page=を叩いて、SWRのキーがクエリパラメータに依存してたので、取得したレスポンスをuseEffectでsetItems(data)してました。サーバーのデータをそのままstateに保存する構造でした。

その後、バックエンドの仕様が変わって全件一括取得に切り替わったんです。最大でも1万件未満。フィルタはクライアント側に移動しました。この時、itemsはpropsに変えたんですけど、filteredItemsをstateで持つやり方はそのまま残っちゃいました。

残した理由は、フィルタ後の件数を親に通知する必要があったからです。親のヘッダーに「該当 N 件」を表示したくて、子から親にレンダリング結果を戻すためにuseState+useEffect+コールバックを組み合わせて回りくどい実装をしてたんですけど、設計ミスでした。

ページネーションを足したらMaximum update depth exceeded

これにページネーションとソート順の機能、さらに一覧が空になったらページ番号を1にリセットする機能を追加したあたりから挙動がおかしくなりました。

const [currentPage, setCurrentPage] = useState(1);

useEffect(() => {
  if (filteredItems.length === 0 && currentPage !== 1) {
    setCurrentPage(1);
  }
}, [filteredItems, currentPage]);

コンソールに出たのがこれです。

Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.

このループはsetCurrentPage単独では止まりません。原因は親への通知経路にあったんです。

実際に追ってみると、親コンポーネント側でitemsをインライン生成して渡してる箇所がありました。

<ProductList items={rawData.map(transformItem)} />

親が再レンダリングされるたびにitemsの参照が新しくなります。するとProductList内の第1のuseEffectが実行されて、setFilteredItemsを呼びます。setFilteredItemsで再レンダリング。この時filteredItemsはfilter()で新しい配列参照として生成されるから、第2のuseEffectがfilteredItemsの変更を検知して実行。特定の条件でsetCurrentPageが発火。さらに親にonFilterApplied(filteredItems.length)を渡してたから、親が再レンダリングされて、新しいitems配列が渡されてきて第1のuseEffectが再実行。無限ループの完成です。

このループ、devのReact 18 StrictModeだとマウント時にEffectが2回呼ばれるせいですぐ検知されて、Maximum update depth exceededが即出ます。production buildだと同じコードでもエラーは出なくて、「文字を1文字打たずにページ番号が1に戻る」「スクロール位置が飛ぶ」という別の症状として現れてました。原因が同じでも環境で見え方が違うから、最初は別々のバグだと思ってて時間がかかってしまいました。

onFilterAppliedをEffectに移してもWarningは消えるだけ

onFilterAppliedは当初、レンダリング中に直接呼んでました。これがReact 18で以下のWarningを出すのです。

Warning: Cannot update a component (Parent) while rendering a different component (ProductList)

レンダリング中に親のstateを更新してるから、そもそもEffectに置く以前の問題です。そこでuseEffectの末尾に移して、警告は消えましたが親の再レンダリングは止まりません。Effect内に移しても、親が再レンダリングされ続けるという別の問題が残りました。

依存配列にJSON.stringifyを使うのもNGな書き方でした。シリアライズのコストが無駄だし、そもそも設計が間違ってます。1万件のオブジェクト配列を毎回文字列に変換してると、処理が重くなって画面の更新に時間がかかっちゃいます。それにundefinedや関数を含むオブジェクトが混ざるとプロパティが消えて、参照が変わってるのに文字列が同一になってしまい、変更を取りこぼします。

itemsを親側でuseMemoしても、親の再レンダリング原因が別にあれば結局再計算されます。実際この親はContext Providerのvalueをインラインのオブジェクトリテラルで渡してて、Provider自身の再レンダリングでitemsのuseMemoは無効化されてました。ContextのvalueをuseMemoで包む修正は別途必要ですけど、それはこのコンポーネントの仕事じゃありません。そもそも子のローカル状態としてfilteredItemsを保持してること自体よくなかったです。

filteredItemsをstateから削除する

公式ドキュメントの「You Might Not Need an Effect」に、propsや既存のstateから計算できる値をstateに保存してはいけない、と書かれています。なのでfilteredItemsをstateから完全に削除します。

// stateを排除
// const [filteredItems, setFilteredItems] = useState<Item[]>([]);

// レンダリング中に直接計算
const filteredItems = useMemo(() => {
  return items.filter(item => {
    const matchQuery = item.name.toLowerCase().includes(query.toLowerCase());
    const matchCategory = activeCategory === 'all' || item.category === activeCategory;
    return matchQuery && matchCategory;
  });
}, [items, query, activeCategory]);

1000件程度ならuseMemoすら不要かもしれません。console.time('filter')で計測しました。10,000件のダミーデータで愚直にitems.filter()を実行した場合の実行時間は平均 1.2ms 〜 2.8ms 程度。JavaScriptのヒープ割り当てを考慮しても、毎レンダリングで直接計算して問題ないレベルです。無意味なuseMemoによる依存配列比較のオーバーヘッドの方が無駄でした。

とりあえずuseMemoも外します。

const filteredItems = items.filter(item => {
  const matchQuery = item.name.toLowerCase().includes(query.toLowerCase());
  const matchCategory = activeCategory === 'all' || item.category === activeCategory;
  return matchQuery && matchCategory;
});

ただし計測条件は配列がフラットな場合です。ソート要件が入ってfilterの後にsortを繋げるようになると、sortは配列を破壊的に並べ替えるからitemsを直接渡せません。[...items].sort()のコピーが毎レンダリング走ります。ここは後で見ることにしました。

ページリセットもEffectでやらない

次に発生した問題は、フィルタ変更時にcurrentPageを1に戻す処理をどう書くかです。これもuseEffectで「フィルタが変わったらsetCurrentPage(1)」と書くと副作用ループの原因になります。レンダリングサイクルが2回発生して、一瞬だけ古いページ番号で新しいリストを描画しようとしてインデックス外参照のエラーが出ました。

TypeError: Cannot read properties of undefined (reading 'id')

これはエラー境界を張ってないと本番では画面が白くなります。SSRしてるページでは、サーバー側の初回レンダリングでfilteredItemsが空でcurrentPageがURLの?page=3から復元されて3、という組み合わせで同じエラーが出てました。ハイドレーション後にクライアントで再計算されますけど、SSRの時点で落ちるからhydrateRootまで到達しません。

状態同期のためのuseEffectは止める。リセット処理は副作用として切り離さず、イベントハンドラ側、あるいはデータの計算時に対処します。

方法Aとして、検索クエリやカテゴリを変更するアクション側で直接リセットします。

const handleQueryChange = (e: React.ChangeEvent<HTMLInputElement>) => {
  setQuery(e.target.value);
  setCurrentPage(1); // イベント側で直接リセット
};

const handleCategoryChange = (category: string) => {
  setActiveCategory(category);
  setCurrentPage(1); // イベント側で直接リセット
};

ただしこの方法は、queryやactiveCategoryを変更しうる経路が他にもあると漏れます。ブラウザの戻る/進む、URL直接編集、別コンポーネントからのリセットボタン。実際、ヘッダー側にある「条件をクリア」ボタンが親からqueryを空にする実装になってて、そのパスではcurrentPageがリセットされませんでした。イベントハンドラでの同期は、変更経路が単一であることが前提です。

方法Bとして、ページ番号そのものを計算で出す、あるいは表示時にクランプします。総ページ数の計算もstateじゃなくてその場での計算です。

const totalPages = Math.ceil(filteredItems.length / PAGE_SIZE) || 1;
const safeCurrentPage = Math.min(currentPage, totalPages);
const paginatedItems = filteredItems.slice(
  (safeCurrentPage - 1) * PAGE_SIZE,
  safeCurrentPage * PAGE_SIZE
);

こうならfilteredItemsの件数が急減してもsafeCurrentPageが勝手に丸められるから、インデックス外参照でクラッシュしません。currentPageのstate自体は次にユーザーが操作するか、クエリ変更ハンドラで戻せばいい。

|| 1を忘れると、0件のときMath.ceil(0 / 20) = 0になってtotalPagesが0、Math.min(currentPage, 0)でsafeCurrentPageが0、slice(-20, 0)が空配列を返します。エラーは出ないですけどページネーションUIの「1 / 0」表示が出ます。わりと気づきにくいところです。

selectedItemIdの同期もDerived Stateに

もう1つの問題は、選択中アイテムselectedItemIdの同期です。フィルタで絞り込んだ結果、現在選択中のアイテムが画面から消えた場合を考えます。

以前の実装はこうでした。

const [selectedItemId, setSelectedItemId] = useState<string | null>(null);

useEffect(() => {
  const exists = filteredItems.some(item => item.id === selectedItemId);
  if (!exists) {
    setSelectedItemId(null);
  }
}, [filteredItems, selectedItemId]);

これも同様にsetFilteredItemsからの連鎖でsetSelectedItemIdが走って、不要な再レンダリングが発生します。加えて、詳細パネルを表示してる状態でフィルタを変えると、パネルが一瞬だけ前のアイテムを表示してから閉じる、というちらつきが出てました。useEffectはコミット後に走るから、DOMには古い選択状態が1フレーム描画されます。

解決策は、selectedItemIdはそのまま持っておいて、selectedItem自体を計算で求めることです。

const [selectedItemId, setSelectedItemId] = useState<string | null>(null);

// 計算された選択アイテム。一覧に存在しなければ自動的に null になる
const selectedItem = filteredItems.find(item => item.id === selectedItemId) ?? null;

UI側ではselectedItemIdを直接参照せず、常にselectedItemを参照します。こうならuseEffectが完全にゼロになりました。selectedItemId自体は「最後に選んだID」として残るから、フィルタを解除したときに選択が復活します。これは仕様として決めてません。UX的にはどちらとも取れるから、いまは残す方向で放置してます。

親への件数通知をやめる

親への件数通知onFilterApplied(count)の問題。親が子の件数を知る必要があるからコールバックを呼んでましたけど、これもuseEffect内で呼ぶと親の再レンダリングをトリガーして危険です。

そもそも親がフィルタ後の件数を必要としてるなら、フィルタの状態(query, activeCategory)自体を親に持ち上げるか、親側でフィルタリングを行って子にはfilteredItemsをそのままpropsとして渡すべきです。

設計を見直して、フィルタ条件を親に引き上げるか、URLのクエリパラメータに任せます。URL(SearchParams)をSingle Source of Truthにするのが一番安全でした。

// useSearchParams を使用して URL から取得
const [searchParams, setSearchParams] = useSearchParams();
const query = searchParams.get('q') ?? '';
const activeCategory = searchParams.get('cat') ?? 'all';
const currentPage = Number(searchParams.get('page') ?? '1');

// stateがそもそもコンポーネント内にないから、状態同期の必要性自体が消滅する
const filteredItems = items.filter(/* 同様のフィルタ処理 */);

ただしこれもコストがあります。setSearchParamsをデフォルトのpushで呼ぶと、1文字入力するたびにhistoryエントリが積まれて、戻るボタンで1文字ずつ消えます。{ replace: true }にすると戻るボタンでフィルタ変更前まで戻れなくなります。どちらが正しいかはプロダクト判断で、いまはreplace: trueにして戻る挙動を犠牲にしてます。あとで見返す予定です。

Number(searchParams.get('page') ?? '1')は、?page=のように値が空文字だとNumber('') === 0になります。safeCurrentPageのクランプで実害は出ないですけど、currentPageが0のままsetSearchParamsに渡る経路が残ってると?page=0が生成されます。

残っている確認項目

  • propsから計算できる値にuseStateを使わない。
  • 他のstateの変更をトリガーにして別のstateを更新するためにuseEffectを使わない。
  • データの加工・変換はレンダリング時に直接計算する(処理が重いときだけuseMemo)。
  • 無効な状態のフォールバックはレンダリング時の計算値で丸める。
  • 状態の変更に合わせてリセットするときは、副作用に頼らずハンドラ側でまとめて実行する。

useEffectは消えました。レンダリング回数は文字入力1回につき1回に収まって、Profilerで確認済みです。

ただ、sortを入れたときに[...items].sort()のコピーが毎回走る問題がまだ未検証です。1万件のコピーとソートを毎キーストロークでやると、filterの2msとは桁が変わる可能性があります。計測用のダミーデータをソート済みにして渡す前提でいいのか、サーバー側でソート順を固定できるのか。ソートキーがUIから変更可能なら結局クライアントで並べ替える必要があります。

あと、useSearchParamsをstartTransitionで包まないと、重いリストの再計算中に入力フィールドが固まります。React 18の自動バッチングでは吸収しきれません。useDeferredValueをqueryに対して使う案もありますけど、URLがSingle Source of Truthになってる以上、queryは毎レンダリングでURLから読み直されます。入力フィールドにdeferredQueryを表示すればURLの?q=と表示がズレて、queryを表示すれば遅延の意味がなくなる。setSearchParamsのタイミングとdeferredQueryの追従が噛み合わず、URLが入力より先行する瞬間が生じます。ここは設計を一段戻す必要があるかもしれません。

?page=0が生成される経路を潰すために、setSearchParamsを呼ぶ箇所を全部洗い出してるところです。