From 5186e06792239301285f188621150874746bdbcb Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Fri, 21 Jan 2022 14:47:47 -0500 Subject: [PATCH] fix(arborist): load actual tree on named updates Arborist was not loading the actual tree when using named updates for global updates, that would result in removing all previously installed deps from a global install anytime the user would try to run `npm update `. This changeset fixes the problem by allowing the load of the actual tree if the `global` and `update.names` options are defined. Added a few more tests to illustrate but some of the snapshots already included were actually demonstrating the problem by having empty trees as result, these are now also updated with the expected tree result. Fixes: https://github.com/npm/cli/issues/3175 --- .../arborist/lib/arborist/build-ideal-tree.js | 2 +- .../arborist/build-ideal-tree.js.test.cjs | 306 +++++++++++++++++- .../test/arborist/build-ideal-tree.js | 4 + 3 files changed, 309 insertions(+), 3 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index f20a554bd5ee8..df4f5015919db 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -320,7 +320,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { // Load on a new Arborist object, so the Nodes aren't the same, // or else it'll get super confusing when we change them! .then(async root => { - if (!this[_updateAll] && !this[_global] && !root.meta.loadedFromDisk) { + if ((!this[_updateAll] && !this[_global] && !root.meta.loadedFromDisk) || (this[_global] && this[_updateNames].length)) { await new this.constructor(this.options).loadActual({ root }) const tree = root.target // even though we didn't load it from a package-lock.json FILE, 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 93ea45862e8f8..b743dab958ffe 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 @@ -121522,6 +121522,66 @@ ArboristNode { exports[`test/arborist/build-ideal-tree.js TAP update global > update a single dep 1`] = ` ArboristNode { "children": Map { + "@isaacs/testing-dev-optional-flags" => ArboristNode { + "children": Map { + "own-or" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "from": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "own-or", + "spec": "^1.0.0", + "type": "prod", + }, + }, + "location": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "name": "own-or", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "version": "1.0.0", + }, + "wrappy" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "error": "INVALID", + "from": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "wrappy", + "spec": "^1.0.2", + "type": "prod", + }, + }, + "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", + }, + }, + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "@isaacs/testing-dev-optional-flags", + "spec": "*", + "type": "prod", + }, + }, + "edgesOut": Map { + "own-or" => EdgeOut { + "name": "own-or", + "spec": "^1.0.0", + "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "type": "prod", + }, + "wrappy" => EdgeOut { + "error": "INVALID", + "name": "wrappy", + "spec": "^1.0.2", + "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", + "type": "prod", + }, + }, + "location": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "@isaacs/testing-dev-optional-flags", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/@isaacs/testing-dev-optional-flags", + "version": "1.0.0", + }, "once" => ArboristNode { "children": Map { "wrappy" => ArboristNode { @@ -121536,8 +121596,7 @@ ArboristNode { "location": "node_modules/once/node_modules/wrappy", "name": "wrappy", "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/once/node_modules/wrappy", - "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", - "version": "1.0.2", + "version": "1.0.1", }, }, "edgesIn": Set { @@ -121564,6 +121623,12 @@ ArboristNode { }, }, "edgesOut": Map { + "@isaacs/testing-dev-optional-flags" => EdgeOut { + "name": "@isaacs/testing-dev-optional-flags", + "spec": "*", + "to": "node_modules/@isaacs/testing-dev-optional-flags", + "type": "prod", + }, "once" => EdgeOut { "name": "once", "spec": "*", @@ -121706,8 +121771,245 @@ ArboristNode { } ` +exports[`test/arborist/build-ideal-tree.js TAP update global > updating missing dep should have no effect 1`] = ` +ArboristNode { + "children": Map { + "@isaacs/testing-dev-optional-flags" => ArboristNode { + "children": Map { + "own-or" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "from": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "own-or", + "spec": "^1.0.0", + "type": "prod", + }, + }, + "location": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "name": "own-or", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "version": "1.0.0", + }, + "wrappy" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "error": "INVALID", + "from": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "wrappy", + "spec": "^1.0.2", + "type": "prod", + }, + }, + "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", + }, + }, + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "@isaacs/testing-dev-optional-flags", + "spec": "*", + "type": "prod", + }, + }, + "edgesOut": Map { + "own-or" => EdgeOut { + "name": "own-or", + "spec": "^1.0.0", + "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "type": "prod", + }, + "wrappy" => EdgeOut { + "error": "INVALID", + "name": "wrappy", + "spec": "^1.0.2", + "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", + "type": "prod", + }, + }, + "location": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "@isaacs/testing-dev-optional-flags", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/@isaacs/testing-dev-optional-flags", + "version": "1.0.0", + }, + "once" => ArboristNode { + "children": Map { + "wrappy" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "from": "node_modules/once", + "name": "wrappy", + "spec": "1", + "type": "prod", + }, + }, + "location": "node_modules/once/node_modules/wrappy", + "name": "wrappy", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/once/node_modules/wrappy", + "version": "1.0.1", + }, + }, + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "once", + "spec": "*", + "type": "prod", + }, + }, + "edgesOut": Map { + "wrappy" => EdgeOut { + "name": "wrappy", + "spec": "1", + "to": "node_modules/once/node_modules/wrappy", + "type": "prod", + }, + }, + "location": "node_modules/once", + "name": "once", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/once", + "version": "1.3.1", + }, + }, + "edgesOut": Map { + "@isaacs/testing-dev-optional-flags" => EdgeOut { + "name": "@isaacs/testing-dev-optional-flags", + "spec": "*", + "to": "node_modules/@isaacs/testing-dev-optional-flags", + "type": "prod", + }, + "once" => EdgeOut { + "name": "once", + "spec": "*", + "to": "node_modules/once", + "type": "prod", + }, + }, + "isProjectRoot": true, + "location": "", + "name": "tap-testdir-build-ideal-tree-update-global", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global", +} +` + exports[`test/arborist/build-ideal-tree.js TAP update global > updating sub-dep has no effect 1`] = ` ArboristNode { + "children": Map { + "@isaacs/testing-dev-optional-flags" => ArboristNode { + "children": Map { + "own-or" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "from": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "own-or", + "spec": "^1.0.0", + "type": "prod", + }, + }, + "location": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "name": "own-or", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "version": "1.0.0", + }, + "wrappy" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "error": "INVALID", + "from": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "wrappy", + "spec": "^1.0.2", + "type": "prod", + }, + }, + "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", + }, + }, + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "@isaacs/testing-dev-optional-flags", + "spec": "*", + "type": "prod", + }, + }, + "edgesOut": Map { + "own-or" => EdgeOut { + "name": "own-or", + "spec": "^1.0.0", + "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/own-or", + "type": "prod", + }, + "wrappy" => EdgeOut { + "error": "INVALID", + "name": "wrappy", + "spec": "^1.0.2", + "to": "node_modules/@isaacs/testing-dev-optional-flags/node_modules/wrappy", + "type": "prod", + }, + }, + "location": "node_modules/@isaacs/testing-dev-optional-flags", + "name": "@isaacs/testing-dev-optional-flags", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/@isaacs/testing-dev-optional-flags", + "version": "1.0.0", + }, + "once" => ArboristNode { + "children": Map { + "wrappy" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "from": "node_modules/once", + "name": "wrappy", + "spec": "1", + "type": "prod", + }, + }, + "location": "node_modules/once/node_modules/wrappy", + "name": "wrappy", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/once/node_modules/wrappy", + "version": "1.0.1", + }, + }, + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "once", + "spec": "*", + "type": "prod", + }, + }, + "edgesOut": Map { + "wrappy" => EdgeOut { + "name": "wrappy", + "spec": "1", + "to": "node_modules/once/node_modules/wrappy", + "type": "prod", + }, + }, + "location": "node_modules/once", + "name": "once", + "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-update-global/node_modules/once", + "version": "1.3.1", + }, + }, + "edgesOut": Map { + "@isaacs/testing-dev-optional-flags" => EdgeOut { + "name": "@isaacs/testing-dev-optional-flags", + "spec": "*", + "to": "node_modules/@isaacs/testing-dev-optional-flags", + "type": "prod", + }, + "once" => EdgeOut { + "name": "once", + "spec": "*", + "to": "node_modules/once", + "type": "prod", + }, + }, "isProjectRoot": true, "location": "", "name": "tap-testdir-build-ideal-tree-update-global", diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 2c058a6a3283e..d8bfe8200c71c 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -2092,6 +2092,10 @@ t.test('update global', async t => { }, }, }) + + t.matchSnapshot(await printIdeal(path, { global: true, update: ['abbrev'] }), + 'updating missing dep should have no effect') + t.matchSnapshot(await printIdeal(path, { global: true, update: ['wrappy'] }), 'updating sub-dep has no effect') t.matchSnapshot(await printIdeal(path, { global: true, update: ['once'] }),