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

Fix cli package resolution for @astrojs/db #10338

Merged
merged 3 commits into from
Mar 6, 2024

Conversation

BryceRussell
Copy link
Member

@BryceRussell BryceRussell commented Mar 6, 2024

Changes

Fixes bug where every time a astro db command is used it prompts the user to add @astrojs/db even if it is already installed

Testing

No tests, it is a pretty small fix for an error that doesn't get surfaced because it is inside a try/catch

Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data, and node are supported by the default ESM loader. On Windows, absolute paths must be valid file:https:// URLs. Received protocol 'c:'

Docs

No docs needed

Copy link

changeset-bot bot commented Mar 6, 2024

⚠️ No Changeset found

Latest commit: 984b50c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Mar 6, 2024
@BryceRussell
Copy link
Member Author

BryceRussell commented Mar 6, 2024

This was mentioned in Discord (as a reply to a triage/PR message for this issue):

We are addressing this! Let me guess, you’re on windows and use an Astro.config.ts instead of mjs?

Not sure if this relates to this issue or another one, maybe the fix for this is more complicated? I will close this if I hear anything else about the issue being fixed

@bluwy
Copy link
Member

bluwy commented Mar 6, 2024

That is being fixed at #10342. It looks like this is a bit different than that though.

I was waiting for someone more familiar with db before merging this, just in case, but seems like this is a simple fix enough. EDIT: nvm Matthew just approved.

@BryceRussell
Copy link
Member Author

Now that this has been approved, is this waiting on me to merge it? Sorry I haven't made a PR in a long time and I just want to make sure I am not blocking this

@bluwy
Copy link
Member

bluwy commented Mar 6, 2024

You can merge but no I didn't expect that and it wasn't blocking the PR. I saw Matthew approved and I thought he was gonna merge it. I'm gonna merge this now 😄

@bluwy bluwy merged commit 74cf1af into withastro:main Mar 6, 2024
13 checks passed
@BryceRussell
Copy link
Member Author

Awesome thanks!

@BryceRussell BryceRussell deleted the fix-db-cli-check branch May 8, 2024 14:19
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants