fix: npm update save=false by default

npm · Jan 12, 2022 · 3cf432a · 3cf432a
1 parent f21ffba
commit 3cf432a
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 8 deletions.
diff --git a/lib/commands/update.js b/lib/commands/update.js
@@ -40,19 +40,27 @@ class Update extends ArboristWorkspaceCmd {
       ? global
       : this.npm.prefix
 
+    // In the context of `npm upgrade` the save
+    // config value should default to `false`
+    const save = this.npm.config.isDefault('save')
+      ? false
+      : this.npm.config.get('save')
+
     if (this.npm.config.get('depth')) {
       log.warn('update', 'The --depth option no longer has any effect. See RFC0019.\n' +
         'https://github.com/npm/rfcs/blob/latest/implemented/0019-remove-update-depth-option.md')
     }
 
-    const arb = new Arborist({
+    const opts = {
       ...this.npm.flatOptions,
       log,
       path: where,
+      save,
       workspaces: this.workspaceNames,
-    })
+    }
+    const arb = new Arborist(opts)
 
-    await arb.reify({ update })
+    await arb.reify({ ...opts, update })
     await reifyFinish(this.npm, arb)
   }
 }

diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js
@@ -147,6 +147,8 @@ class MockNpm {
       // for now just set `find` to what config.find should return
       // this works cause `find` is not an existing config entry
       find: (k) => ({ ...realConfig.defaults, ...config })[k],
+      // for now isDefault is going to just return false if a value was defined
+      isDefault: (k) => !Object.prototype.hasOwnProperty.call(config, k),
       get: (k) => ({ ...realConfig.defaults, ...config })[k],
       set: (k, v) => config[k] = v,
       list: [{ ...realConfig.defaults, ...config }],

diff --git a/test/lib/commands/update.js b/test/lib/commands/update.js
@@ -27,7 +27,7 @@ t.afterEach(() => {
 })
 
 t.test('no args', async t => {
-  t.plan(4)
+  t.plan(5)
 
   npm.prefix = '/project/a'
 
@@ -39,14 +39,16 @@ t.test('no args', async t => {
         {
           ...npm.flatOptions,
           path: npm.prefix,
+          save: false,
           workspaces: null,
         },
         'should call arborist contructor with expected args'
       )
       t.match(log, {}, 'log is passed in')
     }
 
-    reify ({ update }) {
+    reify ({ save, update }) {
+      t.equal(save, false, 'should default to save=false')
       t.equal(update, true, 'should update all deps')
     }
   }
@@ -64,9 +66,10 @@ t.test('no args', async t => {
 })
 
 t.test('with args', async t => {
-  t.plan(4)
+  t.plan(5)
 
   npm.prefix = '/project/a'
+  config.save = true
 
   class Arborist {
     constructor (args) {
@@ -76,14 +79,16 @@ t.test('with args', async t => {
         {
           ...npm.flatOptions,
           path: npm.prefix,
+          save: true,
           workspaces: null,
         },
         'should call arborist contructor with expected args'
       )
       t.match(log, {}, 'log is passed in')
     }
 
-    reify ({ update }) {
+    reify ({ save, update }) {
+      t.equal(save, true, 'should pass save if manually set')
       t.same(update, ['ipt'], 'should update listed deps')
     }
   }
@@ -140,7 +145,7 @@ t.test('update --global', async t => {
       const { path, log, ...rest } = args
       t.same(
         rest,
-        { ...npm.flatOptions, workspaces: undefined },
+        { ...npm.flatOptions, save: true, workspaces: undefined },
         'should call arborist contructor with expected options'
       )