diff --git a/workspaces/arborist/lib/arborist/isolated-reifier.js b/workspaces/arborist/lib/arborist/isolated-reifier.js index 9822645d2527c..8da64ce965b7e 100644 --- a/workspaces/arborist/lib/arborist/isolated-reifier.js +++ b/workspaces/arborist/lib/arborist/isolated-reifier.js @@ -105,7 +105,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 @@ -145,6 +145,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 && !n.inert).map(this.externalProxyMemo)) result.externalOptionalDependencies = await Promise.all(optionalDeps.filter(n => !n.inert).map(this.externalProxyMemo)) @@ -155,7 +170,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 @@ -228,7 +245,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: () => { @@ -246,7 +263,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 = { @@ -301,7 +318,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: [], @@ -335,7 +352,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 => { @@ -361,13 +378,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 @@ -379,12 +400,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)) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index defbdb1d255b5..5f376e94a4cec 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -422,7 +422,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) } diff --git a/workspaces/arborist/test/fixtures/isolated-nock.js b/workspaces/arborist/test/fixtures/isolated-nock.js index 6593c7f0d1935..4d08c1067c6e0 100644 --- a/workspaces/arborist/test/fixtures/isolated-nock.js +++ b/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) diff --git a/workspaces/arborist/test/isolated-mode.js b/workspaces/arborist/test/isolated-mode.js index c086656054987..8e2174fe61bfd 100644 --- a/workspaces/arborist/test/isolated-mode.js +++ b/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 = {