-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass-through extra arguments #34
Comments
So I have this:
I would like to be able to pass extra arguments on the cli, for example:
And effectively that would run |
aomarks
added a commit
that referenced
this issue
Jun 3, 2022
You can now pass extra arguments to a script from the command-line like this: ```shell npm run build -- -- --verbose ``` The two double-dashes are needed because the first one tells npm to forward arguments to wireit, and the second one tells wireit to forward arguments to the script. It is also now an error to pass any unexpected argument to wireit (expected argument are currently just `watch` and `--`). Previously they were silently eaten. It would be possible for us to relax the requirement for the second double-dash, and automatically pass through any arguments which we don't recognize as our own. However, that means every time we add a new argument, we'll need a breaking change. I think this is something we could relax in the future, but unless we get rid of arguments entirely, we'll still want `--` to mean "the following argument is for the script, not wireit" (e.g. if you wanted to pass a `watch` argument to the script). So I think this PR is a good starting point, with the option to relax later. Fixes #34 cc @jpzwarte
aomarks
added a commit
that referenced
this issue
Jun 9, 2022
This PR is a follow up to #272 which changes the syntax for the watch mode flag and extra arguments. Fixes #34 cc @jpzwarte ### Old syntax ```sh npm run build watch -- -- --extra-args ``` ### New syntax ``` npm run build --watch -- --extra-args ``` ### Rationale 1. It turns out that npm allows us to capture the `--watch` argument from `npm run build --watch` by reading the `npm_config_watch` environment variable. (I did not realize this was possible when originally thinking about argument handling). 2. npm allows us to distinguish between `npm run build --watch` and `npm run build -- --watch`. However, it does *not* allow us to distinguish between `npm run build watch` and `npm run build -- watch`. This was the reason we needed the double `-- --` before, so now one is sufficient. 3. Only having to write one `--` seems much more natural, and exactly matches the built-in npm behavior. Writing `--watch` vs `watch` also seems more natural, since most other tools use the `--flag` style. 4. yarn and pnpm handle arguments differently to npm, and were not compatible with the changes I made in #272. Both yarn and pnpm actually allow us to reconstruct all of the command-line arguments, so while they each need special handling, we can match the behavior of this new syntax. They are also now tested. ### Other changes 1. Factored out a `cli-options` module because that code is now quite long, and for the next item... 2. Created a new set of `cli-options` tests that directly check the parsed options. This is more convenient and faster than the full integration tests, where we have to infer what options were parsed from the behavior. 3. Reverted the (unreleased) change from #272 which made unknown arguments an error. The problem with this is that Wireit arguments are now going to appear as `npm_config_<flag>` environment variables, and there are a bunch of `npm_config_*` variables that also get set that we would have to allow-list in order to decide which ones are unrecognized. Since those could change across npm versions or configurations, it seems risky to try and account for them all. Given that npm itself already silently ignores any `--flags` before a `--`, and we're trying to act like an extension to npm, I think this is fine. 4. Documented the support policy for npm/yarn/pnpm.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
No description provided.
The text was updated successfully, but these errors were encountered: