diff options
author | Ruy Adorno <ruyadorno@hotmail.com> | 2022-01-20 21:52:00 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-01-20 21:52:00 +0300 |
commit | cfd59b8c81078f842328b13a23a234150842cd58 (patch) | |
tree | 74fd15153445b59dda6b1d5ec4e8c64436115e94 /workspaces/arborist/test | |
parent | 510f0ecbc9970ed8c8993107cc03cf27b7b996dc (diff) |
fix: npm update --save (#4223)
Previously `npm update` was not respecting the `save` option, it
would be impossible for users to use `npm update` and automatically
update their `package.json` files.
This fixes it by adding extra steps on `Arborist.reify._saveIdealTree`
to read direct dependencies of any `package.json` and update them as
needed when reifying using the `update` and `save` options.
- Uses config.isDefault to set a different value for the `save` config
for both the update and dedupe commands
- Tweaks arborist to make sure saveIdealTree preserves the behavior of
skipping writing to package-lock.json on save=false for install while
still writing the lockfile for `npm update` with its new default value
of save=false.
- Updated and added some new tests on arborist to cover for these tweaks
- Added `npm update --save` smoke test on cli
Fixes: https://github.com/npm/cli/issues/708
Fixes: https://github.com/npm/cli/issues/2704
Relates to: https://github.com/npm/feedback/discussions/270
Diffstat (limited to 'workspaces/arborist/test')
7 files changed, 301 insertions, 6 deletions
diff --git a/workspaces/arborist/test/arborist/deduper.js b/workspaces/arborist/test/arborist/deduper.js index 3ec37bd62..511ba87bf 100644 --- a/workspaces/arborist/test/arborist/deduper.js +++ b/workspaces/arborist/test/arborist/deduper.js @@ -18,7 +18,7 @@ const fixture = (t, p) => require('../fixtures/reify-cases/' + p)(t) const cache = t.testdir() const dedupeTree = (path, opt) => - new Arborist({ registry, path, cache, ...(opt || {}) }).dedupe(opt) + new Arborist({ registry, path, cache, save: false, ...(opt || {}) }).dedupe(opt) t.test('dedupes with actual tree', async t => { const path = fixture(t, 'dedupe-actual') diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index c8c4cb137..d5fc166a5 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/workspaces/arborist/test/arborist/reify.js @@ -256,6 +256,7 @@ t.test('a workspace with a duplicated nested conflicted dep', t => t.test('testing-peer-deps nested with update', t => t.resolveMatchSnapshot(printReified(fixture(t, 'testing-peer-deps-nested'), { update: { names: ['@isaacs/testing-peer-deps'] }, + save: false, }))) t.test('update a bundling node without updating all of its deps', t => { @@ -392,7 +393,7 @@ t.test('multiple bundles at the same level', t => { t.test('update a node without updating its children', t => t.resolveMatchSnapshot(printReified(fixture(t, 'once-outdated'), - { update: { names: ['once'] } }))) + { update: { names: ['once'] }, save: false }))) t.test('do not add shrinkwrapped deps', t => t.resolveMatchSnapshot(printReified( @@ -508,6 +509,7 @@ t.test('update a node without updating a child that has bundle deps', t => { const path = fixture(t, 'testing-bundledeps-3') return t.resolveMatchSnapshot(printReified(path, { update: ['@isaacs/testing-bundledeps-parent'], + save: false, })) }) @@ -524,9 +526,16 @@ t.test('optional dependency failures', t => { 'optional-metadep-postinstall-fail', 'optional-metadep-allinstall-fail', ] - t.plan(cases.length) - cases.forEach(c => t.test(c, t => - t.resolveMatchSnapshot(printReified(fixture(t, c), { update: true })))) + t.plan(cases.length * 2) + let p = [...cases.map(c => t.test(`${c} save=false`, t => + t.resolveMatchSnapshot(printReified(fixture(t, c), + { update: true, save: false }))))] + + // npm update --save + p = [...cases.map(c => t.test(`${c} save=true`, t => + t.resolveMatchSnapshot(printReified(fixture(t, c), + { update: true, save: true }))))] + return p }) t.test('failure to fetch prod dep is failure', t => @@ -665,6 +674,7 @@ t.test('rollbacks', { buffered: false }, t => { return t.resolveMatchSnapshot(a.reify({ update: ['@isaacs/testing-bundledeps-parent'], + save: false, }).then(printTree)) }) @@ -845,6 +855,7 @@ t.test('rollbacks', { buffered: false }, t => { return t.resolveMatchSnapshot(a.reify({ update: ['@isaacs/testing-bundledeps-parent'], + save: false, }).then(tree => printTree(tree))).then(() => { const warnings = check() t.equal(warnings.length, 2) @@ -1367,7 +1378,7 @@ t.test('save complete lockfile on update-all', async t => { const lock = () => fs.readFileSync(`${path}/package-lock.json`, 'utf8') await reify(path, { add: ['abbrev@1.0.4'] }) t.matchSnapshot(lock(), 'should have abbrev 1.0.4') - await reify(path, { update: true }) + await reify(path, { update: true, save: false }) t.matchSnapshot(lock(), 'should update, but not drop root metadata') }) @@ -2432,3 +2443,134 @@ t.test('add local dep with existing dev + peer/optional', async t => { t.equal(tree.children.get('abbrev').resolved, 'file:../../dep', 'resolved') t.equal(tree.children.size, 1, 'children') }) + +t.test('save package.json on update', t => { + t.test('should save many deps in multiple package.json when using save=true', async t => { + const path = fixture(t, 'workspaces-need-update') + + await reify(path, { update: true, save: true }) + + t.same( + require(resolve(path, 'package.json')), + { dependencies: { abbrev: '^1.1.1' }, workspaces: ['a', 'b'] }, + 'should save top level dep update to root package.json' + ) + t.same( + require(resolve(path, 'a', 'package.json')), + { dependencies: { abbrev: '^1.1.1', once: '^1.4.0' } }, + 'should save workspace dep to its package.json file') + + t.matchSnapshot( + fs.readFileSync(resolve(path, 'package-lock.json'), 'utf8'), + 'should update lockfile with many deps updated package.json save=true' + ) + }) + + t.test('should not save many deps in multiple package.json when using save=false', async t => { + const path = fixture(t, 'workspaces-need-update') + + await reify(path, { update: true, save: false }) + + t.same( + require(resolve(path, 'package.json')), + { + dependencies: { abbrev: '^1.0.4' }, + workspaces: ['a', 'b'], + }, + 'should not save top level dep update to root package.json' + ) + t.same( + require(resolve(path, 'a', 'package.json')), + { dependencies: { abbrev: '^1.0.4', once: '^1.3.2' } }, + 'should not save workspace dep to its package.json file') + + // package-lock entries will still get updated: + t.matchSnapshot( + fs.readFileSync(resolve(path, 'package-lock.json'), 'utf8'), + 'should update lockfile with many deps updated package.json save=false' + ) + }) + + t.test('should not save any with save=false and package-lock=false', async t => { + const path = fixture(t, 'workspaces-need-update') + + await reify(path, { update: true, save: false, packageLock: false }) + + t.same( + require(resolve(path, 'package.json')), + { + dependencies: { abbrev: '^1.0.4' }, + workspaces: ['a', 'b'], + }, + 'should not save top level dep update to root package.json' + ) + t.same( + require(resolve(path, 'a', 'package.json')), + { dependencies: { abbrev: '^1.0.4', once: '^1.3.2' } }, + 'should not save workspace dep to its package.json file') + + // package-lock entries will still get updated: + t.matchSnapshot( + JSON.stringify(JSON.parse(fs.readFileSync(resolve(path, 'package-lock.json'), 'utf8')), null, 2), + 'should update lockfile with many deps updated package.json save=false' + ) + }) + + t.test('should update named dep across multiple package.json using save=true', async t => { + const path = fixture(t, 'workspaces-need-update') + + await reify(path, { update: ['abbrev'], save: true }) + + t.same( + require(resolve(path, 'package.json')), + { + dependencies: { abbrev: '^1.1.1' }, + workspaces: ['a', 'b'], + }, + 'should save top level dep update to root package.json' + ) + t.same( + require(resolve(path, 'a', 'package.json')), + { dependencies: { abbrev: '^1.1.1', once: '^1.3.2' } }, + 'should save only workspace a updated dep to its package.json file') + t.same( + require(resolve(path, 'b', 'package.json')), + { dependencies: { abbrev: '^1.1.1' } }, + 'should save only workspace b updated dep to its package.json file') + + t.matchSnapshot( + fs.readFileSync(resolve(path, 'package-lock.json'), 'utf8'), + 'should update lockfile with many deps updated package.json save=true' + ) + }) + + t.test('should update single named dep across multiple package.json using save=true', async t => { + const path = fixture(t, 'workspaces-need-update') + + await reify(path, { update: ['once'], save: true }) + + t.same( + require(resolve(path, 'package.json')), + { + dependencies: { abbrev: '^1.0.4' }, + workspaces: ['a', 'b'], + }, + 'should save no top level dep update to root package.json' + ) + t.same( + require(resolve(path, 'a', 'package.json')), + { dependencies: { abbrev: '^1.0.4', once: '^1.4.0' } }, + 'should save only workspace single updated dep to its package.json file') + t.same( + require(resolve(path, 'b', 'package.json')), + { dependencies: { abbrev: '^1.0.4' } }, + 'should not change workspace b package.json file') + + t.matchSnapshot( + fs.readFileSync(resolve(path, 'package-lock.json'), 'utf8'), + 'should update lockfile with single dep updated package.json save=true' + ) + }) + + t.end() +}) diff --git a/workspaces/arborist/test/fixtures/reify-cases/workspaces-need-update.js b/workspaces/arborist/test/fixtures/reify-cases/workspaces-need-update.js new file mode 100644 index 000000000..2d57f05a0 --- /dev/null +++ b/workspaces/arborist/test/fixtures/reify-cases/workspaces-need-update.js @@ -0,0 +1,83 @@ +// generated from test/fixtures/workspaces-need-update +module.exports = t => { + const path = t.testdir({ + "a": { + "package.json": JSON.stringify({ + "dependencies": { + "abbrev": "^1.0.4", + "once": "^1.3.2" + } + }) + }, + "b": { + "package.json": JSON.stringify({ + "dependencies": { + "abbrev": "^1.0.4" + } + }) + }, + "package-lock.json": JSON.stringify({ + "name": "workspaces-need-update", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "workspaces": [ + "a", + "b" + ], + "dependencies": { + "abbrev": "^1.0.4" + } + }, + "a": { + "dependencies": { + "abbrev": "^1.0.4", + "once": "^1.3.2" + } + }, + "b": { + "dependencies": { + "abbrev": "^1.0.4" + } + }, + "node_modules/a": { + "resolved": "a", + "link": true + }, + "node_modules/abbrev": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.0.4.tgz", + "integrity": "sha1-vVWuXkE7oXIu5Mq6H26hBBSlns0=" + }, + "node_modules/b": { + "resolved": "b", + "link": true + }, + "node_modules/once": { + "version": "1.3.2", + "resolved": "https://registry.npmjs.org/once/-/once-1.3.2.tgz", + "integrity": "sha1-2P7sqTsDnsHc3ud0HJK9rF4oCBs=", + "dependencies": { + "wrappy": "1" + } + }, + "node_modules/wrappy": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" + } + } + }), + "package.json": JSON.stringify({ + "dependencies": { + "abbrev": "^1.0.4" + }, + "workspaces": [ + "a", + "b" + ] + }) +}) + return path +} diff --git a/workspaces/arborist/test/fixtures/workspaces-need-update/a/package.json b/workspaces/arborist/test/fixtures/workspaces-need-update/a/package.json new file mode 100644 index 000000000..765847437 --- /dev/null +++ b/workspaces/arborist/test/fixtures/workspaces-need-update/a/package.json @@ -0,0 +1,6 @@ +{ + "dependencies": { + "abbrev": "^1.0.4", + "once": "^1.3.2" + } +} diff --git a/workspaces/arborist/test/fixtures/workspaces-need-update/b/package.json b/workspaces/arborist/test/fixtures/workspaces-need-update/b/package.json new file mode 100644 index 000000000..8af6070f2 --- /dev/null +++ b/workspaces/arborist/test/fixtures/workspaces-need-update/b/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "abbrev": "^1.0.4" + } +} diff --git a/workspaces/arborist/test/fixtures/workspaces-need-update/package-lock.json b/workspaces/arborist/test/fixtures/workspaces-need-update/package-lock.json new file mode 100644 index 000000000..43865795a --- /dev/null +++ b/workspaces/arborist/test/fixtures/workspaces-need-update/package-lock.json @@ -0,0 +1,53 @@ +{ + "name": "workspaces-need-update", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "workspaces": [ + "a", + "b" + ], + "dependencies": { + "abbrev": "^1.0.4" + } + }, + "a": { + "dependencies": { + "abbrev": "^1.0.4", + "once": "^1.3.2" + } + }, + "b": { + "dependencies": { + "abbrev": "^1.0.4" + } + }, + "node_modules/a": { + "resolved": "a", + "link": true + }, + "node_modules/abbrev": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.0.4.tgz", + "integrity": "sha1-vVWuXkE7oXIu5Mq6H26hBBSlns0=" + }, + "node_modules/b": { + "resolved": "b", + "link": true + }, + "node_modules/once": { + "version": "1.3.2", + "resolved": "https://registry.npmjs.org/once/-/once-1.3.2.tgz", + "integrity": "sha1-2P7sqTsDnsHc3ud0HJK9rF4oCBs=", + "dependencies": { + "wrappy": "1" + } + }, + "node_modules/wrappy": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" + } + } +} diff --git a/workspaces/arborist/test/fixtures/workspaces-need-update/package.json b/workspaces/arborist/test/fixtures/workspaces-need-update/package.json new file mode 100644 index 000000000..ebdce120c --- /dev/null +++ b/workspaces/arborist/test/fixtures/workspaces-need-update/package.json @@ -0,0 +1,6 @@ +{ + "dependencies": { + "abbrev": "^1.0.4" + }, + "workspaces": ["a", "b"] +} |