Open
Conversation
Collaborator
|
Review requested:
|
|
I like the idea, it would greatly reduce the amount of filesystem operations that pnpm has to do in order to create an isolated node_modules layout using symlinks. I also suggested arcanis to possibly go one layer deeper and allow to map the individual files of packages. This would allow to map node_modules directly from a content-addressable store (that consists of package files). Of course, that would increase the size of the file several times but it would also make installation even faster. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62239 +/- ##
==========================================
+ Coverage 89.66% 89.69% +0.02%
==========================================
Files 676 677 +1
Lines 206462 206873 +411
Branches 39533 39602 +69
==========================================
+ Hits 185128 185550 +422
+ Misses 13461 13456 -5
+ Partials 7873 7867 -6
🚀 New features to boost your workflow:
|
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.
This PR adds a new
--experimental-package-map=<path>flag letting Node.js resolve packages using a static JSON file instead of walkingnode_modulesdirectories.Why?
The
node_modulesresolution algorithm predates npm and its clear definition of the concept of packages. It works well enough and is widely supported, but has known issues:Phantom dependencies - packages can accidentally import things they don't declare, because hoisting makes transitive dependencies visible
Peer dependency resolution is broken in monorepos - if
website-v1usesreact@18andwebsite-v2usesreact@19, and both use a sharedcomponent-libwith React as a peer dep, there's nonode_moduleslayout that resolves correctly. The shared lib always gets whichever React was hoisted.Hoisting is lossy - runtimes can't tell if an import is legitimate or accidental
Resolution requires I/O - you have to hit the filesystem to resolve packages
Package managers have tried workarounds (pnpm symlinks, Yarn PnP), but are either limited by what the filesystem itself can offer (like symlinks) or by their complexity and lack of standardization (like Yarn PnP). This PR offers a mechanism for such tools to solve the problems listed above in tandem with Node.js.
How it works
A
package-map.jsondeclares packages, their locations (relative to the package map), and what each can import:{ "packages": { "my-app": { "name": "my-app", "path": "./src", "dependencies": ["lodash", "react"] }, "lodash": { "name": "lodash", "path": "./node_modules/lodash" }, "react": { "name": "react", "path": "./node_modules/react" } } }When resolving a bare specifier:
dependenciespathERR_PACKAGE_MAP_ACCESS_DENIEDMODULE_NOT_FOUNDCompatibility
An important aspect of the package maps feature that separates it from competing options like Yarn PnP is its builtin compatibility with
node_modulesinstalls. Package managers can generate bothnode_modulesfolders ANDpackage-map.jsonfiles, with the later referencing paths from the former.Tools that know how to leverage
package-map.jsoncan then use this pattern for both static package resolution and strict dependency checks (with optional fallbacks to hoisting if they just wish to use the package map information to emit warnings rather than strict errors), whereas tools that don't will fallback to the classicalnode_modulesresolution.Differences with import maps
Issue #49443 requested to implement import maps. In practice these aren't a good fit for runtimes like Node.js for reasons described here and which can be summarized as: import maps take full ownership of the resolution pipeline by spec, thus preventing implementing additional runtime-specific behaviours such as
exportsorimportsfields.This PR comes as close from implementing import maps as possible but with a very light difference in design making it possible to stay compatible with other Node.js resolution features.
Why not a loader?
The ecosystem now has to deal with a variety of third-party resolvers, most of them not implementing the loader API for many different reasons: too complex, turing-complete, or dependent on a JS runtime.
After I've been following this path for more than six years I can confidently say that loaders would work for Node.js itself but wouldn't be standard enough to be included in at least some of those popular third-party tools.
Questions
--experimental-strict-package-mapsis set? Or via astrictfield inpackage-map.json.