diff --git a/domains/performance/skills/performance/references/mm-react-compiler-error-triage.md b/domains/performance/skills/performance/references/mm-react-compiler-error-triage.md new file mode 100644 index 00000000..68db17f8 --- /dev/null +++ b/domains/performance/skills/performance/references/mm-react-compiler-error-triage.md @@ -0,0 +1,90 @@ +--- +title: React Compiler Error Triage & Coverage Accounting (MetaMask) +impact: HIGH +tags: react-compiler, panicThreshold, error-triage, coverage, babel, build +--- + +# Skill: React Compiler Error Triage & Coverage Accounting + +The React Compiler **fails open**: when it can't compile a component, it silently skips it and ships the unoptimized original. The build stays green, DevTools shows no warning — you just don't get the memoization. Once the compiler is enabled broadly (metamask-mobile#31171 enabled v1.0.0 app-wide), the question stops being "is it on?" and becomes **"what is it actually compiling, and which of its errors are worth fixing?"** This file is the triage playbook. Extension PR metamask-extension#38007 is the reference implementation. + +## The `panicThreshold` ladder + +`panicThreshold` controls when a compiler diagnostic fails the build instead of silently skipping the file: + +| Setting | Build fails on | Use for | +|---|---|---| +| `'none'` (default) | never — every failed file is **silently skipped** | production builds, always | +| `'critical_errors'` | only critical errors (compiler-internal invariant violations) | CI / debug builds, first ratchet target | +| `'all_errors'` | every diagnostic, including unsupported syntax | CI / debug builds, end-state ratchet | + +**The ratchet strategy** (extension roadmap, MetaMask-planning#6552 → #6553): keep production at `'none'` permanently; aim for a *non-production* build that passes at `'critical_errors'`, then at `'all_errors'`. Each ratchet step turns a class of silent skips into a visible, fixable error list. Never enable a non-`'none'` threshold in a release build — one un-compilable file would block the release for an optimization that is optional by design. + +## Triage: unsupported syntax vs. legitimate errors + +Compiler diagnostics are **not one bucket**. The logger event's `category` field separates them, and the distinction decides whether you act: + +- **`category === 'Todo'` → "unsupported."** Syntax or a pattern the compiler *itself* has not implemented yet. There is **no actionable fix on our side** — rewriting working code to appease an unimplemented compiler path is wasted effort and churn. Count these separately, leave the code alone, and re-check after compiler upgrades. +- **Any other category (e.g. `InvalidReact`, `InvalidJS`) → legitimate, actionable.** A real Rules-of-React violation in our code (mutation during render, conditional hooks, side effects in render). Fixing it both unlocks compilation *and* removes a latent correctness bug. + +A healthcheck that doesn't make this split is noise: the `Todo` count swamps the actionable list and the team learns to ignore the output. The extension's verbose run at enablement (metamask-extension#38007) is the canonical illustration — of 7,308 files processed: 253 compiled, **31 actionable errors**, **7,024 unsupported** (`Todo`). Without the split that reads as ~7,000 hopeless errors; with it, the team's backlog is 31 files and the rest is the compiler's to burn down across upgrades. The extension's webpack wrapper makes the split in ~10 lines: + +```ts +// adapted from metamask-extension development/webpack/utils/loaders/reactCompilerLoaderWrapper.ts +// (mobile equivalent: pass a `logger` in babel-plugin-react-compiler options) +logger: { + logEvent(filename, event) { + switch (event.kind) { + case 'CompileSuccess': record(filename, 'compiled'); break; + case 'CompileSkip': record(filename, 'skipped'); break; + case 'CompileError': { + const category = event.detail?.options?.category ?? event.detail?.category; + // 'Todo' = not yet supported by the compiler — no actionable fix on our side + record(filename, category === 'Todo' ? 'unsupported' : 'error'); + break; + } + } + }, +} +``` + +The extension exposes this as `yarn webpack --reactCompilerVerbose` (per-file ✅/⏭️/🔍/❌ output + summary stats) and `--reactCompilerDebug={all|critical|none}` (maps to `panicThreshold: '_errors'`). On mobile the same taxonomy is available through the Babel plugin's `logger` option or `eslint-plugin-react-compiler` (the lint rule runs the same analysis the compiler does). + +## Coverage accounting + +Track four buckets — **compiled / skipped / errors / unsupported** — at file and component granularity, with **worst-status-wins per file** (`error > unsupported > skipped > compiled`): a file with five compiled components and one error is an *error file*, otherwise mixed files inflate the compiled count and the number lies to you. + +What the buckets tell you: + +- **compiled** — your real optimization coverage. "The compiler is enabled" claims nothing; this number does. +- **errors** — the actionable backlog. Each is a Rules-of-React fix. +- **unsupported** — the compiler's backlog, not yours. Trend it across compiler upgrades. +- **skipped** — intentional exclusions: test/story files, `'use no memo'` directives, and **class components** (never compiled — metamask-mobile#30919 counted 53 at full enablement; migration to function components is the only way to move them into the compiled bucket). + +## Staged adoption roadmap + +The extension's sequence (epic MetaMask-planning#6549) generalizes to any repo: + +1. **Lint clean:** update `eslint-plugin-react-hooks` / `eslint-plugin-react-compiler` to latest; fix violations — these are exactly what the compiler will refuse to compile. +2. **Audit opt-outs:** every `'use no memo'` carries a reason + TODO; the count only goes down. `grep -rn "use no memo" app --include="*.ts*"`. +3. **Ratchet `critical_errors`:** non-prod build passes; fix what surfaces. +4. **Ratchet `all_errors`:** remaining actionable errors fixed; what's left is the `Todo` (unsupported) set, which you wait out. + +## Verify + +- Per component: `Memo ✨` badge in React DevTools (see [js-profile-react.md](js-profile-react.md)). +- Per repo: the compiled-files count from the logger stats rises (or at least doesn't silently fall) release over release — silent coverage regressions are the failure mode this file exists to catch. +- After a compiler version bump: re-run the verbose build and diff the `unsupported` list — `Todo`s that became `compiled` are free wins; new `error`s are regressions to triage. + +## Don't over-correct + +- **Never "fix" a `Todo`.** Rewriting working code around an unimplemented compiler feature is churn with no perf evidence; the next compiler release may compile it as-is. +- Don't gate releases on compiler errors (`panicThreshold` stays `'none'` in production builds). +- Don't treat `skipped` as a problem — tests, stories, and deliberate opt-outs belong there. The smell is *unexplained* `'use no memo'` directives, not the bucket itself. +- A component without `Memo ✨` is not automatically a bug to chase — check the buckets first; it may be `unsupported`. + +## Related + +- [mm-react-compiler.md](mm-react-compiler.md) — enabling the compiler in this repo (Babel config, Metro cache, ESLint healthcheck) +- [js-react-compiler.md](js-react-compiler.md) — how the compiler transforms code; Rules-of-React background +- [mm-selector-cascade.md](mm-selector-cascade.md) — what the compiler **cannot** fix: unstable values crossing file boundaries (selectors, imported hooks) diff --git a/domains/performance/skills/performance/references/mm-react-compiler.md b/domains/performance/skills/performance/references/mm-react-compiler.md index e95d70d6..4f987cfb 100644 --- a/domains/performance/skills/performance/references/mm-react-compiler.md +++ b/domains/performance/skills/performance/references/mm-react-compiler.md @@ -51,6 +51,8 @@ React Compiler auto-memoizes components, callbacks, and computed values at build On opted-in paths you can gradually drop hand-written `useMemo`/`useCallback`/`React.memo` once the compiler is verified working — but do it deliberately and re-measure. Off opted-in paths, manual memoization still matters. +**Exception — effect dependencies.** Keep any `useMemo`/`useCallback` whose output is used as a `useEffect` dependency, here or in a consumer: the compiler's memoization is not guaranteed to match the manual strategy, and a mismatch causes over/under-firing of effects or infinite loops — a correctness change, not a perf tweak. Official guidance is to leave existing manual memoization in place and only omit it in *new* code ([reactwg/react-compiler#16](https://github.com/reactwg/react-compiler/discussions/16)). + ## What breaks compilation (it will skip the component) - Mutating props or state during render. @@ -68,4 +70,5 @@ Fix the ESLint `react-compiler` warnings on a path before/after opting it in. ## Related - [js-react-compiler.md](js-react-compiler.md) — upstream reference on how the compiler transforms code +- [mm-react-compiler-error-triage.md](mm-react-compiler-error-triage.md) — triaging compiler errors (`Todo`/unsupported vs actionable), `panicThreshold` ratcheting, and measuring real coverage - [mm-selector-memoization.md](mm-selector-memoization.md) — fix data-layer re-renders the compiler can't