Skip to content

Commit cfd59b8

Browse files
ruyadornoruyadorno
authored
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: #708 Fixes: #2704 Relates to: npm/feedback#270
1 parent 510f0ec commit cfd59b8

24 files changed

Lines changed: 1178 additions & 81 deletions

File tree

‎docs/content/commands/npm-dedupe.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ result in new modules being installed.
7272

7373
Using `npm find-dupes` will run the command in `--dry-run` mode.
7474

75+
Note that by default `npm dedupe` will not update the semver values of direct
76+
dependencies in your project `package.json`, if you want to also update
77+
values in `package.json` you can run: `npm dedupe --save` (or add the
78+
`save=true` option to a [configuration file](/configuring-npm/npmrc)
79+
to make that the default behavior).
80+
7581
### Configuration
7682

7783
<!-- AUTOGENERATED CONFIG DESCRIPTIONS START -->

‎docs/content/commands/npm-update.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ packages.
2727
If no package name is specified, all packages in the specified location (global
2828
or local) will be updated.
2929

30+
Note that by default `npm update` will not update the semver values of direct
31+
dependencies in your project `package.json`, if you want to also update
32+
values in `package.json` you can run: `npm update --save` (or add the
33+
`save=true` option to a [configuration file](/configuring-npm/npmrc)
34+
to make that the default behavior).
35+
3036
### Example
3137

3238
For the examples below, assume that the current package is `app` and it depends

‎docs/content/using-npm/config.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,13 +1326,16 @@ The base URL of the npm registry.
13261326

13271327
#### `save`
13281328

1329-
* Default: true
1329+
* Default: `true` unless when using `npm update` or `npm dedupe` where it
1330+
defaults to `false`
13301331
* Type: Boolean
13311332

1332-
Save installed packages to a package.json file as dependencies.
1333+
Save installed packages to a `package.json` file as dependencies.
13331334

13341335
When used with the `npm rm` command, removes the dependency from
1335-
package.json.
1336+
`package.json`.
1337+
1338+
Will also prevent writing to `package-lock.json` if set to `false`.
13361339

13371340
<!-- automatically generated, do not edit manually -->
13381341
<!-- see lib/utils/config/definitions.js -->

‎lib/commands/dedupe.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class Dedupe extends ArboristWorkspaceCmd {
1313
'legacy-bundling',
1414
'strict-peer-deps',
1515
'package-lock',
16+
'save',
1617
'omit',
1718
'ignore-scripts',
1819
'audit',
@@ -29,13 +30,20 @@ class Dedupe extends ArboristWorkspaceCmd {
2930
throw er
3031
}
3132

33+
// In the context of `npm dedupe` the save
34+
// config value should default to `false`
35+
const save = this.npm.config.isDefault('save')
36+
? false
37+
: this.npm.config.get('save')
38+
3239
const dryRun = this.npm.config.get('dry-run')
3340
const where = this.npm.prefix
3441
const opts = {
3542
...this.npm.flatOptions,
3643
log,
3744
path: where,
3845
dryRun,
46+
save,
3947
workspaces: this.workspaceNames,
4048
}
4149
const arb = new Arborist(opts)

‎lib/commands/update.js‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ class Update extends ArboristWorkspaceCmd {
1616
'legacy-bundling',
1717
'strict-peer-deps',
1818
'package-lock',
19+
'save',
1920
'omit',
2021
'ignore-scripts',
2122
'audit',
@@ -40,19 +41,27 @@ class Update extends ArboristWorkspaceCmd {
4041
? global
4142
: this.npm.prefix
4243

44+
// In the context of `npm update` the save
45+
// config value should default to `false`
46+
const save = this.npm.config.isDefault('save')
47+
? false
48+
: this.npm.config.get('save')
49+
4350
if (this.npm.config.get('depth')) {
4451
log.warn('update', 'The --depth option no longer has any effect. See RFC0019.\n' +
4552
'https://github.com/npm/rfcs/blob/latest/implemented/0019-remove-update-depth-option.md')
4653
}
4754

48-
const arb = new Arborist({
55+
const opts = {
4956
...this.npm.flatOptions,
5057
log,
5158
path: where,
59+
save,
5260
workspaces: this.workspaceNames,
53-
})
61+
}
62+
const arb = new Arborist(opts)
5463

55-
await arb.reify({ update })
64+
await arb.reify({ ...opts, update })
5665
await reifyFinish(this.npm, arb)
5766
}
5867
}

‎lib/utils/config/definitions.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1583,14 +1583,18 @@ define('registry', {
15831583

15841584
define('save', {
15851585
default: true,
1586+
defaultDescription: `\`true\` unless when using \`npm update\` or
1587+
\`npm dedupe\` where it defaults to \`false\``,
15861588
usage: '-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer',
15871589
type: Boolean,
15881590
short: 'S',
15891591
description: `
1590-
Save installed packages to a package.json file as dependencies.
1592+
Save installed packages to a \`package.json\` file as dependencies.
15911593
15921594
When used with the \`npm rm\` command, removes the dependency from
1593-
package.json.
1595+
\`package.json\`.
1596+
1597+
Will also prevent writing to \`package-lock.json\` if set to \`false\`.
15941598
`,
15951599
flatten,
15961600
})

‎smoke-tests/index.js‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,3 +267,56 @@ t.test('npm pkg', async t => {
267267
'should have expected npm pkg delete modified package.json result'
268268
)
269269
})
270+
271+
t.test('npm update --no-save --no-package-lock', async t => {
272+
// setup, manually reset dep value
273+
await exec(`${npmBin} pkg set "dependencies.abbrev==1.0.4"`)
274+
await exec(`${npmBin} install`)
275+
await exec(`${npmBin} pkg set "dependencies.abbrev=^1.0.4"`)
276+
277+
const cmd = `${npmBin} update --no-save --no-package-lock`
278+
await exec(cmd)
279+
280+
t.equal(
281+
JSON.parse(readFile('package.json')).dependencies.abbrev,
282+
'^1.0.4',
283+
'should have expected update --no-save --no-package-lock package.json result'
284+
)
285+
t.equal(
286+
JSON.parse(readFile('package-lock.json')).packages['node_modules/abbrev'].version,
287+
'1.0.4',
288+
'should have expected update --no-save --no-package-lock lockfile result'
289+
)
290+
})
291+
292+
t.test('npm update --no-save', async t => {
293+
const cmd = `${npmBin} update --no-save`
294+
await exec(cmd)
295+
296+
t.equal(
297+
JSON.parse(readFile('package.json')).dependencies.abbrev,
298+
'^1.0.4',
299+
'should have expected update --no-save package.json result'
300+
)
301+
t.equal(
302+
JSON.parse(readFile('package-lock.json')).packages['node_modules/abbrev'].version,
303+
'1.1.1',
304+
'should have expected update --no-save lockfile result'
305+
)
306+
})
307+
308+
t.test('npm update --save', async t => {
309+
const cmd = `${npmBin} update --save`
310+
await exec(cmd)
311+
312+
t.equal(
313+
JSON.parse(readFile('package.json')).dependencies.abbrev,
314+
'^1.1.1',
315+
'should have expected update --save package.json result'
316+
)
317+
t.equal(
318+
JSON.parse(readFile('package-lock.json')).packages['node_modules/abbrev'].version,
319+
'1.1.1',
320+
'should have expected update --save lockfile result'
321+
)
322+
})

‎tap-snapshots/test/lib/load-all-commands.js.test.cjs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ npm dedupe
170170
171171
Options:
172172
[--global-style] [--legacy-bundling] [--strict-peer-deps] [--no-package-lock]
173+
[-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer]
173174
[--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]] [--ignore-scripts]
174175
[--no-audit] [--no-bin-links] [--no-fund] [--dry-run]
175176
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
@@ -1061,8 +1062,10 @@ npm update [<pkg>...]
10611062
10621063
Options:
10631064
[-g|--global] [--global-style] [--legacy-bundling] [--strict-peer-deps]
1064-
[--no-package-lock] [--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]]
1065-
[--ignore-scripts] [--no-audit] [--no-bin-links] [--no-fund] [--dry-run]
1065+
[--no-package-lock]
1066+
[-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer]
1067+
[--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]] [--ignore-scripts]
1068+
[--no-audit] [--no-bin-links] [--no-fund] [--dry-run]
10661069
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
10671070
[-ws|--workspaces] [--include-workspace-root]
10681071

‎tap-snapshots/test/lib/utils/config/definitions.js.test.cjs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,13 +1406,16 @@ The base URL of the npm registry.
14061406
exports[`test/lib/utils/config/definitions.js TAP > config description for save 1`] = `
14071407
#### \`save\`
14081408
1409-
* Default: true
1409+
* Default: \`true\` unless when using \`npm update\` or \`npm dedupe\` where it
1410+
defaults to \`false\`
14101411
* Type: Boolean
14111412
1412-
Save installed packages to a package.json file as dependencies.
1413+
Save installed packages to a \`package.json\` file as dependencies.
14131414
14141415
When used with the \`npm rm\` command, removes the dependency from
1415-
package.json.
1416+
\`package.json\`.
1417+
1418+
Will also prevent writing to \`package-lock.json\` if set to \`false\`.
14161419
`
14171420

14181421
exports[`test/lib/utils/config/definitions.js TAP > config description for save-bundle 1`] = `

‎tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,13 +1200,16 @@ The base URL of the npm registry.
12001200
12011201
#### \`save\`
12021202
1203-
* Default: true
1203+
* Default: \`true\` unless when using \`npm update\` or \`npm dedupe\` where it
1204+
defaults to \`false\`
12041205
* Type: Boolean
12051206
1206-
Save installed packages to a package.json file as dependencies.
1207+
Save installed packages to a \`package.json\` file as dependencies.
12071208
12081209
When used with the \`npm rm\` command, removes the dependency from
1209-
package.json.
1210+
\`package.json\`.
1211+
1212+
Will also prevent writing to \`package-lock.json\` if set to \`false\`.
12101213
12111214
<!-- automatically generated, do not edit manually -->
12121215
<!-- see lib/utils/config/definitions.js -->

0 commit comments

Comments
 (0)