-
Notifications
You must be signed in to change notification settings - Fork 342
Add docs for dev imports #1754
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
Add docs for dev imports #1754
Conversation
| However, it can be useful to mark dev dependencies to aid people who are reading | ||
| your package. When using `deno.json`, the convention is to add a `// dev` | ||
| comment after any "dev only" dependency: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, this requires you to rename deno.json to deno.jsonc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deno supports reading jsonc even in deno.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah but in Fresh we read deno.json to check if jsx is set correctly and stuff and we assume that .json means json and .jsonc means jsonc. There is probably more software out there with that assumption
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be good to update fresh to just always assume jsonc?
dsherret
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| [runtime will only load and install dependencies that are actually used in the | ||
| code that is being executed](#why-does-deno-not-have-a-devimports-field). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't exactly true because for deno install it will install all the dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is clarified in the last paragraph of "Why does Deno not have a devImports field?"
| However, it can be useful to mark dev dependencies to aid people who are reading | ||
| your package. When using `deno.json`, the convention is to add a `// dev` | ||
| comment after any "dev only" dependency: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be good to update fresh to just always assume jsonc?
| { | ||
| "imports": { | ||
| "@std/fs": "jsr:@std/fs@1", | ||
| "@std/testing": "jsr:@std/testing@1" // dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a fan :P
No description provided.