fix: linked strategy fixes for scoped packages, aliases, and peer deps (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

Author: manzoorwanijk

workspaces/arborist/lib/arborist/isolated-reifier.js

@@ -106,7 +106,7 @@ module.exports = cls => class IsolatedReifier extends cls {
         node.root.path,
         'node_modules',
         '.store',
-        `${node.name}@${node.version}`
+        `${node.packageName}@${node.version}`
       )
       mkdirSync(dir, { recursive: true })
       // TODO this approach feels wrong
@@ -146,6 +146,21 @@ module.exports = cls => class IsolatedReifier extends cls {
     const optionalDeps = edges.filter(e => e.optional).map(e => e.to.target)
     const nonOptionalDeps = edges.filter(e => !e.optional).map(e => e.to.target)
 
+    // When legacyPeerDeps is enabled, peer dep edges are not created on the
+    // node. Resolve them from the tree so they get symlinked in the store.
+    const peerDeps = node.package.peerDependencies
+    if (peerDeps && node.legacyPeerDeps) {
+      const edgeNames = new Set(edges.map(e => e.name))
+      for (const peerName of Object.keys(peerDeps)) {
+        if (!edgeNames.has(peerName)) {
+          const resolved = node.resolve(peerName)
+          if (resolved && resolved !== node && !resolved.inert) {
+            nonOptionalDeps.push(resolved)
+          }
+        }
+      }
+    }
+
     result.localDependencies = await Promise.all(nonOptionalDeps.filter(n => n.isWorkspace).map(this.workspaceProxyMemo))
     result.externalDependencies = await Promise.all(nonOptionalDeps.filter(n => !n.isWorkspace).map(this.externalProxyMemo))
     result.externalOptionalDependencies = await Promise.all(optionalDeps.map(this.externalProxyMemo))
@@ -156,7 +171,9 @@ module.exports = cls => class IsolatedReifier extends cls {
     ]
     result.root = this.rootNode
     result.id = this.counter++
-    result.name = node.name
+    /* istanbul ignore next - packageName is always set for real packages */
+    result.name = result.isWorkspace ? (node.packageName || node.name) : node.name
+    result.packageName = node.packageName || node.name
     result.package = { ...node.package }
     result.package.bundleDependencies = undefined
     result.hasInstallScript = node.hasInstallScript
@@ -229,7 +246,7 @@ module.exports = cls => class IsolatedReifier extends cls {
         getChildren: node => node.dependencies,
         filter: node => node,
         visit: node => {
-          branch.push(`${node.name}@${node.version}`)
+          branch.push(`${node.packageName}@${node.version}`)
           deps.push(`${branch.join('->')}::${node.resolved}`)
         },
         leave: () => {
@@ -247,7 +264,7 @@ module.exports = cls => class IsolatedReifier extends cls {
     }
 
     const getKey = (idealTreeNode) => {
-      return `${idealTreeNode.name}@${idealTreeNode.version}-${treeHash(idealTreeNode)}`
+      return `${idealTreeNode.packageName}@${idealTreeNode.version}-${treeHash(idealTreeNode)}`
     }
 
     const root = {
@@ -302,7 +319,7 @@ module.exports = cls => class IsolatedReifier extends cls {
         isProjectRoot: false,
         isTop: false,
         location,
-        name: node.name,
+        name: node.packageName || node.name,
         optional: node.optional,
         top: { path: proxiedIdealTree.root.localPath },
         children: [],
@@ -336,7 +353,7 @@ module.exports = cls => class IsolatedReifier extends cls {
         return
       }
       processed.add(key)
-      const location = join('node_modules', '.store', key, 'node_modules', c.name)
+      const location = join('node_modules', '.store', key, 'node_modules', c.packageName)
       generateChild(c, location, c.package, true)
     })
     bundledTree.nodes.forEach(node => {
@@ -362,13 +379,17 @@ module.exports = cls => class IsolatedReifier extends cls {
 
       let from, nmFolder
       if (externalEdge) {
-        const fromLocation = join('node_modules', '.store', key, 'node_modules', node.name)
+        const fromLocation = join('node_modules', '.store', key, 'node_modules', node.packageName)
         from = root.children.find(c => c.location === fromLocation)
         nmFolder = join('node_modules', '.store', key, 'node_modules')
       } else {
         from = node.isProjectRoot ? root : root.fsChildren.find(c => c.location === node.localLocation)
         nmFolder = join(node.localLocation, 'node_modules')
       }
+      /* istanbul ignore next - strict-peer-deps can exclude nodes from the tree */
+      if (!from) {
+        return
+      }
 
       const processDeps = (dep, optional, external) => {
         optional = !!optional
@@ -380,12 +401,16 @@ module.exports = cls => class IsolatedReifier extends cls {
 
         let target
         if (external) {
-          const toLocation = join('node_modules', '.store', toKey, 'node_modules', dep.name)
+          const toLocation = join('node_modules', '.store', toKey, 'node_modules', dep.packageName)
           target = root.children.find(c => c.location === toLocation)
         } else {
           target = root.fsChildren.find(c => c.location === dep.localLocation)
         }
         // TODO: we should no-op is an edge has already been created with the same fromKey and toKey
+        /* istanbul ignore next - strict-peer-deps can exclude nodes from the tree */
+        if (!target) {
+          return
+        }
 
         binNames.forEach(bn => {
           target.binPaths.push(join(from.realpath, 'node_modules', '.bin', bn))

workspaces/arborist/lib/arborist/reify.js

@@ -438,7 +438,7 @@ module.exports = cls => class Reifier extends cls {
       if (includeWorkspaces) {
         // add all ws nodes to filterNodes
         for (const ws of this.options.workspaces) {
-          const ideal = this.idealTree.children.get(ws)
+          const ideal = this.idealTree.children.get && this.idealTree.children.get(ws)
           if (ideal) {
             filterNodes.push(ideal)
           }

workspaces/arborist/test/fixtures/isolated-nock.js

@@ -164,9 +164,13 @@ async function getRepo (graph) {
   // Generate the root of the graph on disk
   const root = graph.root
   const workspaces = graph.workspaces || []
+  const hasScoped = workspaces.some(w => w.name.startsWith('@'))
+  const workspaceGlobs = hasScoped
+    ? ['packages/*', 'packages/@*/*']
+    : ['packages/*']
   const repo = {
     'package.json': JSON.stringify({
-      workspaces: workspaces.length !== 0 ? ['packages/*'] : undefined,
+      workspaces: workspaces.length !== 0 ? workspaceGlobs : undefined,
       ...root,
     }),
     packages: {},
@@ -192,7 +196,7 @@ function createDir (dir, structure) {
   Object.entries(structure).forEach(([key, value]) => {
     if (typeof value === 'object') {
       const newDir = path.join(dir, key)
-      fs.mkdirSync(newDir)
+      fs.mkdirSync(newDir, { recursive: true })
       createDir(newDir, value)
     } else {
       fs.writeFileSync(path.join(dir, key), value)

workspaces/arborist/test/isolated-mode.js

@@ -309,6 +309,65 @@ tap.test('simple peer dependencies scenarios', async t => {
   rule7.apply(t, dir, resolved, asserted)
 })
 
+tap.test('peer dependencies with legacyPeerDeps', async t => {
+  /*
+   * With legacyPeerDeps, peer dep edges are not created on the node.
+   * The linked strategy should still place peer deps alongside the
+   * package in the store so require() works from the real path.
+   *
+   * root -> phpegjs
+   *         phpegjs -> pegjs (peer dep, no edge with legacyPeerDeps)
+   * root -> pegjs
+   */
+
+  const graph = {
+    registry: [
+      { name: 'phpegjs', version: '1.0.0', peerDependencies: { pegjs: '*', missing: '*' } },
+      { name: 'pegjs', version: '2.0.0' },
+      {
+        name: 'adapter',
+        version: '1.0.0',
+        dependencies: { pegjs: '*' },
+        peerDependencies: { pegjs: '*' },
+      },
+    ],
+    root: {
+      name: 'foo',
+      version: '1.2.3',
+      dependencies: { phpegjs: '1.0.0', pegjs: '2.0.0', adapter: '1.0.0' },
+    },
+  }
+
+  const resolved = {
+    'foo@1.2.3 (root)': {
+      'phpegjs@1.0.0': {
+        'pegjs@2.0.0 (peer)': {},
+      },
+      'pegjs@2.0.0': {},
+      'adapter@1.0.0': {
+        'pegjs@2.0.0': {},
+      },
+    },
+  }
+
+  const { dir, registry } = await getRepo(graph)
+
+  const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
+  const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache, legacyPeerDeps: true })
+  await arborist.reify({ installStrategy: 'linked' })
+
+  // phpegjs should be able to require its peer dep pegjs
+  t.ok(setupRequire(dir)('phpegjs', 'pegjs'),
+    'phpegjs can require peer dep pegjs with legacyPeerDeps')
+
+  const asserted = new Set()
+  rule1.apply(t, dir, resolved, asserted)
+  rule2.apply(t, dir, resolved, asserted)
+  rule3.apply(t, dir, resolved, asserted)
+  rule5.apply(t, dir, resolved, asserted)
+  rule7.apply(t, dir, resolved, asserted)
+})
+
 tap.test('Lock file is same in hoisted and in isolated mode', async t => {
   const graph = {
     registry: [
@@ -1300,6 +1359,113 @@ tap.test('scoped package', async t => {
   rule7.apply(t, dir, resolved, asserted)
 })
 
+tap.test('scoped workspace packages', async t => {
+  /*
+   * Dependency graph:
+   *
+   * root -> @scope/package-b (workspace)
+   *         @scope/package-b -> @scope/package-a (workspace)
+   * root -> @scope/package-a (workspace)
+   */
+
+  const graph = {
+    registry: [
+      { name: 'which', version: '1.0.0' },
+    ],
+    root: {
+      name: 'myproject', version: '1.0.0', dependencies: { '@scope/package-a': '*', '@scope/package-b': '*' },
+    },
+    workspaces: [
+      { name: '@scope/package-a', version: '1.0.0', dependencies: { which: '1.0.0' } },
+      { name: '@scope/package-b', version: '1.0.0', dependencies: { '@scope/package-a': '*' } },
+    ],
+  }
+
+  const resolved = {
+    'myproject@1.0.0 (root)': {
+      '@scope/package-a@1.0.0 (workspace)': {
+        'which@1.0.0': {},
+      },
+      '@scope/package-b@1.0.0 (workspace)': {
+        '@scope/package-a@1.0.0 (workspace)': {
+          'which@1.0.0': {},
+        },
+      },
+    },
+  }
+
+  const { dir, registry } = await getRepo(graph)
+
+  const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
+  const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
+  await arborist.reify({ installStrategy: 'linked' })
+
+  const asserted = new Set()
+  rule1.apply(t, dir, resolved, asserted)
+  rule2.apply(t, dir, resolved, asserted)
+  rule3.apply(t, dir, resolved, asserted)
+  rule4.apply(t, dir, resolved, asserted)
+  rule7.apply(t, dir, resolved, asserted)
+})
+
+tap.test('aliased packages in workspace', async t => {
+  /*
+   * Dependency graph:
+   *
+   * root -> prettier (alias for npm:custom-prettier@3.0.3)
+   *         custom-prettier -> isexe
+   * root -> my-pkg (workspace)
+   *         my-pkg -> prettier (alias for npm:custom-prettier@3.0.3)
+   */
+
+  const graph = {
+    registry: [
+      { name: 'custom-prettier', version: '3.0.3', dependencies: { isexe: '1.0.0' } },
+      { name: 'isexe', version: '1.0.0' },
+    ],
+    root: {
+      name: 'myproject',
+      version: '1.0.0',
+      dependencies: { prettier: 'npm:custom-prettier@3.0.3' },
+    },
+    workspaces: [
+      { name: 'my-pkg', version: '1.0.0', dependencies: { prettier: 'npm:custom-prettier@3.0.3' } },
+    ],
+  }
+
+  const resolved = {
+    'myproject@1.0.0 (root)': {
+      'prettier@3.0.3': {
+        'isexe@1.0.0': {},
+      },
+      'my-pkg@1.0.0 (workspace)': {
+        'prettier@3.0.3': {
+          'isexe@1.0.0': {},
+        },
+      },
+    },
+  }
+
+  const { dir, registry } = await getRepo(graph)
+
+  const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
+  const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
+  await arborist.reify({ installStrategy: 'linked' })
+
+  // Verify symlink uses alias name, not real package name
+  t.ok(setupRequire(dir)('prettier'), 'root can require via alias "prettier"')
+  t.notOk(
+    pathExists(path.join(dir, 'node_modules', 'custom-prettier')),
+    'no custom-prettier symlink at root node_modules'
+  )
+
+  const asserted = new Set()
+  rule1.apply(t, dir, resolved, asserted)
+  rule2.apply(t, dir, resolved, asserted)
+  rule3.apply(t, dir, resolved, asserted)
+  rule7.apply(t, dir, resolved, asserted)
+})
+
 tap.test('failing optional peer deps are not installed', async t => {
   // Input of arborist
   const graph = {