Support exec auto pick bin when all bin is alias

[dr-js]

Some scoped package can not use the unscoped portion of package name as bin since it's too common, like @dr-js/core or @dr-js/node. And put the full name in bin will break the auto bin file creation.

This change should allow the package just aliasing bin to skip add bin to npm exec command, like:

{
  "bin": {
    "dr-js": "bin/index.js",
    "DR": "bin/index.js"
  }
}

TODO: If this change is OK, I can update the doc in later commit if needed.

[ruyadorno]

hi @dr-js thanks for taking the time to propose this change 😊 it might be useful for some folks - I'll wait to hear more input from the rest of the team since I don't have much context on the particularities of npx.

What I would like to add to the discussion though (and I think is a relatively less known) is that npx has a syntax that allows you to invoke any command after installing pkgs (while also having available any bin from the installed packages), so today you could use:

npx -p @dr-js/core -c dr-js

[dr-js]

Thanks for the reply!

About the syntax, I do know most of the variation. (also made a docs PR: #1970)
And I think you're right on most people only know and use the shortest one.
And the syntax from v7.0.1/lib/exec.js:

npm exec -- <pkg>[@<version>] [args...]
npm exec --package=<pkg>[@<version>] -- <cmd> [args...]
npm exec -c '<cmd> [args...]'
npm exec --package=foo -c '<cmd> [args...]'

npx <pkg>[@<specifier>] [args...]
npx -p <pkg>[@<specifier>] <cmd> [args...]
npx -c '<cmd> [args...]'
npx -p <pkg>[@<specifier>] -c '<cmd> [args...]'

alias: npm x, npx

--package=<pkg> (may be specified multiple times)
-p is a shorthand for --package only when using npx executable
-c <cmd> --call=<cmd> (may not be mixed with positional arguments)

This PR aims to enable the usage of the shortest syntax for more package. (most scoped package may have this problem, like common @scope-name/cli)

Personally I prefer to use the shortest (and often also the clearest) syntax, for screen-space saving and better readability in package.json scripts and CI shell scripts. And with the npm@7 transition, I think the npx syntax is most likely to be made working for both versions.

Also I've re-done the commit to return bin name instead of bin path, after test working by editing a local npm@7 copy.

[dr-js]

Side note: don't know why test coverage drop from 100% with previous code style:

  const bins = Array.from(new Set(Object.values(mani.bin || {})))
  if (bins.length === 1) {
    return Object.keys(mani.bin || {})[0] // test coverage will report this line uncovered
  }

But reducing code complexity by extracting variable seems to help here.

[isaacs]

The reason you get an uncovered branch on that line is because you can't possibly get there if mani.bin is missing, so the mani.bin || {} will never have a falsey mani.bin and thus never hit the || {} branch.

Try running npm test -- --coverage-report=html to open up a web browser and see missing branches, just trying to go off of line numbers can be tricky for us human brains ;)

[isaacs]

I'd initially been hesitant about this, since we really don't want to guess at the intent if it's not clear. But now that I take a closer look, I see that you're checking that all of the bin values are the same thing, just with different keys. So yes, in that case, I think we can reasonably just take one of them as the default. No fundamental objection from me.

[ruyadorno]

alright, LGTM to me too 👍 I think we just need rebasing and making sure we add document this behavior somewhere in the docs 😊

[dr-js]

Just done rebasing and added simple behavior description to docs/content/commands/npm-exec.md.
I also updated the content of docs/content/commands/npx.md to match the latest.