fix: clean up npm object (#7416)

Removed all the setters, they were dangerous and not the right way to
set those values.

Tweaked `this.#init` so it always returns an object so the coerce in
`this.load` didn't need to happen.

Author: wraithgar

Diff for: lib/npm.js

@@ -94,7 +94,7 @@ class Npm {
 
   async load () {
     return time.start('npm:load', async () => {
-      const { exec = true } = await this.#load().then(r => r ?? {})
+      const { exec } = await this.#load()
       return {
         exec,
         command: this.argv.shift(),
@@ -130,11 +130,12 @@ class Npm {
         output.error('')
       }
 
-      const logsMax = this.config.get('logs-max')
-
       if (this.logFiles.length) {
-        log.error('', `A complete log of this run can be found in: ${this.logFiles}`)
-      } else if (logsMax <= 0) {
+        return log.error('', `A complete log of this run can be found in: ${this.logFiles}`)
+      }
+
+      const logsMax = this.config.get('logs-max')
+      if (logsMax <= 0) {
         // user specified no log file
         log.error('', `Log files were not written due to the config logs-max=${logsMax}`)
       } else {
@@ -151,11 +152,6 @@ class Npm {
     return this.#title
   }
 
-  set title (t) {
-    process.title = t
-    this.#title = t
-  }
-
   async #load () {
     await time.start('npm:load:whichnode', async () => {
       // TODO should we throw here?
@@ -210,8 +206,9 @@ class Npm {
       const { parsedArgv: { cooked, remain } } = this.config
       this.argv = remain
       // Secrets are mostly in configs, so title is set using only the positional args
-      // to keep those from being leaked.
-      this.title = ['npm'].concat(replaceInfo(remain)).join(' ').trim()
+      // to keep those from being leaked.  We still do a best effort replaceInfo.
+      this.#title = ['npm'].concat(replaceInfo(remain)).join(' ').trim()
+      process.title = this.#title
       // The cooked argv is also logged separately for debugging purposes. It is
       // cleaned as a best effort by replacing known secrets like basic auth
       // password and strings that look like npm tokens. XXX: for this to be
@@ -252,6 +249,8 @@ class Npm {
       this.argv = ['version']
       this.config.set('usage', false, 'cli')
     }
+
+    return { exec: true }
   }
 
   get isShellout () {
@@ -331,26 +330,14 @@ class Npm {
     return this.config.get('cache')
   }
 
-  set cache (r) {
-    this.config.set('cache', r)
-  }
-
   get globalPrefix () {
     return this.config.globalPrefix
   }
 
-  set globalPrefix (r) {
-    this.config.globalPrefix = r
-  }
-
   get localPrefix () {
     return this.config.localPrefix
   }
 
-  set localPrefix (r) {
-    this.config.localPrefix = r
-  }
-
   get localPackage () {
     return this.config.localPackage
   }
@@ -386,11 +373,6 @@ class Npm {
     return this.global ? this.globalPrefix : this.localPrefix
   }
 
-  set prefix (r) {
-    const k = this.global ? 'globalPrefix' : 'localPrefix'
-    this[k] = r
-  }
-
   get usage () {
     return usage(this)
   }

Diff for: test/lib/arborist-cmd.js

@@ -117,7 +117,8 @@ t.test('arborist-cmd', async t => {
       chdir: (dirs) => dirs.testdir,
     })
 
-    npm.localPrefix = prefix
+    // TODO there has to be a better way to do this
+    npm.config.localPrefix = prefix
     await cmd.execWorkspaces([])
 
     t.same(cmd.workspaceNames, ['a', 'c'], 'should set array with single ws name')

Diff for: test/lib/commands/exec.js

@@ -76,7 +76,7 @@ t.test('--prefix', async t => {
   })
 
   // This is what `--prefix` does
-  npm.globalPrefix = npm.localPrefix
+  npm.config.globalPrefix = npm.config.localPrefix
 
   await registry.package({
     manifest,

Diff for: test/lib/npm.js

@@ -36,11 +36,8 @@ t.test('npm.load', async t => {
   })
 
   await t.test('basic loading', async t => {
-    const { npm, logs, prefix: dir, cache, other } = await loadMockNpm(t, {
+    const { npm, logs, cache } = await loadMockNpm(t, {
       prefixDir: { node_modules: {} },
-      otherDirs: {
-        newCache: {},
-      },
       config: {
         timing: true,
       },
@@ -61,25 +58,11 @@ t.test('npm.load', async t => {
 
     mockGlobals(t, { process: { platform: 'posix' } })
     t.equal(resolve(npm.cache), resolve(cache), 'cache is cache')
-    npm.cache = other.newCache
-    t.equal(npm.config.get('cache'), other.newCache, 'cache setter sets config')
-    t.equal(npm.cache, other.newCache, 'cache getter gets new config')
     t.equal(npm.lockfileVersion, 2, 'lockfileVersion getter')
     t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix')
     t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix')
-    npm.globalPrefix = npm.prefix
-    t.equal(npm.prefix, npm.globalPrefix, 'globalPrefix setter')
-    npm.localPrefix = dir + '/extra/prefix'
-    t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix after localPrefix setter')
-    t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix after localPrefix setter')
-
-    npm.prefix = dir + '/some/prefix'
-    t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix after prefix setter')
-    t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix after prefix setter')
-    t.equal(npm.bin, npm.localBin, 'bin is local bin after prefix setter')
-    t.not(npm.bin, npm.globalBin, 'bin is not global bin after prefix setter')
-    t.equal(npm.dir, npm.localDir, 'dir is local dir after prefix setter')
-    t.not(npm.dir, npm.globalDir, 'dir is not global dir after prefix setter')
+    t.equal(npm.bin, npm.localBin, 'bin is local bin')
+    t.not(npm.bin, npm.globalBin, 'bin is not global bin')
 
     npm.config.set('global', true)
     t.equal(npm.prefix, npm.globalPrefix, 'prefix is global prefix after setting global')
@@ -89,12 +72,6 @@ t.test('npm.load', async t => {
     t.equal(npm.dir, npm.globalDir, 'dir is global dir after setting global')
     t.not(npm.dir, npm.localDir, 'dir is not local dir after setting global')
 
-    npm.prefix = dir + '/new/global/prefix'
-    t.equal(npm.prefix, npm.globalPrefix, 'prefix is global prefix after prefix setter')
-    t.not(npm.prefix, npm.localPrefix, 'prefix is not local prefix after prefix setter')
-    t.equal(npm.bin, npm.globalBin, 'bin is global bin after prefix setter')
-    t.not(npm.bin, npm.localBin, 'bin is not local bin after prefix setter')
-
     mockGlobals(t, { process: { platform: 'win32' } })
     t.equal(npm.bin, npm.globalBin, 'bin is global bin in windows mode')
     t.equal(npm.dir, npm.globalDir, 'dir is global dir in windows mode')