perf: Augment mms-performance with frontend learnings from the Extension performance audit - #49
Closed
MajorLift wants to merge 10 commits into
Closed
perf: Augment mms-performance with frontend learnings from the Extension performance audit#49MajorLift wants to merge 10 commits into
mms-performance with frontend learnings from the Extension performance audit#49MajorLift wants to merge 10 commits into
Conversation
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
MajorLift
changed the title
mms-performance with frontend learnings from the Extension performance audit
This was referenced Jun 10, 2026
3 tasks
MajorLift
marked this pull request as ready for review
June 10, 2026 20:51
MajorLift
changed the title
mms-performance with frontend learnings from the Extension performance auditmms-performance with frontend learnings from the Extension performance audit
…ormalization references to the `performance` skill Cross-platform React/Redux guidance adapted from contributor-docs `frontend-performance.md` and the extension performance audit (MetaMask-planning#6571; extension PRs #38007, #37147), wired into the mobile overlay's routing tables. Additive only — no changes to existing mobile-specific facts or guidance.
…g in `mm-selector-cascade`
…emoization exception The effect-dependency exception follows official compiler guidance: removing manual memoization whose output feeds a `useEffect` dependency can over/under- fire effects (reactwg/react-compiler#16).
- WDYR (`wdyr.js`, `useSelector` diff tracking) as the live cascade tracer - input-unstable vs output-unstable tool selection (`resultEqualityCheck`) - a live cascade nullifies downstream memo/virtualization/compiler wins - plain-function selector sweep; span quota guardrail; redux-persist caveat
…ptance criteria, cross-platform check
`useSelector` count alone is weak evidence on this codebase: controller state changes batch into a 250ms flush (`app/core/Batcher`) dispatched inside `unstable_batchedUpdates` (`EngineService`), so checks run a few times per second and React renders once per flush. Flag expensive or unstable selectors among the N instead; consolidation is the exception (per-row components, unmemoized derivations), not the default.
…essor note, engine slice contract From a per-selector triage of the 10 highest-subscription components: ~90% of reads ruled out (flag booleans, primitive accessors, useMemo'd factories). Adds: check for `weakMapMemoize` (and fresh-object args that defeat it) before flagging parameterized selectors; inline accessors returning primitives are cleanup not perf findings; verified engine slice mechanics (per-controller key replace + Immer structural sharing); deep-equal selectors are output-stable but pay O(input) per check.
… example in `mm-react-compiler-error-triage`
…he cascade playbooks Result-function greps miss an input function that builds a fresh composite per call, which silently downgrades `createDeepEqualSelector` into a whole-composite deep compare on every check. Found post-hoc via manual tree mapping (`getStateForAssetSelector` → asset-surface root, metamask-mobile#31561); this encodes the search so the skill finds the next one itself.
MajorLift
force-pushed
the
jongsun/docs/260610-performance-skill-frontend-gaps
branch
from
July 22, 2026 16:28
3309fb9 to
65e7388
Compare
The three files this PR adds each restated a pattern that also exists generically. Rather than duplicate, each now opens with a scope note naming the knowledge file that owns the definition — `effect-anti-patterns` for the lifecycle patterns, `render-cascade` and `selector-anti-patterns` for the cascade and shape ones — and keeps what only it can say: the verified Mobile instances, this store's real dependency graph, and the fix recipes. Citations are by name rather than relative link: `install` copies domain knowledge/ and a skill's references/ as siblings under the installed skill directory, so a repo-correct relative path breaks once installed. The knowledge files themselves land in the performance-skills PR (#43); until that merges these notes name a file that is not yet present, which is why they are names and not links.
MajorLift
added a commit
to MajorLift/metamask-skills
that referenced
this pull request
Jul 30, 2026
Folds in the react-render-proof skill (was MetaMask#82) and the mobile reference-library additions (was MetaMask#49). All three were the same effort seen from different ends — moving performance work earlier in the loop — and they share a substrate, so reviewing them apart meant reviewing the substrate three times. The loop this domain now covers: - catch it at review — effect/selector anti-pattern review skills, driven by the knowledge taxonomy - prove it moved — react-render-proof, with a delivery gate so an arm whose treatment never reached the bundle cannot report as a null - measure it honestly — data-analysis, benchmark hygiene, web-vitals framing - know the codebase — the mm-* reference library and its audit playbook Also neutralizes five references to private planning tickets, which do not belong on a public repository — they named internal epic and audit-ticket numbers. The surrounding guidance is unchanged; only the identifiers are gone.
MajorLift
deleted the
jongsun/docs/260610-performance-skill-frontend-gaps
branch
July 30, 2026 12:50
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
2 participants
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.
Key additions
mm-react-compiler-error-triage) — the'Todo'split (unsupported vs actionable) plus thepanicThresholdratchet. Turns ~7,000 apparent errors into a 31-file backlog and stops coverage from regressing silently.mm-selector-cascade) — traverse the selector graph to closure, fix the root, then delete the downstreamisEqual/deep-equal band-aids instead of accumulating more (the metamask-extension#37147 playbook).mm-state-normalization) — parameterized selectors with single-entry caches recompute per row, forever; fresh object-literal arguments defeat evenweakMapMemoize.mm-useeffect-antipatterns) — unmount-safe async, effect chains, cleanup: the missing other half ofmm-hook-dependency-arrays.All additive: wired into the routing tables and audit/planning playbooks, no existing mobile guidance modified.
Field-tested before review
Review notes
Read order: error-triage → cascade → normalization → useeffect → overlay/playbook diffs. Two sanity-checks where mobile knowledge beats mine: the per-slice reference-stability framing in the cascade file vs the store's actual behavior, and the compiler error categories vs what the Babel logger emits in practice (names verified against the
babel-plugin-react-compiler@1.0.0dist this repo ships). Deliberately untouched:skill.md's description (54 chars over the incominglint-skill-entryceiling — eval-optimized, owner's call) and the legacy extensionperf-*skills (planned follow-up).Sources: contributor-docs
frontend-performance.md· MetaMask-planning#6571 · metamask-extension#38007, #37147 · reactwg/react-compiler#16. Validation tickets: metamask-mobile#31490–#31501, #31507, #31509.🤖 Generated with Claude Code
Update — deduplicated against the
knowledge/taxonomyThe three references this PR adds (
mm-useeffect-antipatterns,mm-selector-cascade,mm-state-normalization) each restated a pattern that also exists generically. Each now opens with a scope note naming the knowledge file that owns the definition —effect-anti-patternsfor the lifecycle patterns,render-cascadeandselector-anti-patternsfor the cascade and shape ones — and keeps what only it can say: the verified Mobile instances, this store's real dependency graph, and the fix recipes. No mobile-specific content was removed.Ordering: the knowledge files themselves land in #43. Citations here are by name, not relative link, so nothing 404s if this merges first — it just names a file not yet present. (Names rather than links is also forced by layout:
tools/installcopies domainknowledge/and a skill'sreferences/as siblings under the installed skill dir, while the repo has them three levels apart, so no relative path is correct in both.)