From b50701ad827048b7ed891553c0e999ca0b2170e4 Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Wed, 19 Jan 2022 22:19:03 -0500 Subject: [PATCH] fix: addressed code review - added more tests scenarios - fixed some typos --- docs/content/using-npm/config.md | 8 +- lib/commands/update.js | 2 +- lib/utils/config/definitions.js | 8 +- smoke-tests/index.js | 55 ++- tap-snapshots/smoke-tests/index.js.test.cjs | 70 +--- .../lib/utils/config/definitions.js.test.cjs | 8 +- .../lib/utils/config/describe-all.js.test.cjs | 8 +- workspaces/arborist/lib/arborist/reify.js | 16 +- .../test/arborist/reify.js.test.cjs | 392 +++++++++++++++++- workspaces/arborist/test/arborist/reify.js | 244 ++++++----- .../reify-cases/workspaces-need-update.js | 83 ++++ .../workspaces-need-update/a/package.json | 6 + .../workspaces-need-update/b/package.json | 5 + .../workspaces-need-update/package-lock.json | 53 +++ .../workspaces-need-update/package.json | 6 + 15 files changed, 756 insertions(+), 208 deletions(-) create mode 100644 workspaces/arborist/test/fixtures/reify-cases/workspaces-need-update.js create mode 100644 workspaces/arborist/test/fixtures/workspaces-need-update/a/package.json create mode 100644 workspaces/arborist/test/fixtures/workspaces-need-update/b/package.json create mode 100644 workspaces/arborist/test/fixtures/workspaces-need-update/package-lock.json create mode 100644 workspaces/arborist/test/fixtures/workspaces-need-update/package.json diff --git a/docs/content/using-npm/config.md b/docs/content/using-npm/config.md index 4cb9bdb0fcce3..83a385e08344f 100644 --- a/docs/content/using-npm/config.md +++ b/docs/content/using-npm/config.md @@ -1326,16 +1326,16 @@ The base URL of the npm registry. #### `save` -* Default: true unless when using `npm update` or `npm dedupe` where it +* Default: `true` unless when using `npm update` or `npm dedupe` where it defaults to `false` * Type: Boolean -Save installed packages to a package.json file as dependencies. +Save installed packages to a `package.json` file as dependencies. When used with the `npm rm` command, removes the dependency from -package.json. +`package.json`. -Will also prevent writing to package-lock.json if set to `false`. +Will also prevent writing to `package-lock.json` if set to `false`. diff --git a/lib/commands/update.js b/lib/commands/update.js index a5374d0ae41ca..c55d7cf575d88 100644 --- a/lib/commands/update.js +++ b/lib/commands/update.js @@ -41,7 +41,7 @@ class Update extends ArboristWorkspaceCmd { ? global : this.npm.prefix - // In the context of `npm upgrade` the save + // In the context of `npm update` the save // config value should default to `false` const save = this.npm.config.isDefault('save') ? false diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js index 111ebd7898e67..95d79f0f05325 100644 --- a/lib/utils/config/definitions.js +++ b/lib/utils/config/definitions.js @@ -1583,18 +1583,18 @@ define('registry', { define('save', { default: true, - defaultDescription: `true unless when using \`npm update\` or + defaultDescription: `\`true\` unless when using \`npm update\` or \`npm dedupe\` where it defaults to \`false\``, usage: '-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer', type: Boolean, short: 'S', description: ` - Save installed packages to a package.json file as dependencies. + Save installed packages to a \`package.json\` file as dependencies. When used with the \`npm rm\` command, removes the dependency from - package.json. + \`package.json\`. - Will also prevent writing to package-lock.json if set to \`false\`. + Will also prevent writing to \`package-lock.json\` if set to \`false\`. `, flatten, }) diff --git a/smoke-tests/index.js b/smoke-tests/index.js index 5936bd8702474..080209d73a132 100644 --- a/smoke-tests/index.js +++ b/smoke-tests/index.js @@ -268,14 +268,55 @@ t.test('npm pkg', async t => { ) }) +t.test('npm update --no-save --no-package-lock', async t => { + // setup, manually reset dep value + await exec(`${npmBin} pkg set dependencies.abbrev==1.0.4`) + await exec(`${npmBin} install`) + await exec(`${npmBin} pkg set dependencies.abbrev=^1.0.4`) + + const cmd = `${npmBin} update --no-save --no-package-lock` + await exec(cmd) + + t.equal( + JSON.parse(readFile('package.json')).dependencies.abbrev, + '^1.0.4', + 'should have expected update --no-save --no-package-lock package.json result' + ) + t.matchSnapshot( + JSON.parse(readFile('package-lock.json')).packages['node_modules/abbrev'], + '1.0.4', + 'should have expected update --no-save --no-package-lock lockfile result' + ) +}) + +t.test('npm update --no-save', async t => { + const cmd = `${npmBin} update --no-save` + await exec(cmd) + + t.matchSnapshot( + JSON.parse(readFile('package.json')).dependencies.abbrev, + '^1.0.4', + 'should have expected update --no-save package.json result' + ) + t.matchSnapshot( + JSON.parse(readFile('package-lock.json')).packages['node_modules/abbrev'], + '1.1.1', + 'should have expected update --no-save lockfile result' + ) +}) + t.test('npm update --save', async t => { const cmd = `${npmBin} update --save` - const cmdRes = await exec(cmd) + await exec(cmd) - t.matchSnapshot(cmdRes.replace(/in.*s/, ''), - 'should have expected update --save reify output') - t.matchSnapshot(readFile('package.json'), - 'should have expected update --save package.json result') - t.matchSnapshot(readFile('package-lock.json'), - 'should have expected update --save lockfile result') + t.matchSnapshot( + JSON.parse(readFile('package.json')).dependencies.abbrev, + '^1.1.1', + 'should have expected update --save package.json result' + ) + t.matchSnapshot( + JSON.parse(readFile('package-lock.json')).packages['node_modules/abbrev'], + '1.1.1', + 'should have expected update --save lockfile result' + ) }) diff --git a/tap-snapshots/smoke-tests/index.js.test.cjs b/tap-snapshots/smoke-tests/index.js.test.cjs index b25f9da94e935..e76eb4bdb4f93 100644 --- a/tap-snapshots/smoke-tests/index.js.test.cjs +++ b/tap-snapshots/smoke-tests/index.js.test.cjs @@ -666,62 +666,36 @@ removed 1 package ` -exports[`smoke-tests/index.js TAP npm update --save > should have expected update --save lockfile result 1`] = ` -{ - "name": "project", - "version": "1.0.0", - "lockfileVersion": 2, - "requires": true, - "packages": { - "": { - "name": "project", - "version": "1.0.0", - "license": "ISC", - "dependencies": { - "abbrev": "^1.1.1" - } - }, - "node_modules/abbrev": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", - "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" - } - }, - "dependencies": { - "abbrev": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", - "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" - } - } +exports[`smoke-tests/index.js TAP npm update --no-save --no-package-lock > 1.0.4 1`] = ` +Object { + "integrity": "sha1-vVWuXkE7oXIu5Mq6H26hBBSlns0=", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.0.4.tgz", + "version": "1.0.4", } - ` -exports[`smoke-tests/index.js TAP npm update --save > should have expected update --save package.json result 1`] = ` -{ - "name": "project", - "version": "1.0.0", - "description": "", - "main": "index.js", - "scripts": { - "test": "echo /"Error: no test specified/" && exit 1", - "hello": "echo Hello" - }, - "keywords": [], - "author": "", - "license": "ISC", - "dependencies": { - "abbrev": "^1.1.1" - } +exports[`smoke-tests/index.js TAP npm update --no-save > 1.1.1 1`] = ` +Object { + "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "version": "1.1.1", } - ` -exports[`smoke-tests/index.js TAP npm update --save > should have expected update --save reify output 1`] = ` +exports[`smoke-tests/index.js TAP npm update --no-save > ^1.0.4 1`] = ` +^1.0.4 +` -up to date +exports[`smoke-tests/index.js TAP npm update --save > 1.1.1 1`] = ` +Object { + "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "version": "1.1.1", +} +` +exports[`smoke-tests/index.js TAP npm update --save > ^1.1.1 1`] = ` +^1.1.1 ` exports[`smoke-tests/index.js TAP npm update dep > should have expected update lockfile result 1`] = ` diff --git a/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs b/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs index 1ee8400d7ae85..459c5de8dc284 100644 --- a/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs +++ b/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs @@ -1406,16 +1406,16 @@ The base URL of the npm registry. exports[`test/lib/utils/config/definitions.js TAP > config description for save 1`] = ` #### \`save\` -* Default: true unless when using \`npm update\` or \`npm dedupe\` where it +* Default: \`true\` unless when using \`npm update\` or \`npm dedupe\` where it defaults to \`false\` * Type: Boolean -Save installed packages to a package.json file as dependencies. +Save installed packages to a \`package.json\` file as dependencies. When used with the \`npm rm\` command, removes the dependency from -package.json. +\`package.json\`. -Will also prevent writing to package-lock.json if set to \`false\`. +Will also prevent writing to \`package-lock.json\` if set to \`false\`. ` exports[`test/lib/utils/config/definitions.js TAP > config description for save-bundle 1`] = ` diff --git a/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs b/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs index 31fab28de6f48..ffa6617328bc6 100644 --- a/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs +++ b/tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs @@ -1200,16 +1200,16 @@ The base URL of the npm registry. #### \`save\` -* Default: true unless when using \`npm update\` or \`npm dedupe\` where it +* Default: \`true\` unless when using \`npm update\` or \`npm dedupe\` where it defaults to \`false\` * Type: Boolean -Save installed packages to a package.json file as dependencies. +Save installed packages to a \`package.json\` file as dependencies. When used with the \`npm rm\` command, removes the dependency from -package.json. +\`package.json\`. -Will also prevent writing to package-lock.json if set to \`false\`. +Will also prevent writing to \`package-lock.json\` if set to \`false\`. diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index ad8d8b5c09caf..d5e70323830b6 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -1142,21 +1142,25 @@ module.exports = cls => class Reifier extends cls { // for install failures. Those still end up in the shrinkwrap, so we // save it first, then prune out the optional trash, and then return it. - const skipSave = options.save === false - const save = !skipSave + const save = !(options.save === false) + + // we check for updates in order to make sure we run save ideal tree + // even though save=false since we want `npm update` to be able to + // write to package-lock files by default const hasUpdates = this[_updateAll] || this[_updateNames].length // we're going to completely skip save ideal tree in case of a global or // dry-run install and also if the save option is set to false, EXCEPT for // update since the expected behavior for npm7+ is for update to // NOT save to package.json, we make that exception since we still want - // saveIdealTree to be able to write the lockfile - const skipSaveIdealTree = - (skipSave && !hasUpdates) + // saveIdealTree to be able to write the lockfile by default. + const saveIdealTree = !( + (!save && !hasUpdates) || this[_global] || this[_dryRun] + ) - if (skipSaveIdealTree) { + if (!saveIdealTree) { return false } diff --git a/workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs b/workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs index 1f1bbafa373c6..6d38d724dca22 100644 --- a/workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs @@ -32676,28 +32676,416 @@ exports[`test/arborist/reify.js TAP save complete lockfile on update-all > shoul ` -exports[`test/arborist/reify.js TAP save package.json on update should update both package.json and package-lock.json using save=true > should update lockfile with same dep range from now updated package.json 1`] = ` +exports[`test/arborist/reify.js TAP save package.json on update should not save any with save=false and package-lock=false > should update lockfile with many deps updated package.json save=false 1`] = ` { - "name": "tap-testdir-reify-save-package.json-on-update-should-update-both-package.json-and-package-lock.json-using-save-true", + "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=" + } + } +} +` + +exports[`test/arborist/reify.js TAP save package.json on update should not save many deps in multiple package.json when using save=false > should update lockfile with many deps updated package.json save=false 1`] = ` +{ + "name": "tap-testdir-reify-save-package.json-on-update-should-not-save-many-deps-in-multiple-package.json-when-using-save-false", + "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.1.1", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" + }, + "node_modules/b": { + "resolved": "b", + "link": true + }, + "node_modules/once": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", + "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", + "dependencies": { + "wrappy": "1" + } + }, + "node_modules/wrappy": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" + } + }, + "dependencies": { + "a": { + "version": "file:a", + "requires": { + "abbrev": "^1.0.4", + "once": "^1.3.2" + } + }, + "abbrev": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" + }, + "b": { + "version": "file:b", + "requires": { + "abbrev": "^1.0.4" + } + }, + "once": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", + "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", + "requires": { + "wrappy": "1" + } + }, + "wrappy": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" + } + } +} + +` + +exports[`test/arborist/reify.js TAP save package.json on update should save many deps in multiple package.json when using save=true > should update lockfile with many deps updated package.json save=true 1`] = ` +{ + "name": "tap-testdir-reify-save-package.json-on-update-should-save-many-deps-in-multiple-package.json-when-using-save-true", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "workspaces": [ + "a", + "b" + ], + "dependencies": { + "abbrev": "^1.1.1" + } + }, + "a": { + "dependencies": { + "abbrev": "^1.1.1", + "once": "^1.4.0" + } + }, + "b": { + "dependencies": { + "abbrev": "^1.1.1" + } + }, + "node_modules/a": { + "resolved": "a", + "link": true + }, + "node_modules/abbrev": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" + }, + "node_modules/b": { + "resolved": "b", + "link": true + }, + "node_modules/once": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", + "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", + "dependencies": { + "wrappy": "1" + } + }, + "node_modules/wrappy": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" + } + }, + "dependencies": { + "a": { + "version": "file:a", + "requires": { + "abbrev": "^1.0.4", + "once": "^1.3.2" + } + }, + "abbrev": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" + }, + "b": { + "version": "file:b", + "requires": { + "abbrev": "^1.0.4" + } + }, + "once": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", + "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", + "requires": { + "wrappy": "1" + } + }, + "wrappy": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" + } + } +} + +` + +exports[`test/arborist/reify.js TAP save package.json on update should update named dep across multiple package.json using save=true > should update lockfile with many deps updated package.json save=true 1`] = ` +{ + "name": "tap-testdir-reify-save-package.json-on-update-should-update-named-dep-across-multiple-package.json-using-save-true", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "workspaces": [ + "a", + "b" + ], + "dependencies": { + "abbrev": "^1.1.1" + } + }, + "a": { + "dependencies": { + "abbrev": "^1.1.1", + "once": "^1.3.2" + } + }, + "b": { "dependencies": { "abbrev": "^1.1.1" } }, + "node_modules/a": { + "resolved": "a", + "link": true + }, "node_modules/abbrev": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" + }, + "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=" } }, "dependencies": { + "a": { + "version": "file:a", + "requires": { + "abbrev": "^1.0.4", + "once": "^1.3.2" + } + }, "abbrev": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" + }, + "b": { + "version": "file:b", + "requires": { + "abbrev": "^1.0.4" + } + }, + "once": { + "version": "1.3.2", + "resolved": "https://registry.npmjs.org/once/-/once-1.3.2.tgz", + "integrity": "sha1-2P7sqTsDnsHc3ud0HJK9rF4oCBs=", + "requires": { + "wrappy": "1" + } + }, + "wrappy": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" + } + } +} + +` + +exports[`test/arborist/reify.js TAP save package.json on update should update single named dep across multiple package.json using save=true > should update lockfile with single dep updated package.json save=true 1`] = ` +{ + "name": "tap-testdir-reify-save-package.json-on-update-should-update-single-named-dep-across-multiple-package.json-using-save-true", + "lockfileVersion": 2, + "requires": true, + "packages": { + "": { + "workspaces": [ + "a", + "b" + ], + "dependencies": { + "abbrev": "^1.0.4" + } + }, + "a": { + "dependencies": { + "abbrev": "^1.0.4", + "once": "^1.4.0" + } + }, + "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.4.0", + "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", + "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", + "dependencies": { + "wrappy": "1" + } + }, + "node_modules/wrappy": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" + } + }, + "dependencies": { + "a": { + "version": "file:a", + "requires": { + "abbrev": "^1.0.4", + "once": "^1.3.2" + } + }, + "abbrev": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.0.4.tgz", + "integrity": "sha1-vVWuXkE7oXIu5Mq6H26hBBSlns0=" + }, + "b": { + "version": "file:b", + "requires": { + "abbrev": "^1.0.4" + } + }, + "once": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", + "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", + "requires": { + "wrappy": "1" + } + }, + "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/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index c1052526ba89e..d5fc166a5636d 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/workspaces/arborist/test/arborist/reify.js @@ -2445,143 +2445,131 @@ t.test('add local dep with existing dev + peer/optional', async t => { }) t.test('save package.json on update', t => { - const pjson = { - dependencies: { - abbrev: '^1.0.9', - }, - } - const expectedUsingSaveOption = { - dependencies: { - abbrev: '^1.1.1', - }, - } - const plock = { - name: 'tap-testdir-reify-save-package.json-on-update-should-update-package-lock.json', - version: '1.0.0', - lockfileVersion: 2, - requires: true, - packages: { - '': { - name: 'a', - version: '1.0.0', - license: 'MIT', - dependencies: { - abbrev: '^1.0.9', - }, - }, - 'node_modules/abbrev': { - version: '1.0.9', - resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.0.9.tgz', - integrity: 'sha1-kbR5JYinc4wl813W9jdSovh3YTU=', - }, - }, - dependencies: { - abbrev: { - version: '1.0.9', - resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.0.9.tgz', - integrity: 'sha1-kbR5JYinc4wl813W9jdSovh3YTU=', - }, - }, - } - - t.test('should not touch package.json on update-all', async t => { - const path = t.testdir({ - 'package.json': JSON.stringify(pjson), - }) - - await reify(path, { update: true, save: false }) - - t.same(require(resolve(path, 'package.json')), pjson, - 'should not have changed package.json file on update all') - }) - - t.test('should update package-lock.json', async t => { - const path = t.testdir({ - 'package.json': JSON.stringify(pjson), - 'package-lock.json': JSON.stringify(plock), - }) - - await reify(path, { update: true, save: false }) - - t.same(require(resolve(path, 'package.json')), pjson, - 'should not have changed package.json file on update all') - - // in a package-lock the version gets updated even though the - // package.json dependency info hasn't been updated - const lock = require(resolve(path, 'package-lock.json')) - const lockedVersion = lock.packages['node_modules/abbrev'].version - t.equal(lockedVersion, '1.1.1', - 'should update locked versions on packages entries') - const depRange = lock.packages[''].dependencies.abbrev - t.equal(depRange, '^1.0.9', - 'should have same version range described in package.json') - }) - - t.test('should not touch package.json on named update', async t => { - const path = t.testdir({ - 'package.json': JSON.stringify(pjson), - }) - - await reify(path, { update: { names: ['abbrev'] }, save: false }) - - t.same(require(resolve(path, 'package.json')), pjson, - 'should not have changed package.json file on named update') - }) - - t.test('should save to package.json when using save=true', async t => { - const path = t.testdir({ - 'package.json': JSON.stringify(pjson), - }) + 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')), expectedUsingSaveOption, - 'should save pkg change to package.json file on update all') - }) - - t.test('should save to package.json on named update using save=true', async t => { - const path = t.testdir({ - 'package.json': JSON.stringify(pjson), - }) - - await reify(path, { update: { names: ['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.4.0' } }, + 'should save workspace dep to its package.json file') - t.same(require(resolve(path, 'package.json')), expectedUsingSaveOption, - 'should save pkg change to package.json file on named update') + 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 both package.json and package-lock.json using save=true', async t => { - const path = t.testdir({ - 'package.json': JSON.stringify(pjson), - 'package-lock.json': JSON.stringify(plock), - }) + 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: true }) - - t.same(require(resolve(path, 'package.json')), expectedUsingSaveOption, - 'should change package.json file on update all along with lockfile') - - t.matchSnapshot(fs.readFileSync(resolve(path, 'package-lock.json'), 'utf8'), - 'should update lockfile with same dep range from now updated package.json') - }) + await reify(path, { update: true, save: false }) - t.test('should save to multiple package.json when using save=true', async t => { - const path = t.testdir({ - 'package.json': JSON.stringify({ - ...pjson, - workspaces: ['a'], - }), - a: { - 'package.json': JSON.stringify(pjson), + t.same( + require(resolve(path, 'package.json')), + { + dependencies: { abbrev: '^1.0.4' }, + workspaces: ['a', 'b'], }, - }) - - await reify(path, { update: true, save: true }) - - t.match(require(resolve(path, 'package.json')), expectedUsingSaveOption, - 'should save pkg change to all package.json files on update all') - t.same(require(resolve(path, 'a', 'package.json')), expectedUsingSaveOption, - 'should save workspace pkg change to all package.json files on update all') + '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 0000000000000..2d57f05a0504d --- /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 0000000000000..76584743763c3 --- /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 0000000000000..8af6070f2f798 --- /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 0000000000000..43865795a7bba --- /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 0000000000000..ebdce120ce0c6 --- /dev/null +++ b/workspaces/arborist/test/fixtures/workspaces-need-update/package.json @@ -0,0 +1,6 @@ +{ + "dependencies": { + "abbrev": "^1.0.4" + }, + "workspaces": ["a", "b"] +}