feat: Adds typescript domain with authoring, migration skills - #167
Merged
Merged
Conversation
…-types, decompose-large-files
…ary-identification is step one of any migration
…d exception (bivariant callback `any`)
…und unit conversion, swap in the stronger derive-types example, slim CHANGELOG - migration-context-cost: line count is a factor not a non-factor; fan-in is a reading cost (not a change/review surface), fan-out is the change surface; drop the wrong "upstream types land first" and off-topic barrel bullet - decompose-large-files: the point is converting to TS in small self-contained units, not extraction - derive-types: replace the NetworkState restatement with the reinvented-messenger + hand-copied-return example (derive via `ReturnType<Action['handler']>`) - CHANGELOG: list the domain, not each skill
…action is optional
CHANGELOG.md tracks consumer-facing changes to the `@metamask/skills` package, per CONTRIBUTING's "CLI / tooling changes" section. No merged skill-only PR adds an entry (#80, #78, #70, #62, #61 all touch zero changelog lines). It was also the sole source of this branch's conflict with `main`, since every skill PR edits the same `[Unreleased]` block.
…main Was a separate PR against `domains/testing`. It belongs here: its subject is whether a hand-written type agrees with its authoritative source, which is the question `derive-types` answers from the authoring side, and it shares this domain's premise that a green `tsc` is not evidence the types are correct. Directory name and frontmatter `name` already agree; only the domain moved.
The skill lives in the `typescript` domain, so the prefix repeated information already carried by the path and by every discovery surface that shows it. Installed as `mms-compiler-blindspots`.
Skills install flat as `mms-<name>`, so `domains/typescript/` does not disambiguate the name at the callsite — and "compiler" reads as React Compiler in a repo where that is a live subject. The skill is about `tsc` specifically: its own first clause is "the type defects `tsc` is structurally unable to report". Also adds the slash trigger to the description, which listed only prose phrases.
Four additions, each of which found something the existing sections did not point at, while reviewing a JS->TS conversion. `any` absorption gains its complement: the existing bullet traces where a confidently-typed value entered, which finds nothing when the value is a module's own return. Tracing where an `any` *exits* — assigning the return to two impossible types with a known-typed sibling as control — found `any` escaping a resolver into its caller. The control line is load-bearing: without it a silent probe is indistinguishable from one that cannot fail, and running outside the project tsconfig produces errors that are the harness rather than the finding. Ethers `Contract` dynamic methods are named as a source, since the ABI is runtime data and the call reads as an ordinary typed await. It is already tracked at #31973, where one consumer declares `Promise<any>` with a disable comment and another lets it infer; the second is the dangerous form. Ambient `declare module` verification moves from reading package source to requiring the package and checking exports, return `typeof`, and whether a default export is legitimate under esModuleInterop. Inventory now begins by checking the module is referenced at all — a dead module's types are unfalsifiable, and its conversion is a deletion candidate rather than a typing exercise.
…finds it A precise annotation fed `any` at every call site is reportable by neither `tsc` nor `no-explicit-any`, and an ambient `declare module` in the path re-mints the `any` as a confident `string`. Both arms of the probe verified against `metamask-extension`. Also fixes `avoid-any`'s frontmatter, which did not parse as YAML.
The installer emits `mms-tsc-blindspots`; the description advertised `/tsc-blindspots`.
`tsc-blindspots`, `derive-types` and `avoid-any` shipped without a `repos/` directory, so they installed into every repo by default rather than by decision, carrying content written against no repo in particular. A blind spot only exists where the setting that would catch it is off, so the overlays genuinely differ. Extension and mobile set `allowJs`, making the JS boundary a live unchecked surface; core does not, and its overlay says so rather than omitting the class. Core instead has `composite: true`, where a type can go stale across a project reference. Extension's `strict` is inherited from `@tsconfig/node22` rather than local, which is itself a blind spot for anyone auditing tsconfig.json. `@metamask/eslint-config-typescript` sets `no-explicit-any` and all five `no-unsafe-*` rules to 'off' in every resolved major (13.0.0 mobile, 14.1.1 extension, 15.0.0 core), so each repo's local 'error' is a deliberate re-enable rather than a default. Extension's lint block carrying `no-explicit-any` takes its file list from the parsed tsconfig program, so the ~300 `*.stories.ts(x)` excluded from `tsconfig.json` lose the rule and `tsc` together. Every line citation verified against `origin/main` rather than a working tree; none resolves to a blank line. Installs verified into all three repos with the correct overlay merged.
Every count and line number in the three overlays was read from a local checkout that was behind `origin/main`, so the figures were stale: core reported 75 packages and 1,380 `.ts` files against an actual 98 and 2,052. Adds the two findings the re-read surfaced: `tsconfig.lint.json` declares its own 14 `references` rather than inheriting the root config's 98, so `lint:tsc` covers 14 of 98 packages; and extension's `no-explicit-any` block scopes to the parsed tsconfig program, so the 300 `.stories.ts(x)` excluded from `tsconfig.json` lose the lint rule and `tsc` together. Drops the note in `references/metamask-extension.md` explaining that the notes live outside `repos/` to avoid skipping mobile and core installs — the skill now ships all three overlays, so the reason is spent.
Both platform teams co-own the domain, as they do `coding`, `general`, `performance` and `pr-workflow`. A client's overlay is owned by that client's platform team, placed directly under the domain line so it wins on last-match and so each domain PR inserts at its own anchor.
…o absolutes `RestrictedMessenger` no longer exists in core or the extension, and the example now follows core's `BaseController` declaration. A hand-written type can miss in either direction, which the PR's own divergence shapes already show, and line count is a minor factor rather than none. CI's lint job runs `lint:tsc` over the full project at its 6144 MB heap and exits 0, so the claim that it always runs out of memory is gone.
…rived types An unneeded `@ts-expect-error` fails `tsc` while an unneeded `@ts-ignore` stays silent. `Omit` over a union keeps only the shared keys unless it distributes, `string & Record<never, never>` still accepts every string, and a circular import is answered with `import type`, not a placeholder type. A suppression is read for what it hides, and a decomposition greps the repo for consumers the replacement leaves dead.
MajorLift
marked this pull request as ready for review
September 18, 2026 14:25
rvelaz
approved these changes
Sep 18, 2026
MajorLift
enabled auto-merge (squash)
September 18, 2026 18:00
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.
Overview
Adds a
typescriptdomain — six skills for TypeScript authoring and JS→TS migration, the layercoding-guidelinesonly gestures at. Grounded inMetaMask/contributor-docsdocs/typescript.md.avoid-any—anydisables type checking rather than widening a type. Substitute by position: assignee →unknown, assigned →never.avoid-widening— an annotation wider than what inference already knows throws information away, so annotate only to add information. Keep immutable lookup tables literal withas const satisfies, and widen deliberately only at a boundary where inference is unsound.derive-types— derive from authoritative sources (indexed access,typeof) instead of ad-hoc types that duplicate and drift (#42583).decompose-large-files— split by coherent, independently-mergeable units, unblocking incremental migration. Modeled on theMetamaskControllerdecomposition (#41735).migration-context-cost— the fan-in/fan-out cost of a conversion: cheap first migration, or one pulling half the repo.tsc-blindspots— the defectstscis structurally unable to report. A green build is not evidence the types are correct, which isderive-types' question one step on: having derived from an authoritative source, what does the compiler still not check.Motivation
metamask-extension,metamask-mobileandcoreall author new TypeScript and re-enable@typescript-eslint/no-explicit-anytoerrorover a shared config that turns it off — extension.eslintrc.js:162, mobile.eslintrc.js:170, coreeslint.config.mjs:152.With
checkJsoff, unconverted neighbors are unchecked, so a green build does not establish well-formedness across the boundary.Two are proposed always-loaded, spending shared description budget:
tsc-blindspots, because a green build reads as proof, so nobody searches for whattsccannot report;derive-types, because an author hand-writing a type does not know an authoritative source exists.Showcase
Trial runs against merged PRs nobody flagged. Gap: the run surfaced something the PR had not.
tsc-blindspotsLog.topics; naive probe reports false cleanThe evidence showcase carries the
derive-typesexample: #44397 (derived-type substitution) — 12 hand-written types, 9 with an authoritative source, 5 of those disagreeing, 1 dropped guard traced to inert, 2 unnecessary runtime changes; one arm silent at exit 0.