Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions workspaces/arborist/lib/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,10 @@ class Shrinkwrap {
continue
}
const loc = relpath(this.path, node.path)
// Drop lockfile entries for extraneous nodes outside node_modules. These are stale workspace entries: the workspace was removed from package.json or its directory was deleted, so it should not be tracked in package-lock.json.
if (node.extraneous && !/(^|\/)node_modules\//.test(loc) && loc !== 'node_modules') {
continue
}
this.data.packages[loc] = Shrinkwrap.metaFromNode(
node,
this.path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3589,10 +3589,6 @@ exports[`test/arborist/reify.js TAP filtered reification in workspaces > hidden
"apps/x": {
"version": "1.2.3"
},
"foo/x": {
"version": "1.2.3",
"extraneous": true
},
"node_modules/c": {
"resolved": "packages/c",
"link": true
Expand Down Expand Up @@ -33028,9 +33024,6 @@ Object {
"e",
],
},
"e": Object {
"extraneous": true,
},
"node_modules/a": Object {
"extraneous": true,
"inBundle": true,
Expand Down
45 changes: 0 additions & 45 deletions workspaces/arborist/tap-snapshots/test/shrinkwrap.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2734,14 +2734,6 @@ Object {
"name": "example",
"version": "1.0.0",
},
"../bar": Object {
"extraneous": true,
"version": "1.0.0",
},
"../linked-node-modules/foo": Object {
"extraneous": true,
"version": "1.0.0",
},
"node_modules/bar": Object {
"link": true,
"resolved": "../bar",
Expand Down Expand Up @@ -9755,16 +9747,6 @@ Object {
"": Object {
"name": "workspace3",
},
"app": Object {
"dependencies": Object {
"a": "",
"b": "",
"c": "",
"i": "",
},
"extraneous": true,
"version": "1.2.3",
},
"app/node_modules/i": Object {
"extraneous": true,
"version": "1.2.3",
Expand All @@ -9785,41 +9767,14 @@ Object {
"link": true,
"resolved": "packages/c",
},
"packages/a": Object {
"dependencies": Object {
"b": "",
"c": "",
"x": "",
},
"extraneous": true,
"version": "1.2.3",
},
"packages/a/node_modules/x": Object {
"extraneous": true,
"version": "1.2.3",
},
"packages/b": Object {
"dependencies": Object {
"a": "",
"c": "",
"y": "",
},
"extraneous": true,
"version": "1.2.3",
},
"packages/b/node_modules/y": Object {
"extraneous": true,
"version": "1.2.3",
},
"packages/c": Object {
"dependencies": Object {
"a": "",
"b": "",
"z": "",
},
"extraneous": true,
"version": "1.2.3",
},
"packages/c/node_modules/z": Object {
"extraneous": true,
"version": "1.2.3",
Expand Down
59 changes: 59 additions & 0 deletions workspaces/arborist/test/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -2055,6 +2055,65 @@ t.test('filtered reification in workspaces', async t => {
'hidden lockfile - foo/x linked, c, old x, removed a')
})

// Regression for https://github.com/npm/cli/issues/5463: a workspace whose directory has been deleted should not leave behind an extraneous entry (or a lingering reference in the root's workspaces array) in package-lock.json after `npm install`.
t.test('removed workspace is pruned from package-lock.json', async t => {
const setup = () => {
const path = t.testdir({
'package.json': JSON.stringify({
name: 'remove-ws',
version: '1.0.0',
workspaces: ['packages/a', 'packages/b'],
}),
packages: {
a: {
'package.json': JSON.stringify({ name: 'a', version: '1.0.0' }),
},
b: {
'package.json': JSON.stringify({ name: 'b', version: '1.0.0' }),
},
},
})
return path
}

// The lockfile's root.workspaces array mirrors package.json verbatim and is intentionally not mutated here, so we only assert that orphan package/link entries are dropped.
const assertClean = (t, path, label) => {
const lock = JSON.parse(fs.readFileSync(`${path}/package-lock.json`, 'utf8'))
t.notOk(lock.packages['packages/b'],
`${label}: packages/b entry removed from lockfile`)
t.notOk(lock.packages['node_modules/b'],
`${label}: node_modules/b link removed from lockfile`)
t.notOk(lock.dependencies && lock.dependencies.b,
`${label}: dependencies.b removed from legacy lockfile`)
}

for (const strategy of ['hoisted', 'linked']) {
t.test(`${strategy} strategy, package.json kept stale`, async t => {
const path = setup()
createRegistry(t, false)
await reify(path, { installStrategy: strategy })
// Remove only the directory, leave package.json's workspaces array alone.
fs.rmSync(`${path}/packages/b`, { recursive: true, force: true })
await reify(path, { installStrategy: strategy })
assertClean(t, path, `${strategy}/keep-pj`)
})

t.test(`${strategy} strategy, package.json updated`, async t => {
const path = setup(strategy)
createRegistry(t, false)
await reify(path, { installStrategy: strategy })
fs.rmSync(`${path}/packages/b`, { recursive: true, force: true })
fs.writeFileSync(`${path}/package.json`, JSON.stringify({
name: 'remove-ws',
version: '1.0.0',
workspaces: ['packages/a'],
}))
await reify(path, { installStrategy: strategy })
assertClean(t, path, `${strategy}/clean-pj`)
})
}
})

t.test('project with bundled deps and a link dep on itself', async t => {
const pkg = {
name: '@isaacs/testing-bundle-self-link',
Expand Down
Loading