fix(workspaces): Pass args following "--" when running workspace scripts (#7786)

* fix(workspaces): Passes arguments follwing "--" when running a workspace script

Passes arguments follwing `--` when running a workspace script (`yarn workspace pkg run command --
arg`). Previously these parameters were being trimmed off and ignored.

fixes #7776

* fix spelling in changelog

* fix(workspaces): fix(workspaces): Passes arguments follwing "--" when running a workspace script

Passes arguments follwing `--` when running a workspace script (`yarn workspace pkg run command --
arg`). Previously these parameters were being trimmed off and ignored.

fixes #7776

* don't ignore a parameter when running 'node' command

Author: rally25rs

CHANGELOG.md

@@ -4,6 +4,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa
 
 ## Master
 
+- Passes arguments following `--` when running a workspace script (`yarn workspace pkg run command -- arg`)
+
+  [#7776](https://github.com/yarnpkg/yarn/pull/7776) - [**Jeff Valore**](https://twitter.com/rally25rs)
+
 - Prints workspace names with `yarn workspaces` (silence with `-s`)
 
   [#7722](https://github.com/yarnpkg/yarn/pull/7722) - [**Orta**](https://twitter.com/orta)

__tests__/commands/workspace.js

@@ -38,12 +38,8 @@ async function runWorkspace(
   }
 }
 
-// The unit tests don't use commander.js for argument parsing.
-// `originalArgs` is normally passed by index.js so we just simulate it in the tests.
-
 test('workspace run command', (): Promise<void> => {
-  const originalArgs = ['workspace-1', 'run', 'script'];
-  return runWorkspace({originalArgs}, ['workspace-1', 'run', 'script'], 'run-basic', config => {
+  return runWorkspace({}, ['workspace-1', 'run', 'script'], 'run-basic', config => {
     expect(spawn).toHaveBeenCalledWith(NODE_BIN_PATH, [YARN_BIN_PATH, 'run', 'script'], {
       stdio: 'inherit',
       cwd: path.join(fixturesLoc, 'run-basic', 'packages', 'workspace-child-1'),
@@ -52,8 +48,7 @@ test('workspace run command', (): Promise<void> => {
 });
 
 test('workspace run command forwards raw arguments', (): Promise<void> => {
-  const originalArgs = ['workspace-1', 'run', 'script', 'arg1', '--flag1'];
-  return runWorkspace({originalArgs}, ['workspace-1', 'run', 'script'], 'run-basic', config => {
+  return runWorkspace({}, ['workspace-1', 'run', 'script', 'arg1', '--flag1'], 'run-basic', config => {
     expect(spawn).toHaveBeenCalledWith(NODE_BIN_PATH, [YARN_BIN_PATH, 'run', 'script', 'arg1', '--flag1'], {
       stdio: 'inherit',
       cwd: path.join(fixturesLoc, 'run-basic', 'packages', 'workspace-child-1'),

__tests__/commands/workspaces.js

@@ -49,13 +49,12 @@ test('workspaces info should list the workspaces', (): Promise<void> => {
 });
 
 test('workspaces run should spawn command for each workspace', (): Promise<void> => {
-  const originalArgs = ['run', 'script', 'arg1', '--flag1'];
-  return runWorkspaces({originalArgs}, ['run', 'script', 'arg1', '--flag1'], 'run-basic', config => {
-    expect(spawn).toHaveBeenCalledWith(NODE_BIN_PATH, [YARN_BIN_PATH, 'script', 'arg1', '--flag1'], {
+  return runWorkspaces({}, ['run', 'script', 'arg1', '--flag1'], 'run-basic', config => {
+    expect(spawn).toHaveBeenCalledWith(NODE_BIN_PATH, [YARN_BIN_PATH, 'run', 'script', 'arg1', '--flag1'], {
       stdio: 'inherit',
       cwd: path.join(fixturesLoc, 'run-basic', 'packages', 'workspace-child-1'),
     });
-    expect(spawn).toHaveBeenCalledWith(NODE_BIN_PATH, [YARN_BIN_PATH, 'script', 'arg1', '--flag1'], {
+    expect(spawn).toHaveBeenCalledWith(NODE_BIN_PATH, [YARN_BIN_PATH, 'run', 'script', 'arg1', '--flag1'], {
       stdio: 'inherit',
       cwd: path.join(fixturesLoc, 'run-basic', 'packages', 'workspace-child-2'),
     });

__tests__/fixtures/index/run-workspace/packages/workspace-child-1/package.json

@@ -5,6 +5,6 @@
     "prescript": "echo workspace-1 prescript",
     "script": "echo workspace-1 script",
     "postscript": "echo workspace-1 postscript",
-    "check": "echo $1"
+    "check": "echo ARGS:"
   }
 }

__tests__/fixtures/index/run-workspace/packages/workspace-child-2/package.json

@@ -4,7 +4,8 @@
   "scripts": {
     "prescript": "echo workspace-2 prescript",
     "script": "echo workspace-2 script",
-    "postscript": "echo workspace-2 postscript"
+    "postscript": "echo workspace-2 postscript",
+    "check": "echo WORKSPACE-CHILD-2 ARGS:"
   },
   "dependencies": {
     "workspace-1": "1.0.0"

__tests__/fixtures/request-cache/GET/gitlab.com/leanlabsio/kanban/raw/7f21696fb9d08130dd62abd96c9572f513c05301/package.json.bin

@@ -0,0 +1,62 @@
+HTTP/1.1 200 OK
+Server: nginx
+Date: Sun, 29 Dec 2019 14:24:54 GMT
+Content-Type: text/plain; charset=utf-8
+Content-Length: 1181
+Cache-Control: max-age=3600, public
+Content-Disposition: inline
+Etag: W/"78309fbf8af4479c47eca65b0c5e3f51"
+Referrer-Policy: strict-origin-when-cross-origin
+Set-Cookie: experimentation_subject_id=IjJjZjk1MWRjLWUzMDgtNDc3ZC05MTlkLTA0MGU5MWFjY2VlOCI%3D--64f51f0ae3455ebf84d41c0a0a6723703e719689; domain=.gitlab.com; path=/; expires=Thu, 29 Dec 2039 14:24:54 -0000; secure; HttpOnly
+X-Content-Type-Options: nosniff
+X-Download-Options: noopen
+X-Frame-Options: DENY
+X-Permitted-Cross-Domain-Policies: none
+X-Request-Id: xcIbPa5i6G2
+X-Runtime: 0.056763
+X-Ua-Compatible: IE=edge
+X-Xss-Protection: 1; mode=block
+Strict-Transport-Security: max-age=31536000
+Referrer-Policy: strict-origin-when-cross-origin
+GitLab-LB: fe-07-lb-gprd
+GitLab-SV: web-28-sv-gprd
+
+{
+  "name": "kanban",
+  "version": "0.0.1",
+  "repository": "gitlab.com/leanlabsio/kanban",
+  "scripts": {
+      "install": "npm install",
+      "build": "grunt build",
+      "watch": "grunt watch"
+  },
+  "devDependencies": {
+    "grunt": "~0.4.1",
+    "grunt-cli": "~0.1.13",
+    "grunt-contrib-copy": "^0.5.0",
+    "grunt-contrib-concat": "~0.5.0",
+    "grunt-contrib-watch": "~0.5.3",
+    "grunt-contrib-uglify": "~0.7.0",
+    "grunt-sass": "1.0.0",
+    "grunt-contrib-connect": "~0.8.0",
+    "grunt-connect-proxy": "~0.1.11"
+  },
+  "dependencies": {
+    "angular": "=1.5.6",
+    "angular-lodash": "https://github.com/EMSSConsulting/angular-lodash.git#68a726c",
+    "foundation-sites": "5.5.2",
+    "angular-foundation": "https://github.com/pineconellc/angular-foundation.git#8f3f260",
+    "angular-loading-bar": "=0.5.2",
+    "angular-storage": "=0.0.6",
+    "angular-ui-router": "=0.3.0",
+    "angularjs-datepicker": "=0.2.15",
+    "font-awesome": "=4.6.3",
+    "markdown-it": "=5.0.2",
+    "markdown-it-emoji": "=1.1.0",
+    "ng-sortable": "=1.3.6",
+    "sass-flex-mixin": "=1.0.3",
+    "lodash": "=4.13.1",
+    "twemoji": "=2.1.0",
+    "angular-file-upload": "=2.3.4"
+  }
+}

__tests__/index.js

@@ -355,6 +355,39 @@ test.concurrent('should run help for camelised command', async () => {
 // but actual flags on the command line are passed.
 test.concurrent('should not pass yarnrc flags to workspace command', async () => {
   const stdout = await execCommand('workspace', ['workspace-1', 'run', 'check', '--x'], 'run-workspace', true);
-  const params = stdout.find(x => x && x.indexOf('--x') >= 0);
+  const params = stdout.find(x => x && x.indexOf('ARGS:') === 0);
   expect(params).not.toMatch(/emoji/);
 });
+
+// regression test for #7776
+test.concurrent('should pass args to workspace command without need for "--"', async () => {
+  const stdout = await execCommand('workspace', ['workspace-1', 'run', 'check', '--x', 'y'], 'run-workspace', true);
+  const params = stdout.find(x => x && x.indexOf('ARGS:') === 0);
+  expect(params).toEqual('ARGS: --x y');
+});
+
+// regression test for #7776
+test.concurrent('should pass args after "--" to workspace command', async () => {
+  const stdout = await execCommand(
+    'workspace',
+    ['workspace-1', 'run', 'check', '--', 'x', '-y'],
+    'run-workspace',
+    true,
+  );
+  const params = stdout.find(x => x && x.indexOf('ARGS:') === 0);
+  expect(params).toEqual('ARGS: x -y');
+});
+
+// regression test for #7776
+test.concurrent('should pass args to workspaces command without need for "--"', async () => {
+  const stdout = await execCommand('workspaces', ['run', 'check', '--x', 'y'], 'run-workspace', true);
+  const params = stdout.find(x => x && x.indexOf('ARGS:') === 0);
+  expect(params).toEqual('ARGS: --x y');
+});
+
+// regression test for #7776
+test.concurrent('should pass args after "--" to workspaces command', async () => {
+  const stdout = await execCommand('workspaces', ['run', 'check', '--', 'x', '-y'], 'run-workspace', true);
+  const params = stdout.find(x => x && x.indexOf('ARGS:') === 0);
+  expect(params).toEqual('ARGS: x -y');
+});

src/cli/commands/workspace.js

@@ -21,19 +21,19 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
     throw new MessageError(reporter.lang('workspaceRootNotFound', config.cwd));
   }
 
-  if (flags.originalArgs < 1) {
+  if (args.length < 1) {
     throw new MessageError(reporter.lang('workspaceMissingWorkspace'));
   }
 
-  if (flags.originalArgs < 2) {
+  if (args.length < 2) {
     throw new MessageError(reporter.lang('workspaceMissingCommand'));
   }
 
   const manifest = await config.findManifest(workspaceRootFolder, false);
   invariant(manifest && manifest.workspaces, 'We must find a manifest with a "workspaces" property');
 
   const workspaces = await config.resolveWorkspaces(workspaceRootFolder, manifest);
-  const [workspaceName, ...rest] = flags.originalArgs || [];
+  const [workspaceName, ...rest] = args || [];
 
   if (!Object.prototype.hasOwnProperty.call(workspaces, workspaceName)) {
     throw new MessageError(reporter.lang('workspaceUnknownWorkspace', workspaceName));

src/cli/commands/workspaces.js

@@ -76,12 +76,10 @@ export async function runScript(config: Config, reporter: Reporter, flags: Objec
   const workspaces = await config.resolveWorkspaces(workspaceRootFolder, manifest);
 
   try {
-    const [_, ...rest] = flags.originalArgs || [];
-
     for (const workspaceName of Object.keys(workspaces)) {
       const {loc} = workspaces[workspaceName];
       reporter.log(`${os.EOL}> ${workspaceName}`);
-      await child.spawn(NODE_BIN_PATH, [YARN_BIN_PATH, ...rest], {
+      await child.spawn(NODE_BIN_PATH, [YARN_BIN_PATH, 'run', ...args], {
         stdio: 'inherit',
         cwd: loc,
       });

src/cli/index.js

@@ -198,15 +198,20 @@ export async function main({
   let warnAboutRunDashDash = false;
   // we are using "yarn <script> -abc", "yarn run <script> -abc", or "yarn node -abc", we want -abc
   // to be script options, not yarn options
-  const PROXY_COMMANDS = new Set([`run`, `create`, `node`]);
-  if (PROXY_COMMANDS.has(commandName)) {
+
+  // PROXY_COMMANDS is a map of command name to the number of preservedArgs
+  const PROXY_COMMANDS = {
+    run: 1, // yarn run {command}
+    create: 1, // yarn create {project}
+    node: 0, // yarn node
+    workspaces: 1, // yarn workspaces {command}
+    workspace: 2, // yarn workspace {package} {command}
+  };
+  if (PROXY_COMMANDS.hasOwnProperty(commandName)) {
     if (endArgs.length === 0) {
-      let preservedArgs = 0;
-      // the "run" and "create" command take one argument that we want to parse as usual (the
-      // script/package name), hence the splice(1)
-      if (command === commands.run || command === commands.create) {
-        preservedArgs += 1;
-      }
+      // $FlowFixMe doesn't like that PROXY_COMMANDS doesn't have keys for all commands.
+      let preservedArgs = PROXY_COMMANDS[commandName];
+
       // If the --into option immediately follows the command (or the script name in the "run/create"
       // case), we parse them as regular options so that we can cd into them
       if (args[preservedArgs] === `--into`) {
@@ -218,7 +223,6 @@ export async function main({
     }
   }
 
-  commander.originalArgs = args;
   args = [...preCommandArgs, ...args];
 
   command.setFlags(commander);