From e18e71fb65be33d1f6afac77d88977d6b2512e72 Mon Sep 17 00:00:00 2001 From: nlf Date: Tue, 22 Mar 2022 09:35:12 -0700 Subject: [PATCH 1/2] fix(arborist): identify and repair invalid nodes in the virtual tree --- .../arborist/lib/arborist/build-ideal-tree.js | 15 +++++++++++++ .../arborist/build-ideal-tree.js.test.cjs | 21 ++++++++----------- .../test/arborist/build-ideal-tree.js | 6 +++--- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 3f001f9e9eb10..4db9b3e59003a 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -355,6 +355,21 @@ Try using the package name instead, e.g: }) .then(tree => { + // search the virtual tree for invalid edges, if any are found add their source to + // the depsQueue so that we'll fix it later + depth({ + tree, + getChildren: (node) => [...node.edgesOut.values()].map(edge => edge.to), + filter: node => node, + visit: node => { + for (const edge of node.edgesOut.values()) { + if (!edge.valid) { + this[_depsQueue].push(node) + break // no need to continue the loop after the first hit + } + } + }, + }) // null the virtual tree, because we're about to hack away at it // if you want another one, load another copy. this.idealTree = tree diff --git a/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs b/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs index b743dab958ffe..4f517c72694e3 100644 --- a/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs @@ -121519,7 +121519,7 @@ ArboristNode { } ` -exports[`test/arborist/build-ideal-tree.js TAP update global > update a single dep 1`] = ` +exports[`test/arborist/build-ideal-tree.js TAP update global > update a single dep, also fixes the invalid node 1`] = ` ArboristNode { "children": Map { "@isaacs/testing-dev-optional-flags" => ArboristNode { @@ -121541,7 +121541,6 @@ ArboristNode { "wrappy" => ArboristNode { "edgesIn": Set { EdgeIn { - "error": "INVALID", "from": "node_modules/@isaacs/testing-dev-optional-flags", "name": "wrappy", "spec": "^1.0.2", @@ -121551,7 +121550,8 @@ ArboristNode { "location": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", "name": "wrappy", "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", - "version": "1.0.0", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "version": "1.0.2", }, }, "edgesIn": Set { @@ -121570,7 +121570,6 @@ ArboristNode { "type": "prod", }, "wrappy" => EdgeOut { - "error": "INVALID", "name": "wrappy", "spec": "^1.0.2", "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", @@ -121771,7 +121770,7 @@ ArboristNode { } ` -exports[`test/arborist/build-ideal-tree.js TAP update global > updating missing dep should have no effect 1`] = ` +exports[`test/arborist/build-ideal-tree.js TAP update global > updating missing dep should have no effect, but fix the invalid node 1`] = ` ArboristNode { "children": Map { "@isaacs/testing-dev-optional-flags" => ArboristNode { @@ -121793,7 +121792,6 @@ ArboristNode { "wrappy" => ArboristNode { "edgesIn": Set { EdgeIn { - "error": "INVALID", "from": "node_modules/@isaacs/testing-dev-optional-flags", "name": "wrappy", "spec": "^1.0.2", @@ -121803,7 +121801,8 @@ ArboristNode { "location": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", "name": "wrappy", "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", - "version": "1.0.0", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "version": "1.0.2", }, }, "edgesIn": Set { @@ -121822,7 +121821,6 @@ ArboristNode { "type": "prod", }, "wrappy" => EdgeOut { - "error": "INVALID", "name": "wrappy", "spec": "^1.0.2", "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", @@ -121894,7 +121892,7 @@ ArboristNode { } ` -exports[`test/arborist/build-ideal-tree.js TAP update global > updating sub-dep has no effect 1`] = ` +exports[`test/arborist/build-ideal-tree.js TAP update global > updating sub-dep has no effect, but fixes the invalid node 1`] = ` ArboristNode { "children": Map { "@isaacs/testing-dev-optional-flags" => ArboristNode { @@ -121916,7 +121914,6 @@ ArboristNode { "wrappy" => ArboristNode { "edgesIn": Set { EdgeIn { - "error": "INVALID", "from": "node_modules/@isaacs/testing-dev-optional-flags", "name": "wrappy", "spec": "^1.0.2", @@ -121926,7 +121923,8 @@ ArboristNode { "location": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", "name": "wrappy", "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", - "version": "1.0.0", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "version": "1.0.2", }, }, "edgesIn": Set { @@ -121945,7 +121943,6 @@ ArboristNode { "type": "prod", }, "wrappy" => EdgeOut { - "error": "INVALID", "name": "wrappy", "spec": "^1.0.2", "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index e718b0b58cca4..6b34a47d7cf7c 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -2193,10 +2193,10 @@ t.test('update global', async t => { }) t.matchSnapshot(await printIdeal(path, { global: true, update: ['abbrev'] }), - 'updating missing dep should have no effect') + 'updating missing dep should have no effect, but fix the invalid node') t.matchSnapshot(await printIdeal(path, { global: true, update: ['wrappy'] }), - 'updating sub-dep has no effect') + 'updating sub-dep has no effect, but fixes the invalid node') const invalidArgs = [ 'once@1.4.0', @@ -2214,7 +2214,7 @@ t.test('update global', async t => { } t.matchSnapshot(await printIdeal(path, { global: true, update: ['once'] }), - 'update a single dep') + 'update a single dep, also fixes the invalid node') t.matchSnapshot(await printIdeal(path, { global: true, update: true }), 'update all the deps') }) From bf919a5695429122274a99efc1275c2b327bc129 Mon Sep 17 00:00:00 2001 From: nlf Date: Wed, 23 Mar 2022 08:23:56 -0700 Subject: [PATCH 2/2] fix: make sure we loadOverrides on the root node in loadVirtual() by loading the overrides on the root of the virtual tree, we effectively ensure that `npm ci` will throw an error when your package.json overrides are not in sync with the tree in your package-lock.json --- workspaces/arborist/lib/arborist/load-virtual.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/workspaces/arborist/lib/arborist/load-virtual.js b/workspaces/arborist/lib/arborist/load-virtual.js index 4d65e3da6f683..8a41e7686e7e1 100644 --- a/workspaces/arborist/lib/arborist/load-virtual.js +++ b/workspaces/arborist/lib/arborist/load-virtual.js @@ -79,7 +79,7 @@ module.exports = cls => class VirtualLoader extends cls { async [loadRoot] (s) { const pj = this.path + '/package.json' const pkg = await rpj(pj).catch(() => s.data.packages['']) || {} - return this[loadWorkspaces](this[loadNode]('', pkg)) + return this[loadWorkspaces](this[loadNode]('', pkg, true)) } async [loadFromShrinkwrap] (s, root) { @@ -264,7 +264,7 @@ module.exports = cls => class VirtualLoader extends cls { } } - [loadNode] (location, sw) { + [loadNode] (location, sw, loadOverrides) { const p = this.virtualTree ? this.virtualTree.realpath : this.path const path = resolve(p, location) // shrinkwrap doesn't include package name unless necessary @@ -290,6 +290,7 @@ module.exports = cls => class VirtualLoader extends cls { optional, devOptional, peer, + loadOverrides, }) // cast to boolean because they're undefined in the lock file when false node.extraneous = !!sw.extraneous