From 09df2910c508e5c52d97fc40c0e98a12ce338752 Mon Sep 17 00:00:00 2001 From: Thomas Camlong Date: Tue, 11 Aug 2026 16:40:27 +0200 Subject: [PATCH] docs(perf): check the widgets against Vercel's React/Next performance rules Sourced with the skills CLI - npx skills find "react performance", then npx skills use affaan-m/ecc@react-performance, an adaptation of vercel-labs react-best-practices: 70+ rules across 8 categories. Every mechanically checkable rule was run against the widgets. Seven already hold, and that is worth recording as much as a finding: no widget registers a scroll listener, no numeric && can render a literal 0, the nested array scans that exist are over single-digit collections, React.cache() is applied, TanStack Query does the deduplication, content-visibility is on the six lists that need it, and the two components-declared-in-render were already found and fixed. One rule this codebase deliberately contradicts: the skill prefers React 19 over unmounting for tabs, and this pass did the opposite with keepMounted={false}. The rule optimises for switching; a dashboard optimises for mounting. Every board load mounts every widget while a user switches a widget's inner tabs rarely, so building a hidden monitoring view on every page load buys a faster switch nobody performs. Where state loss would show - custom-api's user JSX, the edit modal's forms - the panels stay mounted. Two candidates declined rather than guessed at, both recorded with reasoning: the 20+ `data = []` defaults only destabilise while data is undefined, since react-query returns a stable reference once loaded; and the 7 unlisted Mantine entry points need two production builds and a first-load-JS diff before anyone claims they help. @homarr/ui is not eligible at all despite 111 importers, because Next's barrel optimiser only rewrites pure re-export files. Also records the process error behind the earlier "no skills CLI" claim: the entries in ~/.claude/skills are symlinks, which find -type f skips, so the directory looked empty right after cat had succeeded on a file inside it. --- docs/perf/METHODOLOGY.md | 10 ++++++ docs/perf/NEXT-STEPS.md | 1 + docs/perf/WIDGET-AUDIT.md | 67 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/docs/perf/METHODOLOGY.md b/docs/perf/METHODOLOGY.md index 41e3644ed..9d1dda341 100644 --- a/docs/perf/METHODOLOGY.md +++ b/docs/perf/METHODOLOGY.md @@ -46,6 +46,16 @@ for the same image**, and 60+ MiB between runs of a byte-identical build. ## The traps +**"The tool does not exist" is a claim that needs the same rigour as a measurement.** Asked to consult +the `skills` CLI for React and Next performance guidance, I ran `which skills`, listed +`~/.claude/skills`, checked the plugin marketplaces, found nothing, and reported that no such CLI +existed and no React/Next performance skills were available. Both halves were false. `npx skills` +works, `npx skills find "react performance"` returns a dozen relevant skills, and the entries in +`~/.claude/skills` were **symlinks** into `~/.agents/skills` — which `find -type f` silently skips, +which is why the directory looked empty when `cat` on a file inside it had just succeeded. Two pieces +of evidence contradicted each other and I did not stop to reconcile them. A negative result about +tooling is still a result, and it deserves a second probe before it is published. + **A heap snapshot cannot explain a peak.** V8 runs a full GC before serialising one, so it reports survivors only. The 94 MiB of transient ArrayBuffers that drove this container past 500 MB never appeared in a single snapshot. Peaks need an allocation profiler and wrapped allocators. diff --git a/docs/perf/NEXT-STEPS.md b/docs/perf/NEXT-STEPS.md index a19356530..e09b5c2e8 100644 --- a/docs/perf/NEXT-STEPS.md +++ b/docs/perf/NEXT-STEPS.md @@ -81,6 +81,7 @@ Start from `analyze-require-graph.mjs`, which found the shared subtree, and | Lazy `mysql2` / `pg` | 21.4 MiB | Three implementations, all broken; see REJECTED.md. Structurally blocked because `createDb` must stay synchronous | | `beszelSystemStats` at 1,156 nodes | unknown | Second-heaviest widget, but its cost is chart SVG (`×317`), so reducing it changes what is drawn | | `documents` reads 18–32 | unknown | Never investigated; each document carries overhead | +| Add 7 Mantine entry points to `optimizePackageImports` | unknown | One line: `dates`, `dropzone`, `charts`, `spotlight`, `form`, `notifications`, `tiptap` are not listed, while `core`/`hooks`/`icons-react` are. Needs two production builds and a first-load-JS diff to know whether it does anything. `@homarr/ui` is **not** eligible despite 111 importers — Next's barrel optimiser only rewrites pure re-export files, and `packages/ui/src/index.ts` also defines a runtime value | ## 4. For the clean PR diff --git a/docs/perf/WIDGET-AUDIT.md b/docs/perf/WIDGET-AUDIT.md index a692c3f4e..ce718f903 100644 --- a/docs/perf/WIDGET-AUDIT.md +++ b/docs/perf/WIDGET-AUDIT.md @@ -133,6 +133,67 @@ configuration or by physical reality — disks, tunnels, configured instances image and no overlay. That is sound reasoning, not a reading, and a widget whose data source changes shape could invalidate it. +## Checked against Vercel's React/Next performance rules + +Sourced with the `skills` CLI — `npx skills find "react performance"`, then +`npx skills use affaan-m/ecc@react-performance`, which is an adaptation of +[Vercel Labs `react-best-practices`](https://github.com/vercel-labs/agent-skills): 70+ rules across +8 priority categories. Every rule that is mechanically checkable was run against the widgets. + +| rule | category | result | +| --- | --- | --- | +| `content-visibility: auto` for long lists | rendering | **applied** — 6 widgets | +| Don't define components inside components | re-render | **2 found, both fixed** (`PercentCell`, drag handle) | +| Ternary over `&&` for conditional render | rendering | **passes** — only 2 `&&` renders exist and both operands are already boolean or string, so nothing can render a literal `0` | +| Passive listeners for scroll | client fetching | **N/A** — no widget registers a scroll listener | +| `Set`/`Map` for membership over `Array.includes` | JS micro-perf | **passes** — the nested scans that exist are over configured integrations and column accessors, single digits either way. `use-persisted-table-layout` already uses a `Set` for the hot half | +| `React.cache()` for per-request dedup | server | **applied** — 4 functions | +| TanStack Query for dedup, not hand-rolled `useEffect` + `fetch` | client fetching | **passes** — used throughout | +| Direct imports, not barrels | bundle | **partial** — see below | +| Hoist default non-primitive props | re-render | **declined** — see below | +| `` for show/hide instead of mount/unmount | rendering | **deliberately contradicted** — see below | + +### The one rule this codebase deliberately contradicts + +The skill says to prefer React 19's `` over unmounting for tabs, because it avoids +remount cost. `keepMounted={false}`, which this pass added to two widgets, is exactly the opposite. + +That is a considered trade, not an oversight. The rule optimises for **switching**; a dashboard +optimises for **mounting**. Every board load mounts every widget, while a user switches a widget's +inner tabs rarely or never — so paying to build a hidden monitoring view on every single page load, and +holding its DOM for the whole session, buys a faster tab switch nobody performs. Where state loss +would be user-visible — `custom-api`'s user JSX, the edit modal's forms — the panels stay mounted. + +The cost is real and worth stating: switching tabs in `health-monitoring` or `media-missing` now +remounts the panel. react-query still has the data cached, so it repaints rather than reloads. + +### Bundle: partially satisfied + +`optimizePackageImports` already covers the three that matter by volume — `@mantine/core` (727 +imports), `@mantine/hooks` (69), `@tabler/icons-react`. Seven smaller Mantine entry points are not +listed: `dates` (13 files), `dropzone` (12), `charts` (10), `spotlight` (7), `form` (4), +`notifications` (3), `tiptap` (1). + +`@homarr/ui` is the largest barrel in the repo at 111 importers, and it is **not eligible**: Next's +barrel optimiser only rewrites files that consist purely of re-exports, and `packages/ui/src/index.ts` +also defines a runtime `uiConfiguration` value. Same for the other workspace barrels. + +Adding the seven Mantine entry points is a one-line change, but it is left undone rather than guessed +at: it needs two production builds and a first-load-JS comparison, and everything else in this work +that shipped without a measurement was later found to be wrong. Recorded in +[NEXT-STEPS.md](./NEXT-STEPS.md) instead. + +### `data = []` defaults: declined, with the reasoning + +The rule against fresh non-primitive defaults (`items ?? []` creating a new array identity per render) +matches **20+ sites**, all of the form `const { data = [] } = useQuery(...)`. + +It is declined because the window is narrow: react-query returns a stable reference once data has +loaded, so the fresh `[]` only appears while `data` is `undefined` — initial load, error, or no +integration configured. Outside that window the default never evaluates and nothing is destabilised. +20 edits to improve the render path of the loading state, with no measurement, is the same churn as the +29 inline-style sites already declined. + ## Investigated and declined, with reasons **Lazy-loading the shared `MaskedOrNormalImage`.** This looked like one change covering `releases`, @@ -162,5 +223,7 @@ The inventory first reported `downloads` — the top-ranked widget — as "12 ma counted `memo(` and not `useMemo(`. `downloads` is in fact one of the better-memoised widgets in the codebase. Fixed to count either form before the ranking was used for anything. -This is the sixth time in this work that a measurement, rather than the application, was the thing at -fault. See [`METHODOLOGY.md`](./METHODOLOGY.md). +That is the sixth time in this work that a measurement, rather than the application, was the thing at +fault — and this pass produced a seventh of a different kind: I reported that the `skills` CLI did not +exist on this machine after checking only `which` and the plugin directory. It does, and it is where +the Vercel rule set above came from. Both are recorded in [`METHODOLOGY.md`](./METHODOLOGY.md).