Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Commit

Permalink
Properly track dev edges on virtual root nodes
Browse files Browse the repository at this point in the history
If the virtualRoot node has a sourceReference, and that sourceReference
node would get its devDep edges loaded, then the virtualRoot node should
as well.

This caused a very useless ERESOLVE error, which could not be properly
forced away, when installing in a project that had conflicting peer deps
in its dev tree.

Related to: npm/cli#2548
  • Loading branch information
isaacs committed Jan 29, 2021
1 parent 3bf786c commit f5be3c4
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 12 deletions.
4 changes: 3 additions & 1 deletion lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,9 @@ class Node {
// Linked targets that are disconnected from the tree are tops,
// but don't have a 'path' field, only a 'realpath', because we
// don't know their canonical location. We don't need their devDeps.
if (this.isTop && this.path && !this.sourceReference)
const { isTop, path, sourceReference } = this
const { isTop: srcTop, path: srcPath } = sourceReference || {}
if (isTop && path && (!sourceReference || srcTop && srcPath))
this[_loadDepType](this.package.devDependencies, 'dev')

const pd = this.package.peerDependencies
Expand Down
53 changes: 42 additions & 11 deletions test/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
const {relative, basename, resolve} = require('path')
const {basename, resolve} = require('path')
const t = require('tap')
const Arborist = require('../..')
const fixtures = resolve(__dirname, '../fixtures')
// load the symbolic links that we depend on
require(fixtures)
const registryServer = require('../fixtures/registry-mocks/server.js')
const {registry, auditResponse} = registryServer
const npa = require('npm-package-arg')
Expand Down Expand Up @@ -31,11 +33,10 @@ const normalizePaths = obj => {
return new Map([...obj].map(([name, val]) => [name, normalizePaths(val)]))

for (const key in obj) {
if (['location', 'path', 'realpath', 'resolved', 'spec'].includes(key)) {
if (['location', 'path', 'realpath', 'resolved', 'spec'].includes(key))
obj[key] = normalizePath(obj[key])
} else if (typeof obj[key] === 'object' && obj[key] !== null) {
else if (typeof obj[key] === 'object' && obj[key] !== null)
obj[key] = normalizePaths(obj[key])
}
}
return obj
}
Expand Down Expand Up @@ -2257,8 +2258,8 @@ t.test('peer dep that needs to be replaced', async t => {
const path = t.testdir({
'package.json': JSON.stringify({
dependencies: {
"@pmmmwh/react-refresh-webpack-plugin": "^0.4.3",
"webpack-dev-server": "^3.11.0"
'@pmmmwh/react-refresh-webpack-plugin': '^0.4.3',
'webpack-dev-server': '^3.11.0',
},
}),
})
Expand All @@ -2270,11 +2271,11 @@ t.test('peer dep override with dep sets being replaced', async t => {
// that has a peer dependency on webpack 4, it overrides but otherwise fails.
const path = t.testdir({
'package.json': JSON.stringify({
"devDependencies": {
"webpack": "5.0.0",
"webpack-dev-server": "3.11.0"
}
})
devDependencies: {
webpack: '5.0.0',
'webpack-dev-server': '3.11.0',
},
}),
})
await t.rejects(printIdeal(path), { code: 'ERESOLVE' })
t.matchSnapshot(await printIdeal(path, { force: true }))
Expand Down Expand Up @@ -2324,3 +2325,33 @@ t.test('do not fail if root peerDep looser than meta peerDep', async t => {
const path = resolve(fixtures, 'test-peer-looser-than-dev')
t.matchSnapshot(await printIdeal(path))
})

t.test('set the current on ERESOLVE triggered by devDeps', async t => {
// fixes a bug where the dev deps are not loaded properly in the
// virtual root, leading to incomprehensible ERESOLVE explanations
const path = t.testdir({
'package.json': JSON.stringify({
name: 'root-dev-peer-conflict',
version: '1.0.0',
devDependencies: {
eslint: '^4.19.1',
'eslint-config-standard': '^11.0.0',
},
}),
})

const arb = new Arborist({ path, ...OPT })
t.rejects(arb.buildIdealTree(), {
code: 'ERESOLVE',
current: {
name: 'eslint',
version: '4.19.1',
whileInstalling: {
name: 'root-dev-peer-conflict',
version: '1.0.0',
path: resolve(path),
},
location: 'node_modules/eslint',
},
})
})
16 changes: 16 additions & 0 deletions test/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -2147,3 +2147,19 @@ t.test('isProjectRoot shows if the node is the root link target', async t => {
t.equal(link.isRoot, true)
t.equal(n.isRoot, false)
})

t.test('virtual references to root node has devDep edges', async t => {
const root = new Node({
path: '/some/project/path',
pkg: {
devDependencies: {
a: '1',
},
},
})
const virtualRoot = new Node({
path: '/virtual-root',
sourceReference: root,
})
t.equal(virtualRoot.edgesOut.get('a').type, 'dev')
})

0 comments on commit f5be3c4

Please sign in to comment.