Confusing order of CLI flags

[zemlanin]

Do you want to request a feature or report a bug?
bug

What is the current behavior?
I've got this script in package.json:

    "build": "webpack --config a.config.js --config b.config.js",

In v0.27.5 (and prior) command yarn build --no-progress --prod called webpack --config a.config.js --config b.config.js with production NODE_ENV

Now, after update to yarn@1.0.2, the same yarn build --no-progress --prod calls webpack --config a.config.js --config b.config.js "--no-progress" "--prod", passing through --no-progress --prod to webpack

I've tried to rearrange arguments in yarn build --no-progress --prod so that it worked as previously, but:

• yarn run build --no-progress --prod still calls webpack --config a.config.js --config b.config.js "--no-progress" "--prod"
• yarn run --no-progress --prod build (maybe I needed set yarn flags before script name) results in error Command "--prod" not found.
  • (flags reordering only changes "not found" command name)
• yarn --no-progress --prod run build results in error No command specified.
• and only yarn --prod --no-progress run build (alongside with yarn --prod --no-progress build) worked as expected

If the current behavior is a bug, please provide the steps to reproduce.

https://gist.github.com/zemlanin/67f3d1db3dc39997bf9aa164a358eb1b

$ git clone https://gist.github.com/67f3d1db3dc39997bf9aa164a358eb1b.git yarn-flags-order

$ cd yarn-flags-order

$ yarn echo --no-progress --prod
  yarn echo v1.0.2
  $ echo $NODE_ENV "--no-progress" "--prod"
  --no-progress --prod
  ✨  Done in 0.13s.

$ yarn --no-progress --prod echo
  yarn echo v1.0.2
  error No command specified.
  info Commands available from binary scripts: 
  info Project commands
     - echo
        echo $NODE_ENV
  ^C
  error No command specified.
  ✨  Done in 2.98s.

$ yarn --prod --no-progress echo
  yarn echo v1.0.2
  $ echo $NODE_ENV
  production
  ✨  Done in 0.12s.

What is the expected behavior?

• flags before double-dash (--) do not pass to the script
• order of the flags should not be important (at least in this case)

Please mention your node.js, yarn and operating system version.

$ node -v
v8.5.0
$ yarn -v
1.0.2
# OS X 10.12.6

[BYK]

Hey, thanks for the report!

So with Yarn 1.0, we have removed the necessity for -- to pass commands to scripts since it was not intuitive and for subscripts, you needed to do multiple -- sequences which was not scaling. The expected way to pass arguments to yarn when running scripts now are as follows:

yarn <yarn args> run <script> <script args>
yarn <yarn args> <script> <script args>

After clearing that up, I see two bugs here:

1. yarn --no-progress --prod run build should definitely work
2. yarn run --no-progress --prod build should report --no-progress as the non-existent command name, not --prod.

Would you like to try submitting a PR to fix this? Shouldn't be too hard. Looks like I overlooked something when fixing this in #4384.

[zemlanin]

How should

yarn run --no-progress --prod build should report --no-progress as the non-existent command name, not --prod.

interact with commands like yarn run --help custom-script and yarn run --silent custom-script --help which are already covered with tests?

yarn/__tests__/index.js

Lines 251 to 262 in 8b665e3

	test.concurrent('should run help of run command if --help is before script', async () => {
	const stdout = await execCommand('run', ['--help', 'custom-script'], 'run-custom-script-with-arguments');
	expect(stdout[0]).toEqual('Usage: yarn [command] [flags]');
	expect(stdout[stdout.length - 1]).toEqual(
	'Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.',
	);
	});
	test.concurrent('should run help of custom-script if --help is after script', async () => {
	const stdout = await execCommand('run', ['--silent', 'custom-script', '--help'], 'run-custom-script-with-arguments');
	expect(JSON.parse(stdout.join('\n'))).toContain('--help');
	});

I was thinking about changing execCommand arguments in 'should run help of custom-script if --help is after script' test case

[zemlanin]

Regarding

yarn --no-progress --prod run build should definitely work

The bug is due to incorrect assignment of the script name to --prod option. After run name is removed from args, CLI parses args as --no-progress --prod=build

[BYK]

I think --prod case is an exception due to commander.option('--prod, --production [prod]', '');. I think this line is faulty and it should be commander.option('--prod, --production [bool]', ''); instead. What do you think?

For yarn run --help custom-script --whatever, I think we should keep supporting yarn <yarn args> run <moar yarn args> <script name> <script args> pattern. This is a bit convoluted, sorry :(

[zemlanin]

I think --prod case is an exception due to commander.option('--prod, --production [prod]', '');. I think this line is faulty and it should be commander.option('--prod, --production [bool]', ''); instead. What do you think?

If instead of yarn --prod run build you try to call yarn --emoji run build, you'll get the same error, even though --emoji flag in commander's options got [bool] type. It looks to me like some limitation/bug in commander 🤔

yarn/src/cli/index.js

Line 85 in 0e16ee9

commander.option('--emoji [bool]', 'enable emoji in output', process.platform === 'darwin');

For yarn run --help custom-script --whatever, I think we should keep supporting yarn <yarn args> run <moar yarn args> <script name> <script args> pattern

If so, then yarn run --no-progress --prod build should report on neither --no-progress nor --prod, shouldn't it? Or should it make an exception for --help flag?

[paul-court]

Not sure if this is related, so please point me in the right direction if not.

Script 1, defined as:-
"less:test1": "./node_modules/less/bin/lessc"

And run with:-
yarn run less:test1

"Works" as expected, in that it outputs the lessc command help

However, script 2 defined as:-
"less:test2": "./node_modules/less/bin/lessc less/app.less >css/app.css"

and run with:-
yarn run less:test2

Fails with the error:-

yarn run v1.0.2-20170916.1056
warning package.json: No license field
$ ./node_modules/less/bin/lessc less/app.less >css/app.css
error Couldn't find the binary ./node_modules/less/bin/lessc less/app.less >css/app.css
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

[BYK]

@zemlanin

If instead of yarn --prod run build you try to call yarn --emoji run build, you'll get the same error, even though --emoji flag in commander's options got [bool] type. It looks to me like some limitation/bug in commander 🤔

Actually, commander is not buggy, it is just harder to reason about. Turns out if a flag is boolean, you just don't specify any values for it. So we need to convert --emoji [bool] to just --emoji and --prod [prod] to just --prod.

@Gargoyle - Not relevant. You are using an unsupported Node version. You need either ^4.8.0 or >=5.7.0. See yarnpkg/website#646 (comment)

[zemlanin]

So we need to convert --emoji [bool] to just --emoji and --prod [prod] to just --prod

Ok, then I'll need to work around --production=false from #2223 (because yarn install --production=false is not equivalent to yarn install 😕), which is not as easy as I thought (already tried different combinations of --prod, --no-prod, defaultValue = true, defaultValue = false…)