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

Recommend npm: specifiers in Deno #2575

Closed
wants to merge 1 commit into from
Closed

Conversation

mxdvl
Copy link

@mxdvl mxdvl commented Nov 14, 2023


Before the change?

  • Use esm.sh for importing NPM modules in Deno

After the change?

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • 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
  • No

This is the easiest way to use NPM packages
in Deno, and works well with lockfiles:

https://docs.deno.com/runtime/manual/node/
@gr2m
Copy link
Contributor

gr2m commented Nov 14, 2023

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
Copy link
Author

mxdvl commented Nov 14, 2023

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
Copy link
Author

mxdvl commented Nov 15, 2023

@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
Copy link
Author

mxdvl commented Nov 15, 2023

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…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants