fix(config): Don't pass .yarnrc generated flags to workspace child command (#5534)

* fix(config): Don't pass .yarnrc generated flags to workspace child command

Previously, if flags were defined in .yarnrc then they would be insterted into the rawArgs array of
commander.js. The yarn workspace command would then attempt to pass these flags to the child process
that it spawns. They were being passed out-of-order and causing the child command to fail to
properly parse it's command line args. This change prevents those flags from being passed at all.
The yarn child process will read .yarnrc and inject the flags into it's own argument array anyway,
so passing them was just duplicating the flags.

#5496

* add test for 5534

Author: rally25rs

__tests__/commands/workspace.js

@@ -39,11 +39,11 @@ async function runWorkspace(
 }
 
 // The unit tests don't use commander.js for argument parsing.
-// `rawArgs` is normally passed by commander.js so we just simulate it in the tests.
+// `originalArgs` is normally passed by index.js so we just simulate it in the tests.
 
 test('workspace run command', (): Promise<void> => {
-  const rawArgs = ['/path/to/node', '/path/to/yarn', 'workspace', 'workspace-1', 'run', 'script'];
-  return runWorkspace({rawArgs}, ['workspace-1', 'run', 'script'], 'run-basic', config => {
+  const originalArgs = ['workspace-1', 'run', 'script'];
+  return runWorkspace({originalArgs}, ['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 +52,8 @@ test('workspace run command', (): Promise<void> => {
 });
 
 test('workspace run command forwards raw arguments', (): Promise<void> => {
-  const rawArgs = ['/path/to/node', '/path/to/yarn', 'workspace', 'workspace-1', 'run', 'script', 'arg1', '--flag1'];
-  return runWorkspace({rawArgs}, ['workspace-1', 'run', 'script'], 'run-basic', config => {
+  const originalArgs = ['workspace-1', 'run', 'script', 'arg1', '--flag1'];
+  return runWorkspace({originalArgs}, ['workspace-1', 'run', 'script'], '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__/fixtures/index/run-workspace/.yarnrc

@@ -0,0 +1 @@
+--*.emoji false

__tests__/fixtures/index/run-workspace/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "my-project",
+  "private": true,
+  "workspaces": [
+    "packages/*"
+  ]
+}

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

@@ -0,0 +1,10 @@
+{
+  "name": "workspace-1",
+  "version": "1.0.0",
+  "scripts": {
+    "prescript": "echo workspace-1 prescript",
+    "script": "echo workspace-1 script",
+    "postscript": "echo workspace-1 postscript",
+    "check": "echo $1"
+  }
+}

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

@@ -0,0 +1,12 @@
+{
+  "name": "workspace-2",
+  "version": "1.0.0",
+  "scripts": {
+    "prescript": "echo workspace-2 prescript",
+    "script": "echo workspace-2 script",
+    "postscript": "echo workspace-2 postscript"
+  },
+  "dependencies": {
+    "workspace-1": "1.0.0"
+  }
+}

__tests__/index.js

@@ -316,3 +316,13 @@ test.concurrent('should run help for camelised command', async () => {
   expect(lastLines[2]).toMatch(/yarn generate-lock-entry --resolved local-file.tgz#hash/);
   expect(lastLines[3]).toMatch(/Visit https:\/\/yarnpkg.com\/en\/docs\/cli\/generate-lock-entry/);
 });
+
+// regression test for #5496
+// this fixture has a .yarnrc in it that sets the `--emoji` flag.
+// we test to make sure that flag is not passed down to the workspace command,
+// 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);
+  expect(params).not.toMatch(/emoji/);
+});

src/cli/commands/workspace.js

@@ -21,22 +21,19 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
     throw new MessageError(reporter.lang('workspaceRootNotFound', config.cwd));
   }
 
-  if (args.length < 1) {
+  if (flags.originalArgs < 1) {
     throw new MessageError(reporter.lang('workspaceMissingWorkspace'));
   }
 
-  if (args.length < 2) {
+  if (flags.originalArgs < 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] = args;
-  // rawArgs contains: [nodePath, yarnPath, 'workspace', workspaceName, ...]
-  const [, , , , ...rest] = flags.rawArgs || [];
+  const [workspaceName, ...rest] = flags.originalArgs || [];
 
   if (!Object.prototype.hasOwnProperty.call(workspaces, workspaceName)) {
     throw new MessageError(reporter.lang('workspaceUnknownWorkspace', workspaceName));

src/cli/index.js

@@ -180,6 +180,7 @@ export function main({
     }
   }
 
+  commander.originalArgs = args;
   args = [...preCommandArgs, ...args];
 
   command.setFlags(commander);