Skip to content
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

Replace minimist with parseArgs from node:util #31

Closed
4 tasks done
ChristianMurphy opened this issue Feb 24, 2023 · 14 comments
Closed
4 tasks done

Replace minimist with parseArgs from node:util #31

ChristianMurphy opened this issue Feb 24, 2023 · 14 comments
Labels
🏡 area/internal This affects the hidden internals 👶 area/size This affects (bundle) size 🙅 no/wontfix This is not (enough of) an issue for this project 👎 phase/no Post cannot or will not be acted on 🧑 semver/major This is a change 🦋 type/enhancement This is great to have

Comments

@ChristianMurphy
Copy link
Member

Initial checklist

Problem

minimist is an additional dependency which needs to be downloaded.

Solution

Node now provides parseArgs as part of node:util, with a similar functionality and API.
It has been made available as far back as Node 16 https://nodejs.org/dist/latest-v16.x/docs/api/util.html#util_util_parseargs_config

Alternatives

continue using a user land argument parser.

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Feb 24, 2023
@ChristianMurphy ChristianMurphy added 🧑 semver/major This is a change 🏡 area/internal This affects the hidden internals and removed 🤞 phase/open Post is being triaged manually labels Feb 24, 2023
@github-actions

This comment has been minimized.

@ChristianMurphy ChristianMurphy added 👶 area/size This affects (bundle) size 🤞 phase/open Post is being triaged manually 🦋 type/enhancement This is great to have labels Feb 24, 2023
@remcohaszing
Copy link
Member

parseArgs is great, but it is also experimental.

@pkgjs/parseargs is a nice polyfill.

@wooorm
Copy link
Member

wooorm commented Feb 24, 2023

nodejs/node#46718

@wooorm
Copy link
Member

wooorm commented Aug 9, 2023

What does it mean if something is stable on Node 20 but experimental on Node 16/lts and 18/lts? 😅
I guess we can use it then, right?

@remcohaszing
Copy link
Member

I guess we can use it then, right?

IMO yes

@ChristianMurphy
Copy link
Member Author

I guess we can use it then, right?

I'd lean Yes as well.

@wooorm
Copy link
Member

wooorm commented Aug 21, 2023

Spent a couple hours investigating. It’s not really bad. But it isn’t an improvement at all. It doesn’t lead to less code. I think one very important thing we currently have is negation: some stuff is default on, but with --no it is negated, so like --no-color. That doesn’t normally work. you can make it work when getting tokens back and with a bunch of extra.
Second is that it doesn’t have good errors. We currently have good errors. I’d need to regex Node’s errors to figure out which flags were incorrectly passed, parse them, and then figure out an improved error message.
Then, it doesn’t support things that can be both booleans and strings (our output).
Last, it handles combined short flags such as -on as -o n (--output n), because it doesn‘t accept it as a boolean, whereas we’d like to.

It’s not an improvement. It’s either a bare bones opinionated parser that throws programming exceptions if things are wrong, or it’s just a tokenizer.
For it to be useful, it would have to have a bunch more options, to make it smarter.

So perhaps in a couple years.

@remcohaszing
Copy link
Member

To be fair I always found the behaviour of -o a bit quirky. If remark . -o works, I expect remark -o . to work the same. In fact, I tend to run most CLIs with some-cli --options --go --first then positional arguments. I would rather have two options:

  -f  --fix                 overwrite input files
  -o  --output <path>       specify output location

@wooorm
Copy link
Member

wooorm commented Aug 21, 2023

Oh, wait. What? How’s that quirky? Seems normal to me. There are tons of CLIs where your expectation doesn‘t work

@wooorm
Copy link
Member

wooorm commented Aug 21, 2023

--fix is a name that implies things that are currently not yet done. Having it be syntax sugar for --output . seems, to me, as not much of an improvement?

@remcohaszing
Copy link
Member

I can’t really think of CLIs with similar behaviour that surprised me. I think the fact that this behaviour can’t be expressed with parseArgs, but neither with for example Python’s builtin argparse and I think bash’s getopts, shows that this behaviour is a bit unconventional.

Indeed --fix would be an alias for --output .. Likewise, putting --output after positional arguments or before another option makes it an alias for --output ..

Given this is command

remark -q -o . .

These do the same

remark -o . -q .
remark -o -q .
remark -q . -o
remark . -qo
remark . -oq
remark -oq .

But these behave differently

remark -q -o .
remark -qo .
remark -o . -q

For similar options ESLint and Stylelint use --fix. Prettier uses --write.

Alternatively --output could treat the input path as the output if no input is given.

@wooorm
Copy link
Member

wooorm commented Aug 22, 2023

This new post seems different from your earlier statement “If remark . -o works, I expect remark -o . to work the same.” That sounds like you want to throw on remark . -o, because from what I understand you don’t want to throw on remark -o .? 🤔 I don’t understand what you’re proposing, or what you’re trying to replace it other languages/tools?

@ChristianMurphy ChristianMurphy closed this as not planned Won't fix, can't repro, duplicate, stale Oct 6, 2023
@ChristianMurphy
Copy link
Member Author

Closing this as stalled

@github-actions

This comment has been minimized.

@ChristianMurphy ChristianMurphy added the 🙅 no/wontfix This is not (enough of) an issue for this project label Oct 6, 2023
@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏡 area/internal This affects the hidden internals 👶 area/size This affects (bundle) size 🙅 no/wontfix This is not (enough of) an issue for this project 👎 phase/no Post cannot or will not be acted on 🧑 semver/major This is a change 🦋 type/enhancement This is great to have
Development

No branches or pull requests

3 participants