diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 3fa176cc..78063d2e 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -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", diff --git a/frontend/package.json b/frontend/package.json index fc8a554e..e9bd1f67 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -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", diff --git a/frontend/src/components/Channels.tsx b/frontend/src/components/Channels.tsx index 59730f94..9048789f 100644 --- a/frontend/src/components/Channels.tsx +++ b/frontend/src/components/Channels.tsx @@ -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 = { 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; + +function RowMoreButton({ + id, + label, + menuProps, + register, + openRow, +}: { + id: string; + label: string; + menuProps: Record; + register: (id: string, el: HTMLButtonElement | null) => void; + openRow: OpenRowStore; +}) { + const mine = useSyncExternalStore(openRow.subscribe, () => openRow.get() === id); + return ( + 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) => { - setFeedFilters({ ...feedFilters, tags: [tagId], channelIds: [] }); - setPage("feed"); - notify({ level: "info", message: t("channels.notify.filteredByTag", { name }) }); - }; + // 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,35 +247,41 @@ 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({ - optimisticToggle: (id, tagId, attach) => - qc.setQueryData(qk.channels(), (old) => - (old ?? []).map((ch) => - ch.id === id - ? { - ...ch, - tag_ids: attach ? [...ch.tag_ids, tagId] : ch.tag_ids.filter((x) => x !== tagId), - } - : ch, + const tagActionOpts = useMemo[0]>( + () => ({ + optimisticToggle: (id, tagId, attach) => + qc.setQueryData(qk.channels(), (old) => + (old ?? []).map((ch) => + ch.id === id + ? { + ...ch, + tag_ids: attach ? [...ch.tag_ids, tagId] : ch.tag_ids.filter((x) => x !== tagId), + } + : ch, + ), ), - ), - // Keep the channel DETAIL cache in step with the manager list (the detail page reads its tags - // from qk.channel(id)); mirrors the detail page invalidating the manager list on its side. Also - // recompute suggestions: a channel that just gained/lost a user tag changes the untagged set and - // the learned pattern (E8·S4). - onSuccess: (id) => { - qc.invalidateQueries({ queryKey: qk.channel(id) }); - qc.invalidateQueries({ queryKey: qk.tagSuggestions() }); - }, - // A failed attach/detach reverts the optimistic change by refetching the list. Also refetch - // suggestions: acceptSuggestion optimistically dropped the channel from that cache, and on - // failure the tag never actually attached — so the server still suggests it and the ghost chips - // must come back rather than leaving an empty cell (honest states). The hook reports the failure. - onError: () => { - qc.invalidateQueries({ queryKey: qk.channels() }); - qc.invalidateQueries({ queryKey: qk.tagSuggestions() }); - }, - }); + // Keep the channel DETAIL cache in step with the manager list (the detail page reads its tags + // from qk.channel(id)); mirrors the detail page invalidating the manager list on its side. Also + // recompute suggestions: a channel that just gained/lost a user tag changes the untagged set and + // the learned pattern (E8·S4). + onSuccess: (id) => { + qc.invalidateQueries({ queryKey: qk.channel(id) }); + qc.invalidateQueries({ queryKey: qk.tagSuggestions() }); + }, + // A failed attach/detach reverts the optimistic change by refetching the list. Also refetch + // suggestions: acceptSuggestion optimistically dropped the channel from that cache, and on + // failure the tag never actually attached — so the server still suggests it and the ghost chips + // must come back rather than leaving an empty cell (honest states). The hook reports the failure. + onError: () => { + 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({ keyOf: (c) => c.id }); + const rowMenu = useContextMenu({ 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,41 +422,86 @@ 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, + // 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) => ( + + ), + onView, + toggleTag, + createAndAttachTag, + onFilterByTag, + suggestionsFor, + onAcceptSuggestion: acceptSuggestion, + onDismissSuggestion: dismissSuggestion, + }); + }, [ t, - lang: i18n.language, - prioOptions, + lang, + allChannels, userTags, bumpPriority, setPriority, - more: (c) => { - const mine = rowMenu.open && rowMenu.target?.id === c.id; - return ( - { - if (el) moreRefs.current.set(c.id, el); - else moreRefs.current.delete(c.id); - }} - label={t("channels.row.menu")} - expanded={mine} - menuProps={rowMenu.buttonProps(c)} - className={mine ? undefined : "reveal"} - /> - ); - }, + rowButtonProps, + openRow, onView, toggleTag, createAndAttachTag, onFilterByTag, suggestionsFor, - onAcceptSuggestion: acceptSuggestion, - onDismissSuggestion: dismissSuggestion, - }); + 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 diff --git a/frontend/src/components/ChannelsProvider.dom.test.tsx b/frontend/src/components/ChannelsProvider.dom.test.tsx index d6521c39..c5b86cc9 100644 --- a/frontend/src/components/ChannelsProvider.dom.test.tsx +++ b/frontend/src/components/ChannelsProvider.dom.test.tsx @@ -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; } diff --git a/frontend/src/components/compilerCoverage.contract.test.ts b/frontend/src/components/compilerCoverage.contract.test.ts new file mode 100644 index 00000000..2cd61f43 --- /dev/null +++ b/frontend/src/components/compilerCoverage.contract.test.ts @@ -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([]); + }); +}); diff --git a/frontend/src/components/ui/DataTable.tsx b/frontend/src/components/ui/DataTable.tsx index 7a2eab8c..b65f8b19 100644 --- a/frontend/src/components/ui/DataTable.tsx +++ b/frontend/src/components/ui/DataTable.tsx @@ -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({ 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); - else setInternalFilters(next); - }, - [controlledFilters], - ); + // Commit a whole new filter map through the owner (controlled) or internal state. + const commitFilters = (next: FilterMap) => { + if (controlledFilters) onFiltersChange!(next); + else setInternalFilters(next); + }; + // 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({ 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({ 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);