Skip to content

Commit

Permalink
fix(install): Make sure target node_modules exists when symlinking (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
nathanwhit authored Sep 6, 2024
1 parent 51f5f57 commit 292344a
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 2 deletions.
15 changes: 13 additions & 2 deletions cli/npm/managed/resolvers/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::cmp::Ordering;
use std::collections::hash_map::Entry;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::collections::HashSet;
use std::fs;
use std::path::Path;
use std::path::PathBuf;
Expand Down Expand Up @@ -620,6 +621,9 @@ async fn sync_resolution_with_fs(

let mut found_names: HashMap<&String, &PackageNv> = HashMap::new();

// set of node_modules in workspace packages that we've already ensured exist
let mut existing_child_node_modules_dirs: HashSet<PathBuf> = HashSet::new();

// 4. Create symlinks for package json dependencies
{
for remote in npm_install_deps_provider.remote_pkgs() {
Expand Down Expand Up @@ -667,8 +671,15 @@ async fn sync_resolution_with_fs(
);
if install_in_child {
// symlink the dep into the package's child node_modules folder
let dest_path =
remote.base_dir.join("node_modules").join(&remote.alias);
let dest_node_modules = remote.base_dir.join("node_modules");
if !existing_child_node_modules_dirs.contains(&dest_node_modules) {
fs::create_dir_all(&dest_node_modules).with_context(|| {
format!("Creating '{}'", dest_node_modules.display())
})?;
existing_child_node_modules_dirs.insert(dest_node_modules.clone());
}
let mut dest_path = dest_node_modules;
dest_path.push(&remote.alias);

symlink_package_dir(&local_registry_package_path, &dest_path)?;
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Regression test for https://github.com/denoland/deno/issues/25493
//
// `deno install` where we need to create a `node_modules` for a workspace member
// due to conflicting package versions, but when we go to set it up, the
// symlink target path (`package2/node_modules/chalk`) has `node_modules` as its parent,
// and it doesn't exist yet
{
"tempDir": true,
"steps": [
{
"args": "install",
"output": "install.out"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"nodeModulesDir": "auto",
"workspace": [
"./package1",
"./package2"
]
}
22 changes: 22 additions & 0 deletions tests/specs/install/workspace_node_modules_not_exists/install.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[UNORDERED_START]
Download http://localhost:4260/chalk
Download http://localhost:4260/ansi-styles
Download http://localhost:4260/supports-color
Download http://localhost:4260/color-convert
Download http://localhost:4260/has-flag
Download http://localhost:4260/color-name
Download http://localhost:4260/chalk/chalk-4.1.2.tgz
Download http://localhost:4260/chalk/chalk-5.0.1.tgz
Download http://localhost:4260/supports-color/supports-color-7.2.0.tgz
Download http://localhost:4260/ansi-styles/ansi-styles-4.3.0.tgz
Download http://localhost:4260/has-flag/has-flag-4.0.0.tgz
Download http://localhost:4260/color-convert/color-convert-2.0.1.tgz
Download http://localhost:4260/color-name/color-name-1.1.4.tgz
Initialize supports-color@7.2.0
Initialize chalk@4.1.2
Initialize ansi-styles@4.3.0
Initialize color-name@1.1.4
Initialize color-convert@2.0.1
Initialize has-flag@4.0.0
Initialize chalk@5.0.1
[UNORDERED_END]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"chalk": "4.1.2"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"chalk": "5.0.1"
}
}

0 comments on commit 292344a

Please sign in to comment.