diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js index cf5cc369..4fd99ee6 100644 --- a/frontend/eslint.config.js +++ b/frontend/eslint.config.js @@ -569,15 +569,15 @@ export default tseslint.config( // bulk `--suppress-all` would hide the next violation instead of // answering it, and it cannot hold a warning-level rule anyway. // - // `refs` is off because it fires on this app's popover architecture, not on its history, so - // suppressing it would not help the next popover anyone writes. Measured over the 217: - // 79 are `pop.refs.setReference` / `getReferenceProps` — @floating-ui/react's callback-ref - // setters, which are the documented way to use it — and 74 more are passing the popover handle - // down as a prop (`popover={pop}`, `ref={fade.ref}`), which is what `components/ui`'s popover - // engine is FOR. That is 153 of 217 flagged for having the word `refs` in an object that is not - // a React ref. The remaining ~64 are genuine render-phase `someRef.current = x` writes and are - // real (E27 S2 answered the ones the other rules could see, at their sites). - // Re-evaluate when the rule can: it shipped in 1.0.0 and this looks like over-approximation. + // `refs` is OFF — the one rule here switched off while it has real hits, and recorded as such + // in docs/gates.md. E27 turned it off calling 153 of its 217 hits false positives (the popover + // engine's `pop.refs.setReference` / `getReferenceProps()`, a popover handle passed as a prop) + // and the other ~64 real render-phase `someRef.current = x` writes. The E32 audit showed the + // triage was wrong: the rule and the React Compiler share one analysis, so every one of those + // "false positives" is a component the compiler SKIPS — 21 popover components among them, and + // Channels' 0.9 s row menu was the bill. It stays off only because 245 hits (measured E32 S1) cannot be fixed in + // one sprint; until they are, compilerCoverage.contract.test.ts pins every skipped function + // (none may be added), and E32 S3+ burns them down to turn this back on. "react-hooks/refs": "off", "local/jsx-comment-text": "error", "local/focus-safe-disabled": "error", diff --git a/frontend/reactPlugin.ts b/frontend/reactPlugin.ts index ce3ddc66..8a1f2843 100644 --- a/frontend/reactPlugin.ts +++ b/frontend/reactPlugin.ts @@ -11,8 +11,14 @@ import reactCompiler from "babel-plugin-react-compiler"; // // The React Compiler auto-memoises: it infers what `useCallback`/`useMemo` were being hand-written // for and applies the equivalent itself. It is enabled repo-wide with no opt-out list because it -// bails out on its own for anything it cannot prove safe (its rules are also linted, via -// eslint-plugin-react-hooks). `target` follows the installed React major and must move with it. +// bails out on its own for anything it cannot prove safe — SILENTLY: `panicThreshold` is "none" and +// there is no logger here, so a skipped component still works and simply re-renders everything under +// it. This comment used to say "its rules are also linted"; it was false while `react-hooks/refs` was +// off, and 95 functions sat uncompiled behind it (E32). What actually holds the line now is +// src/components/compilerCoverage.contract.test.ts, which runs THIS compiler over the shipped source +// and fails on any skip not on its list; the react-hooks lint rules are the early warning, with the +// one still switched off recorded in docs/gates.md. `target` follows the installed React major and +// must move with it. // // The plugin is IMPORTED rather than named by the usual `"babel-plugin-react-compiler"` string. Two // reasons, both about the gate: a string is invisible to knip, which then reports the dependency as diff --git a/frontend/src/components/compilerCoverage.contract.test.ts b/frontend/src/components/compilerCoverage.contract.test.ts index 2cd61f43..3950bdbe 100644 --- a/frontend/src/components/compilerCoverage.contract.test.ts +++ b/frontend/src/components/compilerCoverage.contract.test.ts @@ -1,66 +1,142 @@ -import { readFileSync } from "node:fs"; -import { transformSync } from "@babel/core"; -import reactCompiler from "babel-plugin-react-compiler"; import { describe, expect, it } from "vitest"; +import { compileReport, shippedSources, type CompileReport } from "./compilerSkips.testkit"; // 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. +// still works, it just re-renders everything under it on every render. 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. The E32 audit then found 95 such functions across src. // -// 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"]; +// A RATCHET over everything the build compiles (E32 S1). KNOWN_SKIPS is the exact set the compiler +// skips today, `file → function → the compiler's category`; everything else must compile. Both +// directions are red: +// • a NEW skip (or a known one changing category) — fix the function; if it truly cannot be +// compiled, adding it here is a decision for the owner, with the reason in docs/gates.md; +// • a skip that is GONE — delete its entry, so the list only ever shrinks. +// E32 S3+ burns it down: the popover engine (Refs: `getReferenceProps()` / `refs.setReference` +// passed during render), render-phase ref writes, try/finally (Todo), the two exhaustive-deps +// suppressions. The end state is IncompatibleLibrary alone (VirtualGrid on TanStack Virtual). +const KNOWN_SKIPS: CompileReport["skipped"] = { + "components/AboutCell.tsx": { AboutCell: "Refs" }, + "components/ActiveFilterChips.tsx": { OverflowPopover: "Refs" }, + "components/AddToPlaylist.tsx": { AddToPlaylist: "Refs", MembershipPanel: "Todo" }, + "components/BootGate.tsx": { BootGate: "Refs" }, + "components/ChannelStatsPopover.tsx": { ChannelStatsPopover: "Refs" }, + "components/ChannelTags.tsx": { + ChannelTagsControl: "Refs", + SuggestedChipsLane: "Refs", + TagEditorMenu: "Refs", + }, + "components/ChannelsProvider.tsx": { ChannelsProvider: "Refs" }, + "components/ChatThread.tsx": { ChatThread: "Refs" }, + "components/ConfigPanel.tsx": { ConfigPanel: "Refs" }, + "components/DateFilterSelect.tsx": { DateFilterSelect: "Refs" }, + "components/DownloadCenter.tsx": { DownloadCenter: "Todo" }, + "components/DownloadDialog.tsx": { DownloadDialog: "Todo" }, + "components/Feed.tsx": { Feed: "Immutability" }, + "components/IssueComposer.tsx": { Attachments: "Immutability", ComposerBody: "Refs" }, + "components/KeyGate.tsx": { KeyGate: "Todo" }, + "components/LanguageSwitcher.tsx": { LanguageSwitcher: "Refs" }, + "components/Messages.tsx": { ConversationList: "Refs" }, + "components/NavSidebar.tsx": { NavSidebar: "Refs" }, + "components/NavigationProvider.tsx": { NavigationProvider: "Refs" }, + "components/OnboardingWizard.tsx": { OnboardingWizard: "Refs" }, + "components/PersonalViewsModal.tsx": { PersonalViewsModal: "Refs" }, + "components/PlayerModal.tsx": { PlayerModal: "Todo" }, + "components/PlaylistLimitsHint.tsx": { PlaylistLimitsHint: "Refs" }, + "components/Playlists.tsx": { Playlists: "Immutability" }, + "components/PlaylistsRail.tsx": { PlaylistsRail: "Todo" }, + "components/PlexBrowse.tsx": { PlexBrowse: "Immutability" }, + "components/PlexCollectionEditor.tsx": { PlexCollectionEditor: "Todo" }, + "components/PlexCollectionView.tsx": { PlexCollectionView: "Todo" }, + "components/PlexPlayer.tsx": { PlexPlayer: "Suppression" }, + "components/PlexPlaylistAdd.tsx": { PlexPlaylistAdd: "Todo" }, + "components/PlexPlaylistView.tsx": { PlexPlaylistView: "Refs" }, + "components/PlexStatsOverlay.tsx": { PlexStatsOverlay: "Todo" }, + "components/SavedViewsPopover.tsx": { SavedViewsPopover: "Refs" }, + "components/SavedViewsProvider.tsx": { SavedViewsProvider: "Todo" }, + "components/SettingsPanel.tsx": { SignInMethods: "Todo" }, + "components/SetupWizard.tsx": { SetupWizard: "Todo" }, + "components/SyncStatus.tsx": { SyncStatus: "Refs" }, + "components/TagFilterChips.tsx": { TagFilterChips: "Refs" }, + "components/TagManager.tsx": { TagManager: "Refs", TagRow: "Refs" }, + "components/UpNextCard.tsx": { UpNextCard: "Refs" }, + "components/UploadDialog.tsx": { UploadDialog: "Todo" }, + "components/VideoEditor.tsx": { VideoEditor: "Refs" }, + "components/ViewSwitcher.tsx": { ViewSwitcher: "Refs" }, + "components/WatchPage.tsx": { WatchPage: "Todo", useMediaSession: "Todo" }, + "components/Welcome.tsx": { AuthCard: "Todo", Lightbox: "Refs" }, + "components/YearFilter.tsx": { YearFilter: "Refs" }, + "components/plexViews.tsx": { PlexSeasonView: "Todo", PlexShowView: "Todo" }, + "components/ui/ActionMenu.tsx": { ActionMenu: "Refs" }, + "components/ui/AnchoredPopover.tsx": { PopoverSurfaceInner: "Refs" }, + "components/ui/Combobox.tsx": { Combobox: "Refs" }, + "components/ui/FilterSelect.tsx": { FilterSelect: "Refs" }, + "components/ui/FocusSafeButton.tsx": { FocusSafeButton: "Refs" }, + "components/ui/Modal.tsx": { Modal: "Refs" }, + "components/ui/NavRail.tsx": { NavRailGroup: "Refs" }, + "components/ui/PageScroller.tsx": { PageScroller: "Refs" }, + "components/ui/PriorityStepper.tsx": { PriorityStepper: "Refs" }, + "components/ui/SidePanel.tsx": { SidePanel: "Refs" }, + "components/ui/SortControl.tsx": { SortControl: "Refs" }, + "components/ui/Toaster.tsx": { Toaster: "Globals" }, + "components/ui/Tooltip.tsx": { Tooltip: "Refs" }, + "components/ui/VirtualGrid.tsx": { VirtualGrid: "IncompatibleLibrary" }, + "components/ui/anchoredPopoverHooks.ts": { useAnchoredPopover: "Refs" }, + "components/ui/sortableA11y.ts": { useSortableA11y: "Immutability" }, + "components/ui/tableHeader.tsx": { FilterFunnel: "Refs", FilterOptionList: "Refs" }, + "components/useDebugRecording.ts": { useDebugRecording: "Todo" }, + "components/useDownloadActions.tsx": { useDownloadActions: "Todo" }, + "components/usePlaylistActions.tsx": { usePlaylistActions: "Todo" }, + "components/usePlexPlaylistEdits.ts": { usePlexPlaylistEdits: "Todo" }, + "lib/history.ts": { useBackToClose: "Refs", useBackToExit: "Refs", useHistorySubview: "Refs" }, + "lib/keymap.ts": { useGlobalKeymap: "Refs" }, + "lib/layerStack.ts": { useLayer: "Refs" }, + "lib/listNav.ts": { useListNav: "Refs" }, + "lib/playerModalHooks.ts": { useYtBigView: "Refs", useYtProgress: "Suppression" }, + "lib/plexPlayerHooks.ts": { useAutoSkip: "Refs", usePlexTracks: "Todo" }, + "lib/useChannelPriority.ts": { useChannelPriority: "Refs" }, + "lib/useChipOverflowFit.ts": { useChipOverflowFit: "Refs" }, + "lib/useDismiss.ts": { useDismiss: "Refs" }, + "lib/usePref.ts": { usePref: "Refs" }, + "lib/useUndoable.ts": { useUndoable: "Refs" }, + "lib/useUrlMirror.ts": { useUrlMirror: "Refs" }, +}; -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; -} +// The files a regression hurt most: compiled, and they must stay so (a positive check too, so a +// harness that compiled nothing could not pass as "no skips"). +const MUST_COMPILE: Record = { + "components/Channels.tsx": ["Channels"], + "components/ui/DataTable.tsx": ["DataTable"], +}; + +type Row = [file: string, fn: string, category: string]; +const rows = (skips: CompileReport["skipped"]): Row[] => + Object.entries(skips).flatMap(([file, fns]) => + Object.entries(fns).map(([fn, category]): Row => [file, fn, category]), + ); describe("the React Compiler", () => { - it.each(MUST_COMPILE)("compiles every function in %s", (path) => { - expect(bailouts(path)).toEqual([]); + // ~8 s for the whole tree, once, shared by the checks below. + const report = compileReport(shippedSources()); + const actual = rows(report.skipped); + const known = rows(KNOWN_SKIPS); + const key = ([f, fn]: Row) => `${f} → ${fn}`; + + it("skips nothing new (fix it, or the owner decides to list it)", () => { + const knownAs = new Map(known.map((r) => [key(r), r[2]])); + expect( + actual.filter((r) => knownAs.get(key(r)) !== r[2]).map((r) => `${key(r)}: ${r[2]}`), + ).toEqual([]); + }); + + it("lists no skip that has gone (delete it — the list only shrinks)", () => { + const actualAs = new Map(actual.map((r) => [key(r), r[2]])); + expect(known.filter((r) => !actualAs.has(key(r))).map(key)).toEqual([]); + }); + + it.each(Object.entries(MUST_COMPILE))("compiles %s", (file, fns) => { + expect(report.skipped[file] ?? {}).toEqual({}); + for (const fn of fns) expect(report.compiled[file]).toContain(fn); }); }); diff --git a/frontend/src/components/compilerSkips.testkit.ts b/frontend/src/components/compilerSkips.testkit.ts new file mode 100644 index 00000000..d42731e3 --- /dev/null +++ b/frontend/src/components/compilerSkips.testkit.ts @@ -0,0 +1,124 @@ +import { readdirSync, readFileSync } from "node:fs"; +import { parseSync, transformSync, type types as t } from "@babel/core"; +import reactCompiler from "babel-plugin-react-compiler"; + +// Runs the real React Compiler — the options reactPlugin.ts gives it — over the shipped source and +// reports every function it SKIPS, keyed `file → function → category`. The compiler skips without a +// word (`panicThreshold` is "none" and the build has no logger), so this is the only place a skip is +// visible at all. Used by compilerCoverage.contract.test.ts. + +const SRC = new URL("../", import.meta.url); + +/** Every file the production build compiles: src, minus tests and test kits. */ +export function shippedSources(): string[] { + return (readdirSync(SRC, { recursive: true }) as string[]) + .map((p) => p.split("\\").join("/")) + .filter( + (p) => /\.(ts|tsx)$/.test(p) && !/\.(test|testkit)\.tsx?$/.test(p) && !p.endsWith(".d.ts"), + ) + .sort(); +} + +type Node = t.Node & { [k: string]: unknown }; +const isFn = (n: Node) => + n.type === "FunctionDeclaration" || + n.type === "FunctionExpression" || + n.type === "ArrowFunctionExpression"; + +/** The name a reader would look a function up by: its own, the variable it is assigned to (through + * a `memo(…)` / `forwardRef(…)` wrapper), or `default` for an anonymous default export. */ +function functionNames(ast: t.File): Map { + const names = new Map(); + const walk = (node: Node, parents: Node[]) => { + if (isFn(node) && node.loc) { + let name = (node as t.FunctionDeclaration).id?.name ?? null; + for (let i = parents.length - 1; !name && i >= 0; i--) { + const p = parents[i]!; + if (p.type === "VariableDeclarator" && p.id.type === "Identifier") name = p.id.name; + else if (p.type === "ExportDefaultDeclaration") name = "default"; + else if (p.type !== "CallExpression" && p.type !== "TSAsExpression") break; + } + names.set( + `${node.loc.start.line}:${node.loc.start.column}`, + name ?? ``, + ); + } + for (const key of Object.keys(node)) { + if (key === "loc" || key === "leadingComments" || key === "trailingComments") continue; + const v = node[key]; + if (Array.isArray(v)) { + for (const c of v) + if (c && typeof c === "object" && "type" in c) walk(c as Node, [...parents, node]); + } else if (v && typeof v === "object" && "type" in v) walk(v as Node, [...parents, node]); + } + }; + walk(ast as unknown as Node, []); + return names; +} + +type Detail = { + category?: string; + reason?: string; + options?: { category?: string; reason?: string }; +}; + +export interface CompileReport { + /** file → function → the compiler's category for the skip (Refs, Todo, Immutability, …). */ + skipped: Record>; + /** file → the functions it did compile. */ + compiled: Record; +} + +export function compileReport(files: string[]): CompileReport { + const skipped: CompileReport["skipped"] = {}; + const compiled: CompileReport["compiled"] = {}; + for (const file of files) { + const source = readFileSync(new URL(file, SRC), "utf8"); + const opts = { filename: file, babelrc: false, configFile: false } as const; + const ast = parseSync(source, { ...opts, parserOpts: { plugins: ["jsx", "typescript"] } }); + if (!ast) throw new Error(`could not parse ${file}`); + const names = functionNames(ast); + const nameAt = (loc: t.SourceLocation | null) => + loc + ? (names.get(`${loc.start.line}:${loc.start.column}`) ?? ``) + : ""; + transformSync(source, { + ...opts, + parserOpts: { plugins: ["jsx", "typescript"] }, + plugins: [ + [ + reactCompiler, + { + target: "19", + logger: { + logEvent: ( + _f: string | null, + e: { + kind: string; + fnLoc?: t.SourceLocation | null; + detail?: Detail; + reason?: string; + }, + ) => { + if (e.kind === "CompileSuccess") { + (compiled[file] ??= []).push(nameAt(e.fnLoc ?? null)); + return; + } + let category: string; + if (e.kind === "CompileError") + category = e.detail?.options?.category ?? e.detail?.category ?? "Unknown"; + else if (e.kind === "CompileSkip") category = `Skip: ${e.reason}`; + else if (e.kind === "PipelineError") category = "PipelineError"; + else return; + const fns = (skipped[file] ??= {}); + // The first reason is the one that stopped it; later ones are the same function. + fns[nameAt(e.fnLoc ?? null)] ??= category; + }, + }, + }, + ], + ], + }); + } + return { skipped, compiled }; +}