perf(channels): the row menu opens and closes without re-rendering the table

The React Compiler silently skipped Channels and DataTable (a ref written during render; the refs lint is off), and even once compiled it dropped the memo of the rows and columns (they reach filterRows/optionsFromColumns past the page's hooks) and the columns read the open row. Every open, close and reposition of the right-click menu rebuilt the whole table: on "All" rows ~0.9 s to open and ~0.6 s to close (dev). Now: effect events instead of the render-written refs, explicit memos for rows, columns and their inputs, a module-scope keyOf, and the open row reaches its "…" through a small store. Measured on 397 rows (dev): ~75 ms to open, ~35 ms to close. A contract test runs the real compiler over both files and fails on any skipped function. The dropped memos have no automated guard: they need the full page rendered with its providers; the measurement above is the evidence.
2026-10-03 05:10:52 +02:00
parent 3e96211db0
commit cdfe7a8986
6 changed files with 295 additions and 125 deletions
@@ -23,6 +23,7 @@
        "react-i18next": "^15.7.4"
      },
      "devDependencies": {
+        "@babel/core": "7.29.7",
        "@eslint/js": "9.39.1",
        "@playwright/test": "^1.61.1",
        "@tailwindcss/postcss": "^4.3.3",
@@ -39,6 +39,7 @@
    "react-i18next": "^15.7.4"
  },
  "devDependencies": {
+    "@babel/core": "7.29.7",
    "@eslint/js": "9.39.1",
    "@playwright/test": "^1.61.1",
    "@tailwindcss/postcss": "^4.3.3",
@@ -1,4 +1,12 @@
-import { useEffect, useEffectEvent, useMemo, useRef, useState } from "react";
+import {
+  useCallback,
+  useEffect,
+  useEffectEvent,
+  useMemo,
+  useRef,
+  useState,
+  useSyncExternalStore,
+} from "react";
 import { useTranslation } from "react-i18next";
 import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
 import { qk } from "../lib/queryKeys";
@@ -76,6 +84,57 @@ const LAYOUT_ICON: Record<ChannelLayout, typeof Table> = {
  cards: LayoutGrid,
 };

+// The row menu's identity for a channel. Module scope, as useContextMenu asks: a fresh function per
+// render rebuilt its `buttonProps` on every render, and with them every column.
+const channelKey = (c: ManagedChannel) => c.id;
+
+// Which channel's row menu is open, for its "…" (`aria-expanded`, and staying shown while open). A
+// tiny store rather than a context: a context change walks the whole table to find its readers (~20
+// ms on 400 rows, each open and close), while a store calls its subscribers and re-renders only the
+// two buttons whose answer changed.
+function createOpenRowStore() {
+  let open: string | null = null;
+  const listeners = new Set<() => void>();
+  return {
+    get: () => open,
+    set: (next: string | null) => {
+      if (next === open) return;
+      open = next;
+      listeners.forEach((l) => l());
+    },
+    subscribe: (l: () => void) => {
+      listeners.add(l);
+      return () => void listeners.delete(l);
+    },
+  };
+}
+type OpenRowStore = ReturnType<typeof createOpenRowStore>;
+
+function RowMoreButton({
+  id,
+  label,
+  menuProps,
+  register,
+  openRow,
+}: {
+  id: string;
+  label: string;
+  menuProps: Record<string, unknown>;
+  register: (id: string, el: HTMLButtonElement | null) => void;
+  openRow: OpenRowStore;
+}) {
+  const mine = useSyncExternalStore(openRow.subscribe, () => openRow.get() === id);
+  return (
+    <MoreButton
+      ref={(el) => register(id, el)}
+      label={label}
+      expanded={mine}
+      menuProps={menuProps}
+      className={mine ? undefined : "reveal"}
+    />
+  );
+}
+
 export default function Channels() {
  const me = useMe();
  const panel = useTabPanel();
@@ -113,11 +172,16 @@ export default function Channels() {

  // Clicking a tag chip filters the feed by that tag and jumps there. Internalised (was an App
  // callback) now that Channels reads the feed filters + navigation straight from their providers.
-  const onFilterByTag = (tagId: number, name: string) => {
+  // useCallback (like `onView`): the column memo below names it, and the hook lint cannot see the
+  // compiler's memo of a plain closure.
+  const onFilterByTag = useCallback(
+    (tagId: number, name: string) => {
      setFeedFilters({ ...feedFilters, tags: [tagId], channelIds: [] });
      setPage("feed");
      notify({ level: "info", message: t("channels.notify.filteredByTag", { name }) });
-  };
+    },
+    [setFeedFilters, feedFilters, setPage, t],
+  );

  const channelsQuery = useQuery({ queryKey: qk.channels(), queryFn: api.channels });
  const tagsQuery = useTags();
@@ -143,7 +207,9 @@ export default function Channels() {
    qc.invalidateQueries({ queryKey: qk.feedSummary() });
  };

-  const userTags = (tagsQuery.data ?? []).filter((t) => !t.system);
+  // A memo for the same reason as the tag actions below: the column memo names it.
+  const allTags = tagsQuery.data;
+  const userTags = useMemo(() => (allTags ?? []).filter((t) => !t.system), [allTags]);

  // The row's stepper updates priority optimistically in place and does NOT re-sort/refetch, so the
  // list doesn't jump under a press-and-hold (the server list is priority-sorted; refetching would
@@ -181,7 +247,8 @@ export default function Channels() {
  // Tagging is updated optimistically in place (no refetch of the whole channel list, which
  // made the toggle feel ~2s slow); only revert by refetching if the server call fails. The
  // attach/detach + create-and-attach flow is shared with the channel detail page.
-  const { toggleTag, createAndAttachTag } = useChannelTagActions({
+  const tagActionOpts = useMemo<Parameters<typeof useChannelTagActions>[0]>(
+    () => ({
      optimisticToggle: (id, tagId, attach) =>
        qc.setQueryData<ManagedChannel[]>(qk.channels(), (old) =>
          (old ?? []).map((ch) =>
@@ -209,7 +276,12 @@ export default function Channels() {
        qc.invalidateQueries({ queryKey: qk.channels() });
        qc.invalidateQueries({ queryKey: qk.tagSuggestions() });
      },
-  });
+    }),
+    [qc],
+  );
+  // The options are a memo so the hook's toggleTag/createAndAttachTag stay the same functions: the
+  // column memo below names them (see `columns`).
+  const { toggleTag, createAndAttachTag } = useChannelTagActions(tagActionOpts);
  // --- Suggested tags (E8·S4): learned from the user's own manual-tag pattern, shown as accept/dismiss
  // ghost chips on untagged channels. Dismissal is session-only (an accepted suggestion self-clears —
  // the channel becomes tagged and the server drops it; a dismissed one just hides until reload). The
@@ -291,21 +363,19 @@ export default function Channels() {
  ];
  // A focus-channel intent (header "without full history" / tag manager) seeds the search box.
  // The TOKEN is the intent (it advances even when the same channel is re-focused); the name is the
-  // payload, so it rides a ref. Naming it would re-seed the box on any render that changed it,
-  // overwriting whatever the user had since typed.
+  // payload, so it is read through an effect event. Naming it would re-seed the box on any render that
+  // changed it, overwriting whatever the user had since typed.
  //
  // `consumeFocus` is what stops this page's OWN remounts from re-applying it: this effect runs on
  // every mount, and this page unmounts on every navigation away — and on opening a channel page,
  // which does not even change `page`. The provider then reports `focusName` as null, so a later
  // visit leaves the box exactly as the user left it.
-  const focusChannelNameRef = useRef(focusChannelName);
-  focusChannelNameRef.current = focusChannelName;
-  useEffect(() => {
-    const name = focusChannelNameRef.current;
-    if (!name) return;
-    setSearch(name);
+  const applyFocus = useEffectEvent(() => {
+    if (!focusChannelName) return;
+    setSearch(focusChannelName);
    consumeFocus();
-  }, [focusChannelToken, setSearch, consumeFocus]);
+  });
+  useEffect(() => applyFocus(), [focusChannelToken]);
  // The header's reset intent clears the search box (skip the initial mount); the tag chips ride in
  // `filters.tags`, which goToFullHistory already clears by setting a fresh map.
  const resetRef = useRef(filtersResetToken);
@@ -315,34 +385,25 @@ export default function Channels() {
    setSearch("");
  }, [filtersResetToken, setSearch]);

-  // Tag/priority/text filters stay controlled COLUMN filters (`filters.tags` etc.) shared by the top
-  // chips, the column funnels and the cards. The sync-status filter (`filters.sync`) is no longer a
-  // column — R14 retired the Sync column and folded its status into a ✓ charm on Stored — so it's
-  // applied as a pre-filter (`applySyncFilter`) over the rows fed to BOTH the table and the cards,
-  // still driven by the same top chips. Hidden channels have their own tab (excluded here). Only the
-  // name search filters at this step; the remaining column funnels run in DataTable / filterRows below.
-  const hiddenChannels = (channelsQuery.data ?? []).filter((c) => c.hidden);
-  const channels = (channelsQuery.data ?? []).filter((c) => !c.hidden);
-  const q = search.trim().toLowerCase();
-  const visibleChannels = channels.filter(
-    (c) => !q || `${c.title ?? ""} ${c.handle ?? ""}`.toLowerCase().includes(q),
+  const onView = useCallback(
+    (c: ManagedChannel) => onViewChannel(c.id, c.title ?? t("channels.row.thisChannel")),
+    [onViewChannel, t],
  );
-  // Sync-status pre-filter (top chips → `filters.sync`), applied before the table/cards see the rows.
-  // (Inline, not memoised: `visibleChannels` is a fresh array each render, so a useMemo keyed on it
-  // could never hit its cache — and applySyncFilter is a cheap single pass anyway.)
-  const syncedChannels = applySyncFilter(visibleChannels, filters.sync);
-  const s = statusQuery.data;
-
-  const onView = (c: ManagedChannel) =>
-    onViewChannel(c.id, c.title ?? t("channels.row.thisChannel"));
  // The channel's ONE action list (E28 C1) — ONE menu for the list, shared by the table and the card
  // layout: they are two paints of the same rows, so a right-click, a long-press and the row's "…"
  // must mean the same thing in both. Only mounted under the SUBSCRIBED branch below: a discovery
  // result is not a `Channel` row yet, so pinning or hiding one could only 404.
  // Keyed by id: a tag toggled in the menu, a priority picked, a poll — each rebuilds the row object
  // under the open menu, and its "…" must still know the menu is its own.
-  const rowMenu = useContextMenu<ManagedChannel>({ keyOf: (c) => c.id });
+  const rowMenu = useContextMenu<ManagedChannel>({ keyOf: channelKey });
  const rowPin = useComfortPin(rowMenu.open);
+  // The columns read only the menu's STABLE handles. Which row's menu is open reaches each "…" through
+  // the open-row store instead: read in the columns, it rebuilt every column on each open, close and
+  // reposition, and so re-rendered the whole table — ~0.8 s to open the menu on 400 rows.
+  const { buttonProps: rowButtonProps, triggerProps: rowTriggerProps } = rowMenu;
+  const openRowId = rowMenu.open ? (rowMenu.target?.id ?? null) : null;
+  const [openRow] = useState(createOpenRowStore);
+  useEffect(() => openRow.set(openRowId), [openRow, openRowId]);
  const channelActions = useChannelActions({
    onView,
    onOpenWizard,
@@ -361,33 +422,61 @@ export default function Channels() {
        pin: rowPin.item(pinChannel(menuRow.id), menuRow.title ?? t("channels.row.thisChannel")),
      })
    : [];
-  // Distinct priority levels present in the catalog → the priority column's filter options.
-  const prioOptions = [...new Set((channelsQuery.data ?? []).map((c) => c.priority))]
-    .sort((a, b) => b - a)
-    .map((p) => ({ value: String(p), label: String(p) }));

-  const columns = buildChannelColumns({
+  // Tag/priority/text filters stay controlled COLUMN filters (`filters.tags` etc.) shared by the top
+  // chips, the column funnels and the cards. The sync-status filter (`filters.sync`) is no longer a
+  // column — R14 retired the Sync column and folded its status into a ✓ charm on Stored — so it's
+  // applied as a pre-filter (`applySyncFilter`) over the rows fed to BOTH the table and the cards,
+  // still driven by the same top chips. Hidden channels have their own tab (excluded here). Only the
+  // name search filters at this step; the remaining column funnels run in DataTable / filterRows below.
+  //
+  // The table's rows and columns are explicit memos, not inline. Both later reach calls the React
+  // Compiler must assume may mutate them (`filterRows`, `sortRows`, `optionsFromColumns`) past this
+  // page's hooks, and a compiler memo cannot span a hook call — so inline, the compiler dropped their
+  // memo without a word, and every render (each open, close and reposition of the row menu) handed
+  // DataTable new rows and columns: ~0.9 s to open the menu on 400 rows. A useMemo result is frozen.
+  const hiddenChannels = (channelsQuery.data ?? []).filter((c) => c.hidden);
+  const allChannels = channelsQuery.data;
+  const q = search.trim().toLowerCase();
+  const syncFilterValue = filters.sync;
+  const syncedChannels = useMemo(
+    () =>
+      // Sync-status pre-filter (top chips → `filters.sync`), applied before the table/cards see the rows.
+      applySyncFilter(
+        (allChannels ?? []).filter(
+          (c) =>
+            !c.hidden && (!q || `${c.title ?? ""} ${c.handle ?? ""}`.toLowerCase().includes(q)),
+        ),
+        syncFilterValue,
+      ),
+    [allChannels, q, syncFilterValue],
+  );
+  const s = statusQuery.data;
+
+  const columns = useMemo(() => {
+    const registerMore = (id: string, el: HTMLButtonElement | null) => {
+      if (el) moreRefs.current.set(id, el);
+      else moreRefs.current.delete(id);
+    };
+    return buildChannelColumns({
      t,
-    lang: i18n.language,
-    prioOptions,
+      lang,
+      // Distinct priority levels present in the catalog → the priority column's filter options.
+      prioOptions: [...new Set((allChannels ?? []).map((c) => c.priority))]
+        .sort((a, b) => b - a)
+        .map((p) => ({ value: String(p), label: String(p) })),
      userTags,
      bumpPriority,
      setPriority,
-    more: (c) => {
-      const mine = rowMenu.open && rowMenu.target?.id === c.id;
-      return (
-        <MoreButton
-          ref={(el) => {
-            if (el) moreRefs.current.set(c.id, el);
-            else moreRefs.current.delete(c.id);
-          }}
+      more: (c) => (
+        <RowMoreButton
+          id={c.id}
          label={t("channels.row.menu")}
-          expanded={mine}
-          menuProps={rowMenu.buttonProps(c)}
-          className={mine ? undefined : "reveal"}
+          menuProps={rowButtonProps(c)}
+          register={registerMore}
+          openRow={openRow}
        />
-      );
-    },
+      ),
      onView,
      toggleTag,
      createAndAttachTag,
@@ -396,6 +485,23 @@ export default function Channels() {
      onAcceptSuggestion: acceptSuggestion,
      onDismissSuggestion: dismissSuggestion,
    });
+  }, [
+    t,
+    lang,
+    allChannels,
+    userTags,
+    bumpPriority,
+    setPriority,
+    rowButtonProps,
+    openRow,
+    onView,
+    toggleTag,
+    createAndAttachTag,
+    onFilterByTag,
+    suggestionsFor,
+    acceptSuggestion,
+    dismissSuggestion,
+  ]);

  // Status chips sit in the table's controls row (next to paging) — compact and close to
  // where they act.
@@ -748,7 +854,7 @@ export default function Channels() {
                onFiltersChange={setFilters}
                fixedLayout
                emptyText={t("channels.empty")}
-                rowProps={rowMenu.triggerProps}
+                rowProps={rowTriggerProps}
                // `group`: the row's "…" reveals on its hover or focus (`.reveal`), and stays on touch.
                rowClassName={() => "group"}
                keyboardNav
@@ -775,7 +881,7 @@ export default function Channels() {
                  aboutKey="about"
                  moreKey="actions"
                  emptyText={t("channels.empty")}
-                  rowProps={rowMenu.triggerProps}
+                  rowProps={rowTriggerProps}
                  rowClassName={() => "group"}
                />
                {/* Folded, the status / pager row leaves the band, as the table's does — and the
@@ -9,7 +9,7 @@
 // navigationContext (the sole runtime dependency) is mocked, pinned to the channels page.
 import { describe, it, expect, beforeEach, vi } from "vitest";
 import { renderHook, act, render as renderRoot } from "@testing-library/react";
-import { useEffect, useRef, type ReactNode } from "react";
+import { useEffect, useEffectEvent, type ReactNode } from "react";

 vi.mock("./navigationContext", () => ({
  useNavigation: () => ({ page: "channels" }),
@@ -135,18 +135,16 @@ describe("ChannelsProvider — S8 shareable URL", () => {
 // has to live in the PROVIDER — inside the page it would reset with the page and re-apply an intent
 // from earlier in the session over whatever the user has typed into the search box since.
 describe("ChannelsProvider — the focus-a-channel intent", () => {
-  /** Stands in for Channels.tsx:287-297 — the same effect, the same consume call. */
+  /** Stands in for Channels.tsx's `applyFocus` — the same effect event, the same consume call. */
  function Manager() {
    const { focusName, focusToken } = useChannels();
    const { setSearch, consumeFocus } = useChannelsActions();
-    const nameRef = useRef(focusName);
-    nameRef.current = focusName;
-    useEffect(() => {
-      const name = nameRef.current;
-      if (!name) return;
-      setSearch(name);
+    const applyFocus = useEffectEvent(() => {
+      if (!focusName) return;
+      setSearch(focusName);
      consumeFocus();
-    }, [focusToken, setSearch, consumeFocus]);
+    });
+    useEffect(() => applyFocus(), [focusToken]);
    return null;
  }

@@ -0,0 +1,66 @@
+import { readFileSync } from "node:fs";
+import { transformSync } from "@babel/core";
+import reactCompiler from "babel-plugin-react-compiler";
+import { describe, expect, it } from "vitest";
+
+// The React Compiler bails out of a component it cannot prove safe WITHOUT a word: the component
+// still works, it just re-renders everything under it on every render. Its main tripwire, a ref read
+// or written during render, is not linted here (`react-hooks/refs` is off, see eslint.config.js), so
+// nothing caught it. The channel manager is where it hurt (the owner, 2026-10-03): Channels and
+// DataTable both bailed out, every open, close and reposition of a row's right-click menu rebuilt
+// the rows and columns, and on "All" rows the menu took ~0.9 s to open and ~0.6 s to close.
+//
+// This runs the real compiler (the options reactPlugin.ts gives it) over the files that must stay
+// compiled and fails on any function it skips, with the compiler's own reason.
+const MUST_COMPILE = ["components/Channels.tsx", "components/ui/DataTable.tsx"];
+
+function bailouts(path: string): string[] {
+  const source = readFileSync(new URL(`../${path}`, import.meta.url), "utf8");
+  const found: string[] = [];
+  transformSync(source, {
+    filename: path,
+    babelrc: false,
+    configFile: false,
+    parserOpts: { plugins: ["jsx", "typescript"] },
+    plugins: [
+      [
+        reactCompiler,
+        {
+          target: "19",
+          logger: {
+            logEvent: (
+              _file: string | null,
+              e: { kind: string; fnLoc?: unknown; detail?: unknown },
+            ) => {
+              if (
+                e.kind !== "CompileError" &&
+                e.kind !== "CompileSkip" &&
+                e.kind !== "PipelineError"
+              )
+                return;
+              const loc = e.fnLoc as { start?: { line?: number } } | undefined;
+              const d = e.detail as
+                | { options?: { reason?: string; details?: { message?: string }[] } }
+                | { reason?: string; details?: { message?: string }[] }
+                | undefined;
+              const o =
+                d && "options" in d
+                  ? d.options
+                  : (d as { reason?: string; details?: { message?: string }[] });
+              found.push(
+                `line ${loc?.start?.line}: ${o?.details?.[0]?.message ?? o?.reason ?? e.kind}`,
+              );
+            },
+          },
+        },
+      ],
+    ],
+  });
+  return found;
+}
+
+describe("the React Compiler", () => {
+  it.each(MUST_COMPILE)("compiles every function in %s", (path) => {
+    expect(bailouts(path)).toEqual([]);
+  });
+});
@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useRef, useState, type ReactNode } from "react";
+import { useEffect, useEffectEvent, useRef, useState, type ReactNode } from "react";
 import { useTranslation } from "react-i18next";
 import { sortRows, cycleSort, type SortState } from "../../lib/columnSort";
 import Pager from "./Pager";
@@ -201,24 +201,23 @@ export default function DataTable<T>({
  useListNav(bodyRef, { enabled: keyboardNav && wide });
  useListNav(cardsRef, { enabled: keyboardNav && !wide });

-  // Commit a whole new filter map through the owner (controlled) or internal state. A ref holds the
-  // latest resolved map so the externalFilter/reset effects can derive from it without taking
-  // `filters` as a dep (which would re-run them on every filter change).
-  const filtersRef = useRef(filters);
-  filtersRef.current = filters;
-  // …and the same trick for the CALLBACK, one line further: a controlled caller almost always
-  // passes an inline arrow, so `onFiltersChange` is a fresh function on every render. Reading it
-  // through a ref is what lets `commitFilters` be stable, and a stable commitFilters is what lets
-  // the two effects below NAME it in their dep lists instead of switching the check off.
-  const onFiltersChangeRef = useRef(onFiltersChange);
-  onFiltersChangeRef.current = onFiltersChange;
-  const commitFilters = useCallback(
-    (next: FilterMap) => {
-      if (controlledFilters) onFiltersChangeRef.current!(next);
+  // Commit a whole new filter map through the owner (controlled) or internal state.
+  const commitFilters = (next: FilterMap) => {
+    if (controlledFilters) onFiltersChange!(next);
    else setInternalFilters(next);
-    },
-    [controlledFilters],
-  );
+  };
+  // The externalFilter/reset effects below read the latest filters and callback through effect
+  // events, so they re-run only on their own trigger, not on every filter change or on the inline
+  // `onFiltersChange` a controlled caller passes each render. (They used to ride render-written refs,
+  // which made the React Compiler skip this whole component — and with it every row's memo.)
+  const applyExternalFilter = useEffectEvent((key: string, value: string | string[]) => {
+    commitFilters({ ...filters, [key]: value });
+    setPage(0);
+  });
+  const clearFilters = useEffectEvent(() => {
+    commitFilters({});
+    setPage(0);
+  });

  // Persist page/size always, but only the sort/filter slices this table actually OWNS — a controlled
  // caller persists those itself, so writing them here would just leave a stale, never-read duplicate.
@@ -242,9 +241,9 @@ export default function DataTable<T>({
  const externalValue = externalFilter?.value;
  useEffect(() => {
    if (externalKey === undefined || externalValue === undefined) return;
-    commitFilters({ ...filtersRef.current, [externalKey]: externalValue });
-    setPage(0);
-  }, [externalKey, externalValue, commitFilters]);
+    // eslint-disable-next-line react-hooks/set-state-in-effect -- a controlled filter commits through the PARENT's setter, which may not run during this table's render; the effect fires once per new external value
+    applyExternalFilter(externalKey, externalValue);
+  }, [externalKey, externalValue]);

  // Clear column filters when the reset token advances. The ref starts at 0 (not at the prop)
  // so a navigation that bumped the token still clears even when it remounts this table fresh
@@ -254,9 +253,8 @@ export default function DataTable<T>({
  useEffect(() => {
    if (resetFiltersToken === undefined || resetFiltersToken === resetRef.current) return;
    resetRef.current = resetFiltersToken;
-    commitFilters({});
-    setPage(0);
-  }, [resetFiltersToken, commitFilters]);
+    clearFilters();
+  }, [resetFiltersToken]);

  const sorted = sortRows(filterRows(rows, columns, filters), columns, sort, lang);