From ec520ce32d5e834a32ebd58491df4200e01ce690 Mon Sep 17 00:00:00 2001
From: nlf <quitlahok@gmail.com>
Date: Tue, 30 Mar 2021 08:05:41 -0700
Subject: [PATCH] feat(set-script): implement workspaces

Implements workspaces for set-script, also refactors tests to mock as
little as possible.

PR-URL: https://github.com/npm/cli/pull/2998
Credit: @nlf
Close: #2998
Reviewed-by: @wraithgar
---
 lib/set-script.js                             |  81 ++++--
 .../test-lib-utils-npm-usage.js-TAP.test.js   |   3 +
 test/lib/set-script.js                        | 236 +++++++++---------
 3 files changed, 187 insertions(+), 133 deletions(-)

diff --git a/lib/set-script.js b/lib/set-script.js
index 9d4aadad558f..b31e123becd8 100644
--- a/lib/set-script.js
+++ b/lib/set-script.js
@@ -3,6 +3,7 @@ const fs = require('fs')
 const parseJSON = require('json-parse-even-better-errors')
 const rpj = require('read-package-json-fast')
 const { resolve } = require('path')
+const getWorkspaces = require('./workspaces/get-workspaces.js')
 
 const BaseCommand = require('./base-command.js')
 class SetScript extends BaseCommand {
@@ -11,6 +12,11 @@ class SetScript extends BaseCommand {
     return 'Set tasks in the scripts section of package.json'
   }
 
+  /* istanbul ignore next - see test/lib/load-all-commands.js */
+  static get params () {
+    return ['workspace', 'workspaces']
+  }
+
   /* istanbul ignore next - see test/lib/load-all-commands.js */
   static get name () {
     return 'set-script'
@@ -31,49 +37,90 @@ class SetScript extends BaseCommand {
     }
   }
 
-  exec (args, cb) {
-    this.set(args).then(() => cb()).catch(cb)
-  }
-
-  async set (args) {
+  validate (args) {
     if (process.env.npm_lifecycle_event === 'postinstall')
       throw new Error('Scripts can’t set from the postinstall script')
 
     // Parse arguments
     if (args.length !== 2)
       throw new Error(`Expected 2 arguments: got ${args.length}`)
+  }
 
+  exec (args, cb) {
+    this.set(args).then(() => cb()).catch(cb)
+  }
+
+  async set (args) {
+    this.validate(args)
+    const warn = this.setScript(this.npm.localPrefix, args[0], args[1])
+    if (warn)
+      log.warn('set-script', `Script "${args[0]}" was overwritten`)
+  }
+
+  execWorkspaces (args, filters, cb) {
+    this.setWorkspaces(args, filters).then(() => cb()).catch(cb)
+  }
+
+  async setWorkspaces (args, filters) {
+    this.validate(args)
+    const workspaces =
+      await getWorkspaces(filters, { path: this.npm.localPrefix })
+
+    for (const [name, path] of workspaces) {
+      try {
+        const warn = this.setScript(path, args[0], args[1])
+        if (warn) {
+          log.warn('set-script', `Script "${args[0]}" was overwritten`)
+          log.warn(`  in workspace: ${name}`)
+          log.warn(`  at location: ${path}`)
+        }
+      } catch (err) {
+        log.error('set-script', err.message)
+        log.error(`  in workspace: ${name}`)
+        log.error(`  at location: ${path}`)
+        process.exitCode = 1
+      }
+    }
+  }
+
+  // returns a Boolean that will be true if
+  // the requested script was overwritten
+  // and false if it was set as a new script
+  setScript (path, name, value) {
     // Set the script
     let manifest
     let warn = false
+
     try {
-      manifest = fs.readFileSync(this.npm.localPrefix + '/package.json', 'utf-8')
+      manifest = fs.readFileSync(resolve(path, 'package.json'), 'utf-8')
     } catch (error) {
       throw new Error('package.json not found')
     }
+
     try {
       manifest = parseJSON(manifest)
     } catch (error) {
       throw new Error(`Invalid package.json: ${error}`)
     }
+
     if (!manifest.scripts)
       manifest.scripts = {}
-    if (manifest.scripts[args[0]] && manifest.scripts[args[0]] !== args[1])
+
+    if (manifest.scripts[name] && manifest.scripts[name] !== value)
       warn = true
-    manifest.scripts[args[0]] = args[1]
+    manifest.scripts[name] = value
+
     // format content
-    const packageJsonInfo = await rpj(this.npm.localPrefix + '/package.json')
     const {
       [Symbol.for('indent')]: indent,
       [Symbol.for('newline')]: newline,
-    } = packageJsonInfo
-    const format = indent === undefined ? '  ' : indent
-    const eol = newline === undefined ? '\n' : newline
-    const content = (JSON.stringify(manifest, null, format) + '\n')
-      .replace(/\n/g, eol)
-    fs.writeFileSync(this.npm.localPrefix + '/package.json', content)
-    if (warn)
-      log.warn('set-script', `Script "${args[0]}" was overwritten`)
+    } = manifest
+
+    const content = (JSON.stringify(manifest, null, indent) + '\n')
+      .replace(/\n/g, newline)
+    fs.writeFileSync(resolve(path, 'package.json'), content)
+
+    return warn
   }
 }
 module.exports = SetScript
diff --git a/tap-snapshots/test-lib-utils-npm-usage.js-TAP.test.js b/tap-snapshots/test-lib-utils-npm-usage.js-TAP.test.js
index 2d229a2691f8..4f9c1b44ebf3 100644
--- a/tap-snapshots/test-lib-utils-npm-usage.js-TAP.test.js
+++ b/tap-snapshots/test-lib-utils-npm-usage.js-TAP.test.js
@@ -771,6 +771,9 @@ All commands:
                     Usage:
                     npm set-script [<script>] [<command>]
                     
+                    Options:
+                    [-w|--workspace <workspace>|-w|--workspace <workspace>] [-ws|--workspaces]
+                    
                     Run "npm help set-script" for more info
 
     shrinkwrap      npm shrinkwrap
diff --git a/test/lib/set-script.js b/test/lib/set-script.js
index dc00fd374cc1..0071fa20f247 100644
--- a/test/lib/set-script.js
+++ b/test/lib/set-script.js
@@ -1,195 +1,199 @@
 const test = require('tap')
 const requireInject = require('require-inject')
+const fs = require('fs')
 const parseJSON = require('json-parse-even-better-errors')
+const mockNpm = require('../fixtures/mock-npm.js')
+const { resolve } = require('path')
+
+const flatOptions = {}
+const npm = mockNpm(flatOptions)
+
+const ERROR_OUTPUT = []
+const WARN_OUTPUT = []
+const SetScript = requireInject('../../lib/set-script.js', {
+  npmlog: {
+    error: (...args) => {
+      ERROR_OUTPUT.push(args)
+    },
+    warn: (...args) => {
+      WARN_OUTPUT.push(args)
+    },
+  },
+})
+const setScript = new SetScript(npm)
 
 test.test('completion', t => {
-  const SetScript = requireInject('../../lib/set-script.js')
-  const emptyDir = t.testdir()
   t.test('already have a script name', async t => {
-    const setScript = new SetScript({localPrefix: emptyDir})
+    npm.localPrefix = t.testdir({})
     const res = await setScript.completion({conf: {argv: {remain: ['npm', 'run', 'x']}}})
     t.equal(res, undefined)
     t.end()
   })
+
   t.test('no package.json', async t => {
-    const setScript = new SetScript({localPrefix: emptyDir})
+    npm.localPrefix = t.testdir({})
     const res = await setScript.completion({conf: {argv: {remain: ['npm', 'run']}}})
     t.strictSame(res, [])
     t.end()
   })
+
   t.test('has package.json, no scripts', async t => {
-    const localPrefix = t.testdir({
+    npm.localPrefix = t.testdir({
       'package.json': JSON.stringify({}),
     })
-    const setScript = new SetScript({localPrefix})
     const res = await setScript.completion({conf: {argv: {remain: ['npm', 'run']}}})
     t.strictSame(res, [])
     t.end()
   })
+
   t.test('has package.json, with scripts', async t => {
-    const localPrefix = t.testdir({
+    npm.localPrefix = t.testdir({
       'package.json': JSON.stringify({
         scripts: { hello: 'echo hello', world: 'echo world' },
       }),
     })
-    const setScript = new SetScript({localPrefix})
     const res = await setScript.completion({conf: {argv: {remain: ['npm', 'run']}}})
     t.strictSame(res, ['hello', 'world'])
     t.end()
   })
+
   t.end()
 })
+
 test.test('fails on invalid arguments', (t) => {
-  const SetScript = requireInject('../../lib/set-script.js', {
-    npmlog: {},
-  })
-  const setScript = new SetScript({})
   t.plan(3)
   setScript.exec(['arg1'], (fail) => t.match(fail, /Expected 2 arguments: got 1/))
   setScript.exec(['arg1', 'arg2', 'arg3'], (fail) => t.match(fail, /Expected 2 arguments: got 3/))
   setScript.exec(['arg1', 'arg2', 'arg3', 'arg4'], (fail) => t.match(fail, /Expected 2 arguments: got 4/))
 })
+
 test.test('fails if run in postinstall script', (t) => {
-  const originalVar = process.env.npm_lifecycle_event
-  process.env.npm_lifecycle_event = 'postinstall'
-  const SetScript = requireInject('../../lib/set-script.js', {
-    npmlog: {},
+  const lifecycleEvent = process.env.npm_lifecycle_event
+  t.teardown(() => {
+    process.env.npm_lifecycle_event = lifecycleEvent
   })
+
+  process.env.npm_lifecycle_event = 'postinstall'
   t.plan(1)
-  const setScript = new SetScript({})
   setScript.exec(['arg1', 'arg2'], (fail) => t.equal(fail.toString(), 'Error: Scripts can’t set from the postinstall script'))
-  process.env.npm_lifecycle_event = originalVar
 })
+
 test.test('fails when package.json not found', (t) => {
-  const SetScript = requireInject('../../lib/set-script.js')
-  const setScript = new SetScript({})
   t.plan(1)
   setScript.exec(['arg1', 'arg2'], (fail) => t.match(fail, /package.json not found/))
 })
+
 test.test('fails on invalid JSON', (t) => {
-  const SetScript = requireInject('../../lib/set-script.js', {
-    '../../lib/utils/config/definitions.js': {},
-    fs: {
-      readFile: () => {}, // read-package-json-fast explodes w/o this
-      readFileSync: (name, charcode) => {
-        return 'iamnotjson'
-      },
-    },
+  npm.localPrefix = t.testdir({
+    'package.json': 'iamnotjson',
   })
-  const setScript = new SetScript({})
+
   t.plan(1)
   setScript.exec(['arg1', 'arg2'], (fail) => t.match(fail, /Invalid package.json: JSONParseError/))
 })
+
 test.test('creates scripts object', (t) => {
-  var mockFile = ''
-  const SetScript = requireInject('../../lib/set-script.js', {
-    '../../lib/utils/config/definitions.js': {},
-    fs: {
-      readFileSync: (name, charcode) => {
-        return '{}'
-      },
-      writeFileSync: (location, inner) => {
-        mockFile = inner
-      },
-    },
-    'read-package-json-fast': async function (filename) {
-      return {
-        [Symbol.for('indent')]: '  ',
-        [Symbol.for('newline')]: '\n',
-      }
-    },
+  npm.localPrefix = t.testdir({
+    'package.json': '{}',
   })
-  const setScript = new SetScript({})
+
   t.plan(2)
   setScript.exec(['arg1', 'arg2'], (error) => {
     t.equal(error, undefined)
-    t.assert(parseJSON(mockFile), {scripts: {arg1: 'arg2'}})
+    const contents = fs.readFileSync(resolve(npm.localPrefix, 'package.json'))
+    t.assert(parseJSON(contents), {scripts: {arg1: 'arg2'}})
   })
 })
-test.test('warns before overwriting', (t) => {
-  var warningListened = ''
-  const SetScript = requireInject('../../lib/set-script.js', {
-    '../../lib/utils/config/definitions.js': {},
-    fs: {
-      readFileSync: (name, charcode) => {
-        return JSON.stringify({
-          scripts: {
-            arg1: 'blah',
-          },
-        })
-      },
-      writeFileSync: (name, content) => {},
-    },
-    'read-package-json-fast': async function (filename) {
-      return {
-        [Symbol.for('indent')]: '  ',
-        [Symbol.for('newline')]: '\n',
-      }
-    },
-    npmlog: {
-      warn: (prefix, message) => {
-        warningListened = message
+
+test.test('warns when overwriting', (t) => {
+  WARN_OUTPUT.length = 0
+  npm.localPrefix = t.testdir({
+    'package.json': JSON.stringify({
+      scripts: {
+        arg1: 'blah',
       },
-    },
+    }),
   })
-  const setScript = new SetScript({})
+
   t.plan(2)
   setScript.exec(['arg1', 'arg2'], (error) => {
     t.equal(error, undefined, 'no error')
-    t.equal(warningListened, 'Script "arg1" was overwritten')
+    t.hasStrict(WARN_OUTPUT[0], ['set-script', 'Script "arg1" was overwritten'], 'warning was logged')
   })
 })
+
 test.test('provided indentation and eol is used', (t) => {
-  var mockFile = ''
-  const SetScript = requireInject('../../lib/set-script.js', {
-    '../../lib/utils/config/definitions.js': {},
-    fs: {
-      readFileSync: (name, charcode) => {
-        return '{}'
-      },
-      writeFileSync: (name, content) => {
-        mockFile = content
-      },
-    },
-    'read-package-json-fast': async function (filename) {
-      return {
-        [Symbol.for('indent')]: ' '.repeat(6),
-        [Symbol.for('newline')]: '\r\n',
-      }
-    },
+  npm.localPrefix = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'foo',
+    }, null, ' '.repeat(6)).replace(/\n/g, '\r\n'),
   })
-  const setScript = new SetScript({})
+
   t.plan(3)
   setScript.exec(['arg1', 'arg2'], (error) => {
     t.equal(error, undefined)
-    t.equal(mockFile.split('\r\n').length > 1, true)
-    t.equal(mockFile.split('\r\n').every((value) => !value.startsWith(' ') || value.startsWith(' '.repeat(6))), true)
+    // rather than checking every line's content
+    // we parse the result and verify the symbols match
+    const contents = fs.readFileSync(resolve(npm.localPrefix, 'package.json'))
+    const data = parseJSON(contents)
+    t.equal(data[Symbol.for('indent')], ' '.repeat(6), 'keeps indenting')
+    t.equal(data[Symbol.for('newline')], '\r\n', 'keeps newlines')
   })
 })
-test.test('goes to default when undefined indent and eol provided', (t) => {
-  var mockFile = ''
-  const SetScript = requireInject('../../lib/set-script.js', {
-    '../../lib/utils/config/definitions.js': {},
-    fs: {
-      readFileSync: (name, charcode) => {
-        return '{}'
-      },
-      writeFileSync: (name, content) => {
-        mockFile = content
-      },
+
+test.test('workspaces', (t) => {
+  ERROR_OUTPUT.length = 0
+  WARN_OUTPUT.length = 0
+  npm.localPrefix = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'workspaces-test',
+      version: '1.0.0',
+      workspaces: ['workspace-a', 'workspace-b', 'workspace-c'],
+    }),
+    'workspace-a': {
+      'package.json': '{}',
+    },
+    'workspace-b': {
+      'package.json': '"notjson"',
     },
-    'read-package-json-fast': async function (filename) {
-      return {
-        [Symbol.for('indent')]: undefined,
-        [Symbol.for('newline')]: undefined,
-      }
+    'workspace-c': {
+      'package.json': JSON.stringify({
+        scripts: {
+          arg1: 'test',
+        },
+      }, null, ' '.repeat(6)).replace(/\n/g, '\r\n'),
     },
   })
-  const setScript = new SetScript({})
-  t.plan(3)
-  setScript.exec(['arg1', 'arg2'], (error) => {
-    t.equal(error, undefined)
-    t.equal(mockFile.split('\n').length > 1, true)
-    t.equal(mockFile.split('\n').every((value) => !value.startsWith(' ') || value.startsWith('  ')), true)
+
+  setScript.execWorkspaces(['arg1', 'arg2'], [], (error) => {
+    t.equal(error, undefined, 'did not callback with an error')
+    t.equal(process.exitCode, 1, 'did set the exitCode to 1')
+    // force the exitCode back to 0 to make tap happy
+    process.exitCode = 0
+
+    // workspace-a had the script added
+    const contentsA = fs.readFileSync(resolve(npm.localPrefix, 'workspace-a', 'package.json'))
+    const dataA = parseJSON(contentsA)
+    t.hasStrict(dataA, { scripts: { arg1: 'arg2' } }, 'defined the script')
+
+    // workspace-b logged an error
+    t.match(ERROR_OUTPUT, [
+      ['set-script', `Cannot create property 'scripts' on string 'notjson'`],
+      ['  in workspace: workspace-b'],
+      [`  at location: ${resolve(npm.localPrefix, 'workspace-b')}`],
+    ], 'logged workspace-b error')
+
+    // workspace-c overwrite a script and logged a warning
+    const contentsC = fs.readFileSync(resolve(npm.localPrefix, 'workspace-c', 'package.json'))
+    const dataC = parseJSON(contentsC)
+    t.hasStrict(dataC, { scripts: { arg1: 'arg2' } }, 'defined the script')
+    t.equal(dataC[Symbol.for('indent')], ' '.repeat(6), 'kept the correct indent')
+    t.equal(dataC[Symbol.for('newline')], '\r\n', 'kept the correct newline')
+    t.match(WARN_OUTPUT, [
+      ['set-script', 'Script "arg1" was overwritten'],
+      ['  in workspace: workspace-c'],
+      [`  at location: ${resolve(npm.localPrefix, 'workspace-c')}`],
+    ], 'logged workspace-c warning')
+    t.end()
   })
 })