Skip to content

fix(npm): resolve bundled npm deps in packages properly when not using a node_modules directory#32679

Merged
dsherret merged 6 commits intodenoland:mainfrom
dsherret:fix_bundled_deps_resolution_global_install
Mar 13, 2026
Merged

fix(npm): resolve bundled npm deps in packages properly when not using a node_modules directory#32679
dsherret merged 6 commits intodenoland:mainfrom
dsherret:fix_bundled_deps_resolution_global_install

Conversation

@dsherret
Copy link
Copy Markdown
Contributor

We now attempt to find bundled dependencies at the start instead of on error. Previously we were doing it only on error... and sometimes it wouldn't error leading to bad resolution.

Closes #32529

));
ManagedNpmResolverCreateOptions {
sys,
sys: factory.node_resolution_sys.clone(),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As part of this change, I was worried it would slow things down, so I updated the global resolver to use the NodeResolutionSys, which has a built-in cache. It should be faster than before now, but I didn't measure it yet.

}
}

fn name_without_path(name: &str) -> &str {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed and consolidated.

}
}

pub fn join_package_name_to_path(path: &Path, package_name: &str) -> PathBuf {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed and consolidated. New method is the more efficient one that accepts a Cow<'_, Path> that can be pushed to when owned.

// first, as these should always take priority over global resolution
if let Some(bundled_folder) = self.resolve_bundled_dep(name, referrer)? {
return Ok(bundled_folder);
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Main fix is here. Notice we do this upfront rather than only on error.

@dsherret dsherret merged commit 9ab5c0c into denoland:main Mar 13, 2026
112 checks passed
@dsherret dsherret deleted the fix_bundled_deps_resolution_global_install branch March 13, 2026 11:44
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.

deno run -A npm:npm --version returns nothing

2 participants