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

feat: experimental support for deno v2 #158

Merged
merged 10 commits into from
Nov 26, 2024
Merged

feat: experimental support for deno v2 #158

merged 10 commits into from
Nov 26, 2024

Conversation

yassilah
Copy link
Contributor

This PR adds support for deno. As Deno supports reading from package.json the only real differences are the following:

  • add a default npm: prefix to dependencies that do not already have one to align with the behavior of other package managers;
  • immediately detect deno as the package manager if a deno.json file is present in the root folder since deno is not a valid entry in the packageManager property of package.json (if no deno.json file is present but a deno.lock is, nypm is still able to detect it).

resolves #157.

@pi0 pi0 changed the title Add basic support for deno (>=2.0.0) feat: basic support for deno (v2) Oct 22, 2024
src/api.ts Outdated Show resolved Hide resolved
src/api.ts Outdated Show resolved Hide resolved
src/package-manager.ts Outdated Show resolved Hide resolved
src/package-manager.ts Outdated Show resolved Hide resolved
@@ -29,6 +29,7 @@ export async function installDependencies(
yarn: ["install", "--immutable"],
bun: ["install", "--frozen-lockfile"],
pnpm: ["install", "--frozen-lockfile"],
deno: ["install", "--frozen"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Deno doesn't run lifecycle scripts by default, should we include the --allow-scripts flag in the command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mhmm i genuinely don't know what's the best option here @pi0 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point however as per docs:

Note: Scripts will only be executed when using a node_modules directory (--node-modules-dir).

So I'm not sure either. I would say let's iterate and see if we need it in practice.

@pi0 pi0 changed the title feat: basic support for deno (v2) feat: experimental support for deno v2 Nov 26, 2024
Copy link

codecov bot commented Nov 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.96%. Comparing base (660392f) to head (d694f0e).
Report is 57 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #158       +/-   ##
===========================================
- Coverage   82.17%   64.96%   -17.21%     
===========================================
  Files           6        5        -1     
  Lines         516      491       -25     
  Branches       71       91       +20     
===========================================
- Hits          424      319      -105     
- Misses         91      169       +78     
- Partials        1        3        +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for nice work on this ❤️

@pi0 pi0 merged commit c7448e4 into unjs:main Nov 26, 2024
5 of 6 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.

Support Deno
4 participants