feat: Extends performance domain with React Compiler error triage and coverage antipatterns

domains/performance/skills/perf-hooks-effects/repos/metamask-extension.md

@@ -48,37 +48,39 @@ const TokenDisplay = ({ token }: TokenDisplayProps) => {
 
 **DO:**
 
-- Reduce dependencies by moving values to default parameters when possible
-- Only include dependencies that actually trigger the effect
+- Depend on the primitive the effect reads (an ID, an address, a number) rather than an object rebuilt on each render
+- Keep values the effect does not use out of the effect
 
 **DON'T:**
 
 - Include unnecessary dependencies that cause effects to run too often
+- Drop a value the effect reads to make it run less often. A default parameter does not stop it being a dependency (see Rule: Include All Dependencies in useEffect)
 
 **Example - WRONG:**
 
 ```typescript
-const TokenBalance = ({ address, network, refreshInterval }: Props) => {
+const TokenBalance = ({ account, network, refreshInterval }: Props) => {
   const [balance, setBalance] = useState('0');
 
   useEffect(() => {
     const fetch = async () => {
-      const result = await fetchBalance(address, network);
+      const result = await fetchBalance(account.address, network);
       setBalance(result);
     };
 
     fetch();
     const interval = setInterval(fetch, refreshInterval);
     return () => clearInterval(interval);
-  }, [address, network, refreshInterval]); // Effect runs too often
+  }, [account, network, refreshInterval]); // A parent passing account={{ address }} rebuilds it on every render, so the effect refetches and resets the interval each time
 };
 ```
 
 **Example - CORRECT:**
 
 ```typescript
-const TokenBalance = ({ address, network, refreshInterval = 10000 }: Props) => {
+const TokenBalance = ({ account, network, refreshInterval }: Props) => {
   const [balance, setBalance] = useState('0');
+  const { address } = account;
 
   useEffect(() => {
     const fetch = async () => {
@@ -89,7 +91,7 @@ const TokenBalance = ({ address, network, refreshInterval = 10000 }: Props) => {
     fetch();
     const interval = setInterval(fetch, refreshInterval);
     return () => clearInterval(interval);
-  }, [address, network]); // refreshInterval moved to default param
+  }, [address, network, refreshInterval]); // The address string is equal across renders
 };
 ```
 
@@ -344,7 +346,7 @@ const useHistoricalPrices = () => {
 **DO:**
 
 - Split effects when conditional logic excludes some dependencies
-- Ensure all dependencies in array are actually used
+- Ensure the effect reads every dependency (see Rule: Don't List Dependencies the Effect Doesn't Read)
 
 **DON'T:**
 
@@ -374,19 +376,61 @@ const useHistoricalPrices = ({ isEvm, chainId, address }: Props) => {
       fetchPrices(chainId, address);
     }
   }, [isEvm, chainId, address]); // All deps are used
+};
+```
+
+### Rule: Don't List Dependencies the Effect Doesn't Read
+
+The dependency array describes the effect's code. A value that must restart the effect when it changes has to be a value the effect uses.
+
+**DO:**
 
-  // OR Option 2: Separate effects
+- Treat a dependency the effect never reads as a finding, even when the effect is meant to restart when it changes
+- If the effect's work depends on the value, pass the value into that work: the call that starts the connection, subscription or background registration
+- If the value marks a scope whose state must all reset when it changes, render that scope as a component and give it the value as its `key`
+- If the value changes because of a specific action, do the work where that action is handled instead of in an effect that watches for the change
+
+**DON'T:**
+
+- Delete the dependency without one of the changes above. The effect then keeps a session bound to the old value
+- Keep the unread dependency and explain it in a comment. React Compiler's effect-dependency validation (`validateExhaustiveEffectDependencies`, lint rule `react-hooks/exhaustive-effect-dependencies`, off by default) reports it as unnecessary, and with that validation on, the compiler leaves the whole component or hook uncompiled, or fails the build under a strict `panicThreshold`
+- Add `'use no memo'` to protect the dependency array. It opts the whole function out of React Compiler, and React documents it as a temporary debugging tool
+- Add a parameter the called code ignores just so the effect reads the value. That moves the unread value into the call
+
+**Reference:** [Removing Effect Dependencies](https://react.dev/learn/removing-effect-dependencies), [Resetting all state when a prop changes](https://react.dev/learn/you-might-not-need-an-effect#resetting-all-state-when-a-prop-changes), [`"use no memo"`](https://react.dev/reference/react-compiler/directives/use-no-memo)
+
+**Example - WRONG:**
+
+```typescript
+const useSession = ({ address, networkId }: Props) => {
   useEffect(() => {
-    if (isEvm) return;
-    fetchPrices(chainId, address);
-  }, [isEvm]); // Only depends on condition
+    const session = startSession(address);
+    return () => session.stop();
+  }, [address, networkId]); // networkId is never read
+};
+```
+
+**Example - CORRECT:**
 
+```typescript
+// The session is for one network, so the effect passes the network in
+const useSession = ({ address, networkId }: Props) => {
   useEffect(() => {
-    if (!isEvm) {
-      fetchPrices(chainId, address);
-    }
-  }, [chainId, address]); // Only when not EVM
+    const session = startSession({ address, networkId });
+    return () => session.stop();
+  }, [address, networkId]);
 };
+
+// Caches must be cleared when the account or network changes:
+// a keyed child unmounts on the change, and its cleanup clears them
+const CacheScope = () => {
+  useEffect(() => () => clearCaches(), []);
+  return null;
+};
+
+const Root = ({ address, networkId }: Props) => (
+  <CacheScope key={`${address}:${networkId}`} />
+);
 ```
 
 ### Rule: Use useRef for Persistent Values
@@ -856,6 +900,76 @@ const PriceTicker = ({ tokenAddress }: PriceTickerProps) => {
 };
 ```
 
+### Rule: Keep Multi-Step Async Protocols Out of the Effect
+
+**DO:**
+
+- When an effect acquires an external resource through more than one async step, with a timeout or more than one way to end, move that protocol into a plain object owned outside React: `start()` returns a handle, and `handle.stop()` releases the resource
+- Keep the effect to acquiring and releasing: start in the body, stop in the cleanup
+- Unit-test the object's end paths directly (ready, timeout, failure, stop while a step is in flight), without rendering
+
+**DON'T:**
+
+- Grow a state machine of closure flags (`cancelled`, `ended`, `ready`) inside the effect. Every end path needs its own cleanup, every fix has to reason about every flag at once, and the only way to test it is through `renderHook`
+
+**Why:** every line of such an effect can pass the rules above (stable dependencies, timers cleared, a cancelled flag on the async chain) while the bugs sit between its end paths: a timeout that fires mid-acquire, a release sent before registration, a release that also runs on the success path. Those bugs belong to the protocol, not to any one line, so a line-level review does not catch them.
+
+**Example - WRONG:**
+
+```typescript
+useEffect(() => {
+  let cancelled = false;
+  let ended = false;
+  let ready = false;
+  const finish = (reason: string) => {
+    if (!ended) {
+      ended = true;
+      report(reason);
+    }
+  };
+  const timeout = setTimeout(() => {
+    cancelled = true;
+    finish('timeout');
+    release(id);
+  }, 30_000);
+  register(id)
+    .then(() => !cancelled && init(address))
+    .then(() => !cancelled && start(id))
+    .then(() => {
+      if (!cancelled) {
+        ready = true;
+        finish('ready');
+      }
+    })
+    .catch(() => {
+      finish('failed');
+      if (!cancelled) release(id);
+    })
+    .finally(() => clearTimeout(timeout));
+  return () => {
+    cancelled = true;
+    clearTimeout(timeout);
+    finish('released');
+    release(id, ready);
+  };
+}, [address]);
+```
+
+**Example - CORRECT:**
+
+```typescript
+// preload-session.ts: a plain object, unit-tested without React
+export function startPreloadSession(address: string): { stop: () => void } {
+  // register, init, start, the timeout, and every end path live here
+}
+
+// The hook only acquires and releases
+useEffect(() => {
+  const session = startPreloadSession(address);
+  return () => session.stop();
+}, [address]);
+```
+
 ### Rule: Avoid Large Object Retention in Closures
 
 **DO:**

domains/performance/skills/perf-react-compiler/repos/metamask-extension.md

@@ -694,6 +694,46 @@ const TokenList = ({ tokens, filter }: TokenListProps) => {
 };
 ```
 
+### ❌ Anti-Pattern: Dependencies the Effect Never Reads
+
+**Problem:** An entry the effect does not read is not a harmless extra. The compiler's
+effect-dependency validation treats it as an error and **skips the whole function**, so the
+hook ships with no memoization at all. A comment next to the array does not help: the check
+reads the array, not the prose.
+
+**Solution:** Make the value one the effect actually reads, by passing it into the work the
+effect starts. Where it marks a scope that must reset as a whole, put a `key` on a component
+instead. Never satisfy it with a comment, with `'use no memo'`, or with a parameter the callee
+ignores.
+
+**Example - WRONG:**
+
+```typescript
+useEffect(() => {
+  const session = manager.startPreload({ address, accountChanged });
+  return () => session.stop();
+}, [
+  address,
+  // Provider or network changes release the old preload and register a new one.
+  provider,
+  isTestnet,
+]);
+```
+
+**Example - CORRECT:**
+
+```typescript
+useEffect(() => {
+  // `provider` and `isTestnet` are now read, so the array describes the code.
+  const session = manager.startPreload({ address, accountChanged, provider, isTestnet });
+  return () => session.stop();
+}, [address, provider, isTestnet]);
+```
+
+**Why this lives here as well as in `perf-hooks-effects`:** the rule is stated there as an
+effects rule, and the cost is a React Compiler cost. Someone asking what loses them compiler
+coverage opens this file, so the rule has to be reachable from this side too.
+
 ### ❌ Anti-Pattern: Using Index as Key for Dynamic Lists
 
 **Problem:** Breaks React's reconciliation when lists can be reordered, filtered, or have items added/removed.

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): 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: '<value>_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, minus one gap: a module-scope `'use no memo'` directive still logs `CompileSuccess` for every function in the file before the directive discards the transform, so those files land here too, not in `skipped`. "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 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). A `'use no memo'` directive lands here only when it sits inside one function's body, logging `CompileSkip`. A module-scope one, at the top of the file, logs as `compiled` instead.
+
+## Staged adoption roadmap
+
+The extension's sequence 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*"`. A directive can be masking nothing: remove it and re-run the compiler, and zero new errors means it was not load-bearing.
+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, read against the `'use no memo'` opt-out count from the roadmap's audit step. A module-scope directive still logs as `compiled`, so the raw count alone can rise while real coverage doesn't. 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 function-body opt-outs belong there (a module-scope `'use no memo'` counts as `compiled`, not `skipped`). 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)