Skip to content

Commit

Permalink
fix(install): compare versions directly to decide whether to create a…
Browse files Browse the repository at this point in the history
… child node_modules dir for a workspace member (#26001)

Fixes #25861.

Previously we were attempting to match the version requirement against
the version already present in `node_modules` root, and if they didn't
match we would create a node_modules dir in the workspace member's
directory with the dependency.

Aside from the fact that this caused the panic, on second thought it
just doesn't make sense in general. We shouldn't be semver matching, as
resolution has already occurred and decided what package versions are
required. Instead, we can just compare the versions directly.
  • Loading branch information
nathanwhit authored Oct 2, 2024
1 parent cac28b5 commit cb74975
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 3 deletions.
6 changes: 3 additions & 3 deletions cli/npm/managed/resolvers/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,9 @@ async fn sync_resolution_with_fs(
// linked into the root
match found_names.entry(remote_alias) {
Entry::Occupied(nv) => {
alias_clashes
|| remote.req.name != nv.get().name // alias to a different package (in case of duplicate aliases)
|| !remote.req.version_req.matches(&nv.get().version) // incompatible version
// alias to a different package (in case of duplicate aliases)
// or the version doesn't match the version in the root node_modules
alias_clashes || &remote_pkg.id.nv != *nv.get()
}
Entry::Vacant(entry) => {
entry.insert(&remote_pkg.id.nv);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"tempDir": true,
"steps": [
{
"args": "install",
"output": "install.out"
}
]
}
3 changes: 3 additions & 0 deletions tests/specs/install/workspace_member_with_tag_dep/install.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Download http://localhost:4260/@denotest/esm-basic
Download http://localhost:4260/@denotest/esm-basic/1.0.0.tgz
Initialize @denotest/[email protected]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"dependencies": {
"@denotest/esm-basic": "latest"
},
"workspaces": ["package1"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"@denotest/esm-basic": "latest"
}
}

0 comments on commit cb74975

Please sign in to comment.