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(byonm): resolve npm deps of jsr deps #25399

Merged
merged 22 commits into from
Sep 4, 2024

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Sep 3, 2024

This allows using npm deps of jsr deps without having to add them to the root package.json.

Works by taking the package requirement and scanning the node_modules/.deno directory for the best matching package, so it relies on deno's node_modules structure.

Additionally to make the transition from package.json to deno.json easier, Deno now:

  1. Installs npm deps in a deno.json at the same time as installing npm deps from a package.json.
  2. Uses the alias in the import map for node_modules/<alias> for better package.json compatiblity.

@dsherret dsherret added the ci-draft Run the CI on draft PRs. label Sep 3, 2024
@dsherret dsherret force-pushed the fix_install_npm_deps_of_jsr_deps branch from 27a8128 to 3346d06 Compare September 4, 2024 10:35
@dsherret dsherret marked this pull request as ready for review September 4, 2024 10:38
@dsherret dsherret removed the ci-draft Run the CI on draft PRs. label Sep 4, 2024
cli/npm/byonm.rs Outdated Show resolved Hide resolved
@dsherret dsherret enabled auto-merge (squash) September 4, 2024 12:54
@dsherret dsherret merged commit c6d1b0a into denoland:main Sep 4, 2024
17 checks passed
@dsherret dsherret deleted the fix_install_npm_deps_of_jsr_deps branch September 4, 2024 14:08
dsherret added a commit that referenced this pull request Sep 9, 2024
This was initially added in #25399 in order to make transitioning over
from package.json to deno.json more easy, but it causes some problems
that are shown in the issue and it also means that the output of `deno
install` would have different resolution than `npm install`. Overall, I
think it's too much complexity to be smarter about this and it's
probably best to not do it. If someone needs an aliased folder then they
should keep using a package.json

Closes #25538
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.

2 participants