fix: restate workspace @metamask/* paths per package with local baseUrl - #169
Closed
ulissesferreira wants to merge 10 commits into
Closed
fix: restate workspace @metamask/* paths per package with local baseUrl#169ulissesferreira wants to merge 10 commits into
ulissesferreira wants to merge 10 commits into
Conversation
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>
Alternative to #167's root-anchored, baseUrl-free tsconfig.packages.json mapping. Each package sets its own `baseUrl: "./"` (preserving `src/...`-relative import ergonomics) and restates the `@metamask/*` workspace mapping relative to it, since TypeScript resolves inherited `paths` against whichever `baseUrl` is present in the final merged config, not the file that declared `paths`.
4 tasks
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.
4 tasks
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
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.
Explanation
Status: working ✅ — same verification as #167 (builds,
tsc --noEmit,traceResolution, fullbitcoin-wallet-snapbuild +snaps-jestrun), confirming the per-package restated mapping resolves correctly.Alternative to #167, built on top of its branch, for side-by-side comparison.
@metamask/*imports resolve to siblingsrc, falling back tonode_moduleswhen no localsrcexists) but changes the ergonomics: instead of anchoringpathsonce at the shared root config, each package sets its own"baseUrl": "./".bitcoin-wallet-snap,tron-wallet-snap,stellar-wallet-snap,sample-snap,snap-networks-utils) restates the@metamask/*(and, where needed,@metamask/snap-networks-utils/*subpath) mapping relative to its ownbaseUrl, since TypeScript resolves inheritedpathsagainst whicheverbaseUrlis present in the final merged config — not the file that declaredpaths.baseUrl-relative import ergonomics (e.g.from 'src/entities'), already used insolana-wallet-snap's tests, for every package rather than just one.tsc --traceResolutionthat settingbaseUrlper package without restating the mapping silently breaks workspace-source resolution (falls through to publisheddist/innode_modules) — restating it is what fixes that.tsconfig.json, rather than defined once in a shared root config, in exchange for per-packagebaseUrlergonomics.Pros and Cons
Pros:
baseUrl-relative import ergonomics, which the team explicitly prefers.tsconfig.json, no cross-file inheritance reasoning needed.Cons:
@metamask/*(and subpath) mapping must be repeated in every package, and in every future package with workspace imports — higher risk of drift/typos, and a maintenance tax that scales with the number of packages.baseUrlwithout restating the mapping silently reverts tonode_modules/distresolution — confirmed empirically, so this is a real footgun for anyone adding a package who copiesbaseUrlbut forgets the paths block.Clean-install verification
Re-verified from a genuinely fresh worktree — no
node_modules, nopackages/*/distanywhere — with onlyyarn installrun (noyarn build):yarn lint: passes (exit 0) — ESLint, Prettier, constraints, dependencies, README check all clean.tsc --noEmitper package (bypassing the roottypecheckscript's own pre-build step, to test resolution with zero builds anywhere): all 6 workspaces pass with 0 errors, correctly resolving@metamask/*(including thesnap-networks-utils/*subpath) straight fromsrc/per package's own restatedbaseUrl-relative mapping.stellar-wallet-snapcouldn't run viayarn workspaces foreach ... exec tsc(command not found: tsc) — pre-existing gap: that package'spackage.jsonhas notypescriptdevDependency, unrelated to this PR. Runningtscdirectly 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
baseUrl) and fix: add TypeScript project references, mirroring MetaMask/core #170 (references-only, does not fix resolution)Checklist