Conversation
`mm-react-compiler-error-triage` — sorting compiler errors into `Todo` and unsupported versus actionable, ratcheting `panicThreshold`, and the `useMemo`/`useCallback` exception for effect dependencies, whose output the compiler's memoization does not preserve. Split out of #43: compiler adoption is a different subject from the render antipattern scans, and nothing in either scan cites it.
`babel-plugin-react-compiler` 1.0.0 logs `CompileSuccess` for every function in a file whose top-level directive then discards the transform. Only a directive inside one function's body logs `CompileSkip`. The compiled count therefore rises with opt-outs and is read against the opt-out count.
…s it metamask-mobile#31171 (enable the React Compiler across the Metro bundle) replaced the two-path allowlist. The plugin now applies to every file except under Jest, with no `target` override, so its default of React 19 applies. The triage reference also named two internal planning tickets.
…ng it Removing the directive and re-running the compiler settles it: zero new errors means the directive was masking nothing.
MajorLift
marked this pull request as ready for review
September 15, 2026 20:02
MajorLift
added a commit
that referenced
this pull request
Sep 18, 2026
…nd coverage antipatterns (#170) ## Motivation The React Compiler fails open. A component it cannot compile ships unoptimized with a green build and no warning, so the coverage you believe you have and the coverage you have are different numbers and nothing reports the gap. That makes two questions urgent, in that order. Which components is it skipping, and which patterns are quietly removing components from the set it compiles. ## Finding out what the compiler is skipping Raising `panicThreshold` to `'all_errors'` is how you see the skipped set, and it returns an error list that mixes unsupported-syntax noise with real bugs, so the list is only useful once it is sorted. - **`references/mm-react-compiler-error-triage.md`** sorts compiler errors into unsupported, which is the compiler's own `Todo` category, and actionable. It ratchets `panicThreshold` in non-production builds rather than setting it once, so the actionable set shrinks under a threshold that tightens instead of being read once and abandoned. - **`references/mm-react-compiler.md`** counts a file carrying a module-scope `'use no memo'` as compiled rather than skipped, which is the measurement error that makes a coverage figure read better than the build. - The same file describes the compiler as running app-wide on its default `target: '19'`, as set up in [metamask-mobile#31171 (fully enable React Compiler)](MetaMask/metamask-mobile#31171), replacing the path allowlist and the `target: '18'` rule. That is the denominator the skipped set is measured against. ## Coverage Anti-Patterns Each of these removes a whole function from the compiled set, and each looks locally harmless at the call site. - **A dependency the effect never reads.** The compiler's effect-dependency validation (`validateExhaustiveEffectDependencies`) reports it as an error and skips the entire function. A comment beside the array does not help, because the check reads the array rather than the prose. `perf-hooks-effects` gains the rule with the four ways to make the array describe the code: pass the value into the work the effect starts, put a `key` on a component where the value marks a scope that must reset as a whole, handle it where the action occurs, or treat it as a finding. It names what does not work, which is deleting the dependency outright, explaining it in a comment, reaching for `'use no memo'`, and adding a parameter the callee ignores. - **`'use no memo'` left in place after the reason for it is gone.** `mm-react-compiler.md` now tests whether the directive is doing any work before keeping it, since React documents it as a temporary debugging tool and it opts the whole function out. - **Removing a `useMemo` the compiler appears to make redundant.** Its memoization is not guaranteed to preserve the identity a `useEffect` dependency needs, so `mm-react-compiler.md` keeps any `useMemo` or `useCallback` whose output feeds an effect's dependency array. - **`perf-react-compiler`** states the unread-dependency rule as a compiler-coverage anti-pattern, because someone asking what costs them coverage opens that file rather than the effects one. ## Two effects rules from the same review These do not affect compiler coverage and are here because they came out of the same review rather than because they belong to the argument above. - **`perf-hooks-effects`** corrects its dependency-minimization guidance to depend on the primitive the effect reads rather than on an object the parent rebuilds each render. The previous worked pair moved `refreshInterval` to a default parameter and dropped it from the array, which does not change whether it is a dependency. - **`perf-hooks-effects`** moves a multi-step async acquire with a timeout and several end paths into a plain object owned outside React, so those end paths can be unit-tested without rendering. ## Where this lands The triage material goes to `skills/performance/`, which reaches mobile alone because that skill's `repos/` holds only `metamask-mobile.md` and `tools/install` skips a skill entirely for any repo with no overlay file. The coverage rules go to `skills/perf-*/repos/metamask-extension.md`, four skills whose `repos/` holds only that file. Of the 85 lines added on the first path, 6 are mobile-specific. Of the 140 on the second, 0 are extension-specific. So most of this is platform-independent React guidance landing where the existing rules already sit. Widening either path's reach needs `repos/` files in skills this pull request does not touch and is tracked separately. The anti-patterns came out of review on [metamask-extension#46240](MetaMask/metamask-extension#46240). --- Replaces #145, which was closed when its fork was deleted. GitHub cannot reopen a pull request whose head repository is gone, so this is a new pull request from a branch in this repository. It carries #145's four commits plus four more, which were briefly opened separately before being folded in here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The React Compiler fails open: a component it cannot compile ships unoptimized with a green build and no warning, and raising
panicThresholdto'all_errors'to find those components returns an error list that mixes unsupported-syntax noise with real bugs. Its memoization is also not guaranteed to preserve the identity auseEffectdependency needs, so removing auseMemothe compiler seems to make redundant can silently change when an effect fires.Overview
Extends the mobile
performanceskill with a React Compiler error-triage reference and an effect-dependency exception.references/mm-react-compiler-error-triage.mdtells the agent to sort compiler errors into unsupported (the compiler'sTodocategory) and actionable errors, and to ratchetpanicThresholdin non-production builds rather than set it once.references/mm-react-compiler.mdnow describes the compiler as running app-wide on its defaulttarget: '19', as set up in metamask-mobile#31171 (fully enable React Compiler), in place of the path allowlist and thetarget: '18'rule, and tells the agent to keep anyuseMemooruseCallbackwhose output is auseEffectdependency.