wip: Add workspaces functionality to reify commands

Not 100% confident in this approach, but seems like it works, and
keeps the duplication within a reasonable level.

One big downside is that now we have two base classes.

Author: isaacs

lib/audit.js

@@ -2,9 +2,9 @@ const Arborist = require('@npmcli/arborist')
 const auditReport = require('npm-audit-report')
 const reifyFinish = require('./utils/reify-finish.js')
 const auditError = require('./utils/audit-error.js')
-const BaseCommand = require('./base-command.js')
+const BaseReifyCommand = require('./base-reify-command.js')
 
-class Audit extends BaseCommand {
+class Audit extends BaseReifyCommand {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
   static get description () {
     return 'Run a security audit'
@@ -57,7 +57,9 @@ class Audit extends BaseCommand {
       audit: true,
       path: this.npm.prefix,
       reporter,
+      workspaces: this.workspaces,
     }
+
     const arb = new Arborist(opts)
     const fix = args[0] === 'fix'
     await arb.audit({ fix })

lib/base-command.js

@@ -6,6 +6,7 @@ class BaseCommand {
   constructor (npm) {
     this.wrapWidth = 80
     this.npm = npm
+    this.workspaces = null
   }
 
   get name () {

lib/base-reify-command.js

@@ -0,0 +1,17 @@
+// This is the base for all commands whose execWorkspaces just gets
+// a list of workspace names and passes it on to new Arborist() to
+// be able to run a filtered Arborist.reify() at some point.
+
+const BaseCommand = require('./base-command.js')
+const getWorkspaces = require('./workspaces/get-workspaces.js')
+class BaseReifyCommand extends BaseCommand {
+  execWorkspaces (args, filters, cb) {
+    getWorkspaces(filters, { path: this.npm.localPrefix })
+      .then(workspaces => {
+        this.workspaces = workspaces.map(([name]) => name)
+        this.exec(args, cb)
+      })
+  }
+}
+
+module.exports = BaseReifyCommand

lib/ci.js

@@ -17,9 +17,9 @@ const removeNodeModules = async where => {
   await Promise.all(entries.map(f => rimraf(`${path}/${f}`, rimrafOpts)))
   process.emit('timeEnd', 'npm-ci:rm')
 }
-const BaseCommand = require('./base-command.js')
+const BaseReifyCommand = require('./base-reify-command.js')
 
-class CI extends BaseCommand {
+class CI extends BaseReifyCommand {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
   static get description () {
     return 'Install a project with a clean slate'
@@ -47,6 +47,7 @@ class CI extends BaseCommand {
       path: where,
       log: this.npm.log,
       save: false, // npm ci should never modify the lockfile or package.json
+      workspaces: this.workspaces,
     }
 
     const arb = new Arborist(opts)

lib/dedupe.js

@@ -2,9 +2,9 @@
 const Arborist = require('@npmcli/arborist')
 const reifyFinish = require('./utils/reify-finish.js')
 
-const BaseCommand = require('./base-command.js')
+const BaseReifyCommand = require('./base-reify-command.js')
 
-class Dedupe extends BaseCommand {
+class Dedupe extends BaseReifyCommand {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
   static get description () {
     return 'Reduce duplication in the package tree'
@@ -33,6 +33,7 @@ class Dedupe extends BaseCommand {
       log: this.npm.log,
       path: where,
       dryRun,
+      workspaces: this.workspaces,
     }
     const arb = new Arborist(opts)
     await arb.dedupe(opts)

lib/install.js

@@ -9,8 +9,8 @@ const { resolve, join } = require('path')
 const Arborist = require('@npmcli/arborist')
 const runScript = require('@npmcli/run-script')
 
-const BaseCommand = require('./base-command.js')
-class Install extends BaseCommand {
+const BaseReifyCommand = require('./base-reify-command.js')
+class Install extends BaseReifyCommand {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
   static get description () {
     return 'Install a package'
@@ -132,6 +132,7 @@ class Install extends BaseCommand {
       auditLevel: null,
       path: where,
       add: args,
+      workspaces: this.workspaces,
     }
     const arb = new Arborist(opts)
     await arb.reify(opts)

lib/prune.js

@@ -2,8 +2,8 @@
 const Arborist = require('@npmcli/arborist')
 const reifyFinish = require('./utils/reify-finish.js')
 
-const BaseCommand = require('./base-command.js')
-class Prune extends BaseCommand {
+const BaseReifyCommand = require('./base-reify-command.js')
+class Prune extends BaseReifyCommand {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
   static get description () {
     return 'Remove extraneous packages'
@@ -34,6 +34,7 @@ class Prune extends BaseCommand {
       ...this.npm.flatOptions,
       path: where,
       log: this.npm.log,
+      workspaces: this.workspaces,
     }
     const arb = new Arborist(opts)
     await arb.prune(opts)

lib/rebuild.js

@@ -4,6 +4,8 @@ const npa = require('npm-package-arg')
 const semver = require('semver')
 const completion = require('./utils/completion/installed-deep.js')
 
+// TODO: make Arborist.rebuild() understand the workspaces option
+// and then extend BaseReifyCommand instead
 const BaseCommand = require('./base-command.js')
 class Rebuild extends BaseCommand {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
@@ -36,6 +38,8 @@ class Rebuild extends BaseCommand {
     const arb = new Arborist({
       ...this.npm.flatOptions,
       path: where,
+      // TODO when extending BaseReifyCommand
+      // workspaces: this.workspaces,
     })
 
     if (args.length) {

lib/uninstall.js

@@ -5,8 +5,8 @@ const rpj = require('read-package-json-fast')
 const reifyFinish = require('./utils/reify-finish.js')
 const completion = require('./utils/completion/installed-shallow.js')
 
-const BaseCommand = require('./base-command.js')
-class Uninstall extends BaseCommand {
+const BaseReifyCommand = require('./base-reify-command.js')
+class Uninstall extends BaseReifyCommand {
   static get description () {
     return 'Remove a package'
   }
@@ -66,7 +66,7 @@ class Uninstall extends BaseCommand {
       path,
       log: this.npm.log,
       rm: args,
-
+      workspaces: this.workspaces,
     }
     const arb = new Arborist(opts)
     await arb.reify(opts)

lib/update.js

@@ -6,8 +6,8 @@ const log = require('npmlog')
 const reifyFinish = require('./utils/reify-finish.js')
 const completion = require('./utils/completion/installed-deep.js')
 
-const BaseCommand = require('./base-command.js')
-class Update extends BaseCommand {
+const BaseReifyCommand = require('./base-reify-command.js')
+class Update extends BaseReifyCommand {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
   static get description () {
     return 'Update packages'
@@ -53,6 +53,7 @@ class Update extends BaseCommand {
       ...this.npm.flatOptions,
       log: this.npm.log,
       path: where,
+      workspaces: this.workspaces,
     })
 
     await arb.reify({ update })

test/lib/audit.js

@@ -191,3 +191,5 @@ t.test('completion', t => {
 
   t.end()
 })
+
+t.test('workspaces')

test/lib/base-reify-command.js

@@ -0,0 +1,7 @@
+const t = require('tap')
+const BaseReifyCommand = require('../../lib/base-reify-command.js')
+// Just a dummy thing here so eslint will stop yelling, but it'll still
+// log a todo.
+t.todo('BaseReifyCommand basic tests', { todo: true }, async t => {
+  console.log(BaseReifyCommand)
+})

test/lib/ci.js

@@ -242,3 +242,5 @@ t.test('should remove existing node_modules before installing', (t) => {
       throw er
   })
 })
+
+t.test('workspaces')

test/lib/dedupe.js

@@ -62,3 +62,5 @@ t.test('should remove dupes using Arborist - no arguments', (t) => {
     t.end()
   })
 })
+
+t.test('workspaces')

test/lib/install.js

@@ -218,3 +218,5 @@ t.test('completion', async t => {
   t.notOk(res)
   t.end()
 })
+
+t.test('workspaces')

test/lib/prune.js

@@ -26,3 +26,5 @@ t.test('should prune using Arborist', (t) => {
     t.end()
   })
 })
+
+t.test('workspaces')

test/lib/rebuild.js

@@ -256,3 +256,5 @@ t.test('global prefix', t => {
     t.end()
   })
 })
+
+t.test('workspaces')

test/lib/uninstall.js

@@ -249,3 +249,5 @@ t.test('unknown error reading from localPrefix package.json', t => {
     t.end()
   })
 })
+
+t.test('workspaces')

test/lib/update.js

@@ -37,7 +37,12 @@ t.test('no args', t => {
     constructor (args) {
       t.same(
         args,
-        { ...npm.flatOptions, path: npm.prefix, log: noop },
+        {
+          ...npm.flatOptions,
+          path: npm.prefix,
+          log: noop,
+          workspaces: null,
+        },
         'should call arborist contructor with expected args'
       )
     }
@@ -71,7 +76,12 @@ t.test('with args', t => {
     constructor (args) {
       t.same(
         args,
-        { ...npm.flatOptions, path: npm.prefix, log: noop },
+        {
+          ...npm.flatOptions,
+          path: npm.prefix,
+          log: noop,
+          workspaces: null,
+        },
         'should call arborist contructor with expected args'
       )
     }
@@ -139,7 +149,7 @@ t.test('update --global', t => {
       const { path, ...opts } = args
       t.same(
         opts,
-        { ...npm.flatOptions, log: noop },
+        { ...npm.flatOptions, log: noop, workspaces: undefined },
         'should call arborist contructor with expected options'
       )
 
@@ -164,3 +174,5 @@ t.test('update --global', t => {
       throw err
   })
 })
+
+t.test('workspaces')