Yarn: 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 [email protected], 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?

https://github.com/yarnpkg/yarn/blob/8b665e32a3cce6f8c1ce9e355fefa317cf205081/__tests__/index.js#L251-L262

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 🤔

https://github.com/yarnpkg/yarn/blob/0e16ee9a3edf0bf9381458eba6ac9f12047f51d2/src/cli/index.js#L85

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?

[gargoyle]

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 https://github.com/yarnpkg/website/issues/646#issue-257094476

[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 https://github.com/yarnpkg/yarn/pull/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…)

[olingern]

~@BYK Closable via #4498~

[BYK]

@olingern not entirely since there's an edge case when doing yarn run --prod build for instance. It will complain about --prod not being a script which is not the correct behavior.

[olingern]

@BYK 👍 Gotcha