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

Revert parseArgs change #11733

Merged
merged 2 commits into from
Aug 15, 2024
Merged

Revert parseArgs change #11733

merged 2 commits into from
Aug 15, 2024

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Aug 15, 2024

Changes

fix #11731

Did a manual revert by hand since there's some good refactors that were made in #11645:

  • Full revert of create-astro, @astrojs/db, and @astrojs/upgrade (uses args or yargs-parser)
  • Partial revert in astro (uses args-parser), some non-breaking refactors were kept.
  • Other internal scripts are kept using parseArgs as they don't affect us as much.

Explanation of problem

With our previous option, parseArgs parse things rather strangely:

pnpm dev --host --port 1234
=> values: { 'host': '--port' }, positionals: [..., '1234']

pnpm dev --port 1234 --host
=> values: { 'port': true, 'host': true }, positionals: [..., '1234']

pnpm dev --port 1234 --open --host
=> values: { 'port': true, 'open': '--host' }, positionals: [..., '1234']

pnpm dev --host --port
=> values: { 'host': '--port' }, positionals: [...]

I also tested enabling strict: true, but things get sillier since parseArgs doesn't support dual boolean/string arguments. I also feel like it'll cause more problems for the other packages so I reverted them all back. Why can't node design proper APIs :(

Testing

Tested manually. The cli.test.js should also pass.

Docs

n/a. bug fix.

Copy link

changeset-bot bot commented Aug 15, 2024

🦋 Changeset detected

Latest commit: eed0b49

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: create-astro Related to the `create-astro` package (scope) pkg: astro Related to the core `astro` package (scope) labels Aug 15, 2024
@bluwy bluwy merged commit 391324d into main Aug 15, 2024
14 checks passed
@bluwy bluwy deleted the revert-parseargs branch August 15, 2024 16:27
@astrobot-houston astrobot-houston mentioned this pull request Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pkg: create-astro Related to the `create-astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

context.site is undefined when using the "--site https://foo.com" CLI argument
3 participants