Recommend `npm:` specifiers in Deno

octokit / octokit.js

The all-batteries-included GitHub SDK for Browsers, Node.js, and Deno.

MIT License

6.97k stars 1.02k forks source link

Recommend `npm:` specifiers in Deno #2575

Closed mxdvl closed 10 months ago

mxdvl commented 10 months ago

Before the change?

Use esm.sh for importing NPM modules in Deno

After the change?

Recommend using npm: specifiers in Deno, as this is the easiest way to use NPM packages in Deno, and works well with lockfiles: https://docs.deno.com/runtime/manual/node/

Pull request checklist

[ ] Tests for the changes have been added (for bug fixes / features)
[X] Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

[ ] Yes
[X] No

gr2m commented 10 months ago

I'm not sure if this is better though? I know Deno continuously improves its support for npm, but I think they still recommend the web standards way? Or did they change their standpoint? Or do you think we should recommend to use npm with Deno to take advantage of the lockfile?

mxdvl commented 10 months ago

did they change their standpoint?

The docs now recommend this as the first option, but it's true that the other option still works fine if you ignore the lockfile. I've reached out in the Deno Discord to see what would be the current stance.

Or do you think we should recommend to use npm with Deno to take advantage of the lockfile?

This has come to my attention as we were maintaining libraries at @guardian and couldn't get CI to pass with slightly cryptic errors mentioning mismatches in file contents. I'd imagine CI or running from fresh cache is a common use case for Octokit in Deno.

mxdvl commented 10 months ago

@gr2m it seems like the consensus is that this should be up to personal preference. Having worked with Octokit in Deno for 18 months, I have found npm: specifiers to be much more reliable, since their introduction a year ago: https://deno.com/blog/v1.28

Which is to say, if you close this PR that's entirely fine.

mxdvl commented 10 months ago

Actually, I’ve realised that the npm: specifier version lacks proper typing and Octokit ends up as any, so it would be unwise to recommend it…