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

4 files changed, 22 insertions, 10 deletions

diff --git a/tap-snapshots/test/lib/load-all-commands.js.test.cjs b/tap-snapshots/test/lib/load-all-commands.js.test.cjs
index 6efecf208..e7142c249 100644
--- a/tap-snapshots/test/lib/load-all-commands.js.test.cjs
+++ b/tap-snapshots/test/lib/load-all-commands.js.test.cjs
@@ -170,6 +170,7 @@ npm dedupe
 
 Options:
 [--global-style] [--legacy-bundling] [--strict-peer-deps] [--no-package-lock]
+[-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer]
 [--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]] [--ignore-scripts]
 [--no-audit] [--no-bin-links] [--no-fund] [--dry-run]
 [-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
@@ -1061,8 +1062,10 @@ npm update [<pkg>...]
 
 Options:
 [-g|--global] [--global-style] [--legacy-bundling] [--strict-peer-deps]
-[--no-package-lock] [--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]]
-[--ignore-scripts] [--no-audit] [--no-bin-links] [--no-fund] [--dry-run]
+[--no-package-lock]
+[-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer]
+[--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]] [--ignore-scripts]
+[--no-audit] [--no-bin-links] [--no-fund] [--dry-run]
 [-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
 [-ws|--workspaces] [--include-workspace-root]
 
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 84bb22ff0..459c5de8d 100644
--- a/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs
+++ b/tap-snapshots/test/lib/utils/config/definitions.js.test.cjs
@@ -1406,13 +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
+* 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\`.
 `
 
 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 3db90f767..ffa661732 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,13 +1200,16 @@ The base URL of the npm registry.
 
 #### \`save\`
 
-* Default: true
+* 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\`.
 
 <!-- automatically generated, do not edit manually -->
 <!-- see lib/utils/config/definitions.js -->
diff --git a/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs b/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs
index 0e48cfa61..ab8f1cf3b 100644
--- a/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs
+++ b/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs
@@ -311,6 +311,7 @@ All commands:
                     
                     Options:
                     [--global-style] [--legacy-bundling] [--strict-peer-deps] [--no-package-lock]
+                    [-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer]
                     [--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]] [--ignore-scripts]
                     [--no-audit] [--no-bin-links] [--no-fund] [--dry-run]
                     [-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
@@ -1096,8 +1097,10 @@ All commands:
                     
                     Options:
                     [-g|--global] [--global-style] [--legacy-bundling] [--strict-peer-deps]
-                    [--no-package-lock] [--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]]
-                    [--ignore-scripts] [--no-audit] [--no-bin-links] [--no-fund] [--dry-run]
+                    [--no-package-lock]
+                    [-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer]
+                    [--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]] [--ignore-scripts]
+                    [--no-audit] [--no-bin-links] [--no-fund] [--dry-run]
                     [-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
                     [-ws|--workspaces] [--include-workspace-root]