fix: linked strategy for scoped packages, aliases, and peer deps [v10 backport]#9011
Draft
manzoorwanijk wants to merge 2 commits intonpm:release/v10from
Draft
fix: linked strategy for scoped packages, aliases, and peer deps [v10 backport]#9011manzoorwanijk wants to merge 2 commits intonpm:release/v10from
manzoorwanijk wants to merge 2 commits intonpm:release/v10from
Conversation
npm#8996) We're looking at using `install-strategy=linked` in the [Gutenberg monorepo](https://github.com/WordPress/gutenberg) (~200 workspace packages), which powers the WordPress Block Editor. While [testing it in a PR](WordPress/gutenberg#75213), we ran into several issues with the linked strategy that this PR fixes. ## Summary 1. Scoped workspace packages were losing their `@scope/` prefix in `node_modules` because the symlink name came from the folder basename instead of the package name. 2. Aliased packages (e.g. `"prettier": "npm:custom-prettier@3.0.3"`) in workspace projects were getting symlinked under the real package name instead of the alias, so `require('prettier')` would fail. 3. With `legacy-peer-deps = true`, peer dependencies weren't being placed alongside packages in the store, so `require()` calls for peer deps failed from within the store. 4. With `strict-peer-deps = true`, the linked strategy could crash with `Cannot read properties of undefined` when store entries or parent nodes were missing for excluded peer deps. ## Root cause `assignCommonProperties` was using a single `result.name` for both consumer-facing symlinks and store-internal paths. For workspaces, `node.name` comes from the folder basename (missing the scope). For aliases, `node.packageName` gives the real name but we need the alias for the consumer symlink. Separately, `legacy-peer-deps` tells the arborist to skip creating peer dep edges entirely, so the isolated reifier never saw them and never placed them in the store. And `strict-peer-deps` can cause nodes to be excluded from the tree while still being referenced by edges, leading to undefined lookups. ## Changes - Split proxy identity into `result.name` (consumer-facing: alias or scoped workspace name) and `result.packageName` (store-internal: real package name from `package.json`). Store paths (`getKey`, `treeHash`, `generateChild`, `processEdges`, `processDeps`) use `packageName`; consumer symlinks keep using `name`. - When `legacyPeerDeps` is enabled, resolve missing peer dep edges from the tree via `node.resolve()` so they still get symlinked in the store. - Guard against undefined `from` and `target` nodes in `processEdges`/`processDeps` to prevent crashes with `strict-peer-deps`. - Guard `idealTree.children.get(ws)` in `reify.js` since the isolated tree uses an array for `children`, not a Map. - Test fixture updates: `recursive: true` for `mkdirSync`, scoped workspace glob support. - New tests for scoped workspace packages, aliased packages in workspaces, and peer deps with `legacyPeerDeps`. ## References Fixes npm#6122
manzoorwanijk
changed the title
manzoorwanijk
changed the title
manzoorwanijk
changed the title
Contributor
Author
|
I am holding this off until #9013 is merged and then we can backport that as well. |
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
1 participant
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.
Cherry-pick of the a2154cd (#8996)
Since Node 22 doesn't have npm 11 yet, it would be better to have this change backported to v10