Skip to content

Commit

Permalink
fix(arborist): load actual tree on named updates
Browse files Browse the repository at this point in the history 
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 <pkgname>`.

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: #3175
  • Loading branch information
@ruyadorno
ruyadorno committed Jan 26, 2022
1 parent 0813625 commit 1f853f8
Show file tree
Hide file tree
Showing 3 changed files with 309 additions and 3 deletions.
2 changes: 1 addition & 1 deletion workspaces/arborist/lib/arborist/build-ideal-tree.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
306 changes: 304 additions & 2 deletions workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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": "*",
Expand Down Expand Up @@ -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",
Expand Down
4 changes: 4 additions & 0 deletions workspaces/arborist/test/arborist/build-ideal-tree.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'] }),
Expand Down

0 comments on commit 1f853f8

Please sign in to comment.