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(add): Better error message when missing npm specifier #24970

Merged
merged 2 commits into from
Aug 9, 2024

Conversation

nathanwhit
Copy link
Member

@nathanwhit nathanwhit commented Aug 9, 2024

Before:
Screenshot 2024-08-09 at 3 15 01 PM

After:
Screenshot 2024-08-09 at 3 52 15 PM

@marvinhagemeister
Copy link
Contributor

marvinhagemeister commented Aug 9, 2024

Nice, this is a great DX improvement! Wondering if we should we print deno add npm:ajv without the version as the command as that would match more what the user would type. I'm mainly worried that otherwise users would think that they'd need to provide the version arg themselves.

@nathanwhit
Copy link
Member Author

Nice, this is a great DX improvement! Wondering if we should we print deno add npm:ajv without the version as the command as that would match more what the user would type. I'm mainly worried that otherwise users would think that they'd need to provide the version arg themselves.

Good idea! I changed it so that we suggest whatever the user specified, but with npm:. So if you do

deno add ajv

we suggest

deno add npm:ajv

and if you do

deno add ajv@8

we suggest

deno add npm:ajv@8

@marvinhagemeister
Copy link
Contributor

Nice, this is a great DX improvement! Wondering if we should we print deno add npm:ajv without the version as the command as that would match more what the user would type. I'm mainly worried that otherwise users would think that they'd need to provide the version arg themselves.

Good idea! I changed it so that we suggest whatever the user specified, but with npm:. So if you do

deno add ajv

we suggest

deno add npm:ajv

and if you do

deno add ajv@8

we suggest

deno add npm:ajv@8

Perfect, this is even better!

@nathanwhit nathanwhit enabled auto-merge (squash) August 9, 2024 14:18
@nathanwhit nathanwhit merged commit 218ee1b into denoland:main Aug 9, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants