Skip to content

fix: resolve workspace MetaMask packages from the monorepo root - #167

Closed
ulissesferreira wants to merge 7 commits into
mainfrom
ulissesferreira/fix-typescript-package-resolution-5832
Closed

fix: resolve workspace MetaMask packages from the monorepo root#167
ulissesferreira wants to merge 7 commits into
mainfrom
ulissesferreira/fix-typescript-package-resolution-5832

Conversation

@ulissesferreira

@ulissesferreira ulissesferreira commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Explanation

Status: working ✅ — verified via tsc --noEmit, tsc --traceResolution, full builds, and a real snaps-jest run.

  • Changes the shared paths entry in tsconfig.packages.json to resolve @metamask/* relative to the monorepo root (./packages/*/src) instead of relying on an unset/inherited baseUrl, which is what made the previous ../*/src mapping fragile (only correct if the consumer happened to be a sibling directory).
  • A local src candidate is used only when it exists on disk; otherwise resolution falls through to the installed node_modules package — so importing a published @metamask/* package not in this repo is unaffected.
  • Adds @metamask/snap-networks-utils/* as an explicit subpath mapping (TypeScript paths patterns only allow one *), covering extra entry points like /logger.
  • Adds tsconfig.snaps.json as a shared config for snap packages, with composite: false and no baseUrl, so they can typecheck sibling workspace source without project-reference build artifacts.
  • Snap configs deliberately don't set baseUrl unless they also re-declare the paths mapping relative to it (solana-wallet-snap does this to keep baseUrl-relative src/... imports).
  • Updates jest.config.packages.js's moduleNameMapper to the same local-source-then-installed-package rule.
  • Logger now assigns optional prefix/decorators only when provided, so solana-wallet-snap (exactOptionalPropertyTypes) can typecheck against workspace source.
  • Along the way, consolidates some per-package compiler options into the shared configs — flagged separately as it dropped a couple of options (module/moduleResolution for bitcoin-wallet-snap/sample-snap) that turned out to be silent behavior changes rather than pure dedup.

Pros and Cons

Pros:

  • Single source of truth: the @metamask/* mapping lives once in the shared tsconfig.packages.json, so adding a new workspace package requires no per-package config change.
  • Simplest mental model — resolution behavior is centralized and consistent for every package by default.
  • Least ongoing maintenance burden as the monorepo grows (more snaps/libraries later).

Cons:

  • Packages lose baseUrl-relative import ergonomics (e.g. from 'src/entities') unless they opt back in and re-declare paths anyway — which is exactly what fix: restate workspace @metamask/* paths per package with local baseUrl #169 does, partially undermining the "shared config" simplicity.
  • Consolidating configs surfaced silent behavior changes (dropped module/moduleResolution overrides for bitcoin-wallet-snap/sample-snap) — a reminder that shared-config consolidation has hidden blast radius.
  • Adds a new shared tsconfig.snaps.json layer, which is one more file in the inheritance chain to reason about.

Clean-install verification

Re-verified from a genuinely fresh worktree — no node_modules, no packages/*/dist anywhere — with only yarn install run (no yarn build):

  • yarn lint: passes (exit 0). lint:eslint itself runs build:only-clean first, so it never depended on a prebuilt dist/.
  • Raw tsc --noEmit per package (bypassing the root typecheck script's own pre-build step, to test resolution with zero builds anywhere): bitcoin-wallet-snap, sample-snap, snap-networks-utils, solana-wallet-snap, tron-wallet-snap all pass with 0 errors, correctly resolving @metamask/* (including the snap-networks-utils/* subpath) straight from src/ via the node_modules symlink.
  • stellar-wallet-snap couldn't run via yarn workspaces foreach ... exec tsc (command not found: tsc) — pre-existing gap: that package's package.json has no typescript devDependency, unrelated to this PR. Running tsc directly against it passes with 0 errors.

Confirms the fix holds in the worst case (fresh clone, install-only, zero builds), not just when a stale dist/ happens to already exist.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

cursoragent and others added 7 commits August 18, 2026 23:24
Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
…ypescript-package-resolution-5832

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
Map @metamask/* imports to packages/*/src (including subpaths) from the
repository root, disable composite for Snap configs so they can consume
sibling source, and fall back to node_modules when no local package exists.

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
TypeScript path patterns allow only one star, so keep the root wildcard
for package-root imports and add an explicit mapping for
@metamask/snap-networks-utils subpaths. Assign optional Logger fields
without writing undefined so Snap consumers with exactOptionalPropertyTypes
can typecheck against local source.

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
Remove the TypeScript resolution plan and design notes from the
branch, document the consumer-facing package changes, and update
snap manifest shasums to match the CI build.

Co-authored-by: Ulisses Ferreira <ulisses@hey.com>
@cursor cursor Bot changed the title fix: resolve only selected MetaMask packages locally fix: resolve workspace MetaMask packages from the monorepo root Aug 18, 2026
ulissesferreira added a commit that referenced this pull request Aug 19, 2026
…demonstrative)

Structurally replicates core's incremental repository-wide type-checking
setup: tsconfig.packages.lint.json (shared), a tsconfig.lint.json per
package (outDir/tsBuildInfoFile -> .tsc-lint-cache, references to
workspace deps), a root tsconfig.lint.json referencing all six packages,
and a `lint:tsc` script wired into `yarn lint`, exactly matching
MetaMask/core/tsconfig.lint.json and MetaMask/core/packages/base-controller/tsconfig.lint.json.

This intentionally does NOT touch the existing @metamask/* paths/baseUrl
mapping (per #170's original scope). Running `yarn lint:tsc` from a
clean state (no packages/*/dist, no .tsbuildinfo/.tsc-lint-cache)
currently fails:

- ~70 "Cannot find module '@metamask/snap-networks-utils/...'" errors
  in bitcoin-wallet-snap, solana-wallet-snap, tron-wallet-snap. This is
  the expected, confirmed result: the reference graph rebuilds each
  project's OWN .tsc-lint-cache output, but the importing package still
  resolves the bare specifier via node_modules -> package.json -> the
  real dist/, which this graph never touches. `references` + `tsc
  --build` do not remove the need for a paths -> src mapping (#167/#169)
  -- core's own tsconfig.lint.json files extend each package's regular
  tsconfig.json specifically to inherit that mapping.
- 13 TS6307 "Projects must list all files or use an 'include' pattern"
  errors, specific to composite/--build mode's stricter accounting of
  every file entering the program (nested JSON test fixtures,
  snap.manifest.json) vs plain `tsc --noEmit`. A real migration would
  need broader `include` patterns per package.
- 18 pre-existing, unrelated "Cannot find module '@metamask/superstruct'"
  errors via a ../snaps-registry path, confirmed byte-for-byte identical
  on unmodified main with or without any of this branch's changes.

Committed as-is, findings documented, to answer "what would full Core
parity require" -- not intended to be merged in this state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants