diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs index cbd820320a..2240bf0c02 100644 --- a/cli/npm/managed/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -7,9 +7,9 @@ mod bin_entries; use std::borrow::Cow; use std::cell::RefCell; 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; @@ -609,16 +609,81 @@ async fn sync_resolution_with_fs( } } - // 4. Create all the top level packages in the node_modules folder, which are symlinks. - // - // Symlink node_modules/ to - // node_modules/.deno//node_modules/ - let mut found_names = HashSet::new(); - let mut ids = snapshot.top_level_packages().collect::>(); + let mut found_names: HashMap<&String, &PackageNv> = HashMap::new(); + + // 4. Create symlinks for package json dependencies + { + for remote in pkg_json_deps_provider.remote_pkgs() { + let Some(remote_id) = snapshot + .resolve_best_package_id(&remote.req.name, &remote.req.version_req) + else { + continue; // skip, package not found + }; + let remote_pkg = snapshot.package_from_id(&remote_id).unwrap(); + let alias_clashes = remote.req.name != remote.alias + && newest_packages_by_name.contains_key(&remote.alias); + let install_in_child = { + // we'll install in the child if the alias is taken by another package, or + // if there's already a package with the same name but different version + // 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 + } + Entry::Vacant(entry) => { + entry.insert(&remote_pkg.id.nv); + alias_clashes + } + } + }; + let target_folder_name = get_package_folder_id_folder_name( + &remote_pkg.get_package_cache_folder_id(), + ); + let local_registry_package_path = join_package_name( + &deno_local_registry_dir + .join(&target_folder_name) + .join("node_modules"), + &remote_pkg.id.nv.name, + ); + 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); + + symlink_package_dir(&local_registry_package_path, &dest_path)?; + } else { + // symlink the package into `node_modules/` + if setup_cache + .insert_root_symlink(&remote_pkg.id.nv.name, &target_folder_name) + { + symlink_package_dir( + &local_registry_package_path, + &join_package_name(root_node_modules_dir_path, &remote.alias), + )?; + } + } + } + } + + // 5. Create symlinks for the remaining top level packages in the node_modules folder. + // (These may be present if they are not in the package.json dependencies, such as ) + // Symlink node_modules/.deno//node_modules/ to + // node_modules/ + let mut ids = snapshot + .top_level_packages() + .filter(|f| !found_names.contains_key(&f.nv.name)) + .collect::>(); ids.sort_by(|a, b| b.cmp(a)); // create determinism and only include the latest version for id in ids { - if !found_names.insert(&id.nv.name) { - continue; // skip, already handled + match found_names.entry(&id.nv.name) { + Entry::Occupied(_) => { + continue; // skip, already handled + } + Entry::Vacant(entry) => { + entry.insert(&id.nv); + } } let package = snapshot.package_from_id(id).unwrap(); let target_folder_name = @@ -638,11 +703,16 @@ async fn sync_resolution_with_fs( } } - // 5. Create a node_modules/.deno/node_modules/ directory with + // 6. Create a node_modules/.deno/node_modules/ directory with // the remaining packages for package in newest_packages_by_name.values() { - if !found_names.insert(&package.id.nv.name) { - continue; // skip, already handled + match found_names.entry(&package.id.nv.name) { + Entry::Occupied(_) => { + continue; // skip, already handled + } + Entry::Vacant(entry) => { + entry.insert(&package.id.nv); + } } let target_folder_name = @@ -663,13 +733,13 @@ async fn sync_resolution_with_fs( } } - // 6. Set up `node_modules/.bin` entries for packages that need it. + // 7. Set up `node_modules/.bin` entries for packages that need it. { let bin_entries = std::mem::take(&mut *bin_entries.borrow_mut()); bin_entries.finish(snapshot, &bin_node_modules_dir_path)?; } - // 7. Create symlinks for the workspace packages + // 8. Create symlinks for the workspace packages { // todo(#24419): this is not exactly correct because it should // install correctly for a workspace (potentially in sub directories), diff --git a/tests/registry/npm/@denotest/add/0.5.0/index.d.ts b/tests/registry/npm/@denotest/add/0.5.0/index.d.ts new file mode 100644 index 0000000000..0e1188e04a --- /dev/null +++ b/tests/registry/npm/@denotest/add/0.5.0/index.d.ts @@ -0,0 +1 @@ +export function sum(a: number, b: number): number; diff --git a/tests/registry/npm/@denotest/add/0.5.0/index.js b/tests/registry/npm/@denotest/add/0.5.0/index.js new file mode 100644 index 0000000000..e112dddf76 --- /dev/null +++ b/tests/registry/npm/@denotest/add/0.5.0/index.js @@ -0,0 +1 @@ +module.exports.sum = (a, b) => a + b; \ No newline at end of file diff --git a/tests/registry/npm/@denotest/add/0.5.0/package.json b/tests/registry/npm/@denotest/add/0.5.0/package.json new file mode 100644 index 0000000000..32573ca912 --- /dev/null +++ b/tests/registry/npm/@denotest/add/0.5.0/package.json @@ -0,0 +1,4 @@ +{ + "name": "@denotest/add", + "version": "0.5.0" +} diff --git a/tests/specs/npm/workspace_conflicting_dep/__test__.jsonc b/tests/specs/npm/workspace_conflicting_dep/__test__.jsonc new file mode 100644 index 0000000000..82d59906e6 --- /dev/null +++ b/tests/specs/npm/workspace_conflicting_dep/__test__.jsonc @@ -0,0 +1,24 @@ +{ + "tempDir": true, + "tests": { + "conflicting_deps": { + "envs": { + "DENO_FUTURE": "1" + }, + "steps": [ + { + "args": "install", + "output": "[WILDCARD]" + }, + { + "args": "run ./a/index.js", + "output": "1 + 2 = 3\n" + }, + { + "args": "run ./b/index.js", + "output": "1 + 2 = 3\n" + } + ] + } + } +} diff --git a/tests/specs/npm/workspace_conflicting_dep/a/index.js b/tests/specs/npm/workspace_conflicting_dep/a/index.js new file mode 100644 index 0000000000..2fb3069d8a --- /dev/null +++ b/tests/specs/npm/workspace_conflicting_dep/a/index.js @@ -0,0 +1,3 @@ +import { sum } from "@denotest/add"; + +console.log(`1 + 2 = ${sum(1, 2)}`); diff --git a/tests/specs/npm/workspace_conflicting_dep/a/package.json b/tests/specs/npm/workspace_conflicting_dep/a/package.json new file mode 100644 index 0000000000..48a6b4b5bc --- /dev/null +++ b/tests/specs/npm/workspace_conflicting_dep/a/package.json @@ -0,0 +1,7 @@ +{ + "name": "@denotest/a", + "version": "1.0.0", + "dependencies": { + "@denotest/add": "0.5.0" + } +} diff --git a/tests/specs/npm/workspace_conflicting_dep/b/index.js b/tests/specs/npm/workspace_conflicting_dep/b/index.js new file mode 100644 index 0000000000..5ef8d200f7 --- /dev/null +++ b/tests/specs/npm/workspace_conflicting_dep/b/index.js @@ -0,0 +1,3 @@ +import { add } from "@denotest/add"; + +console.log(`1 + 2 = ${add(1, 2)}`); diff --git a/tests/specs/npm/workspace_conflicting_dep/b/package.json b/tests/specs/npm/workspace_conflicting_dep/b/package.json new file mode 100644 index 0000000000..60b7afe675 --- /dev/null +++ b/tests/specs/npm/workspace_conflicting_dep/b/package.json @@ -0,0 +1,7 @@ +{ + "name": "@denotest/b", + "version": "1.0.0", + "dependencies": { + "@denotest/add": "1.0.0" + } +} diff --git a/tests/specs/npm/workspace_conflicting_dep/package.json b/tests/specs/npm/workspace_conflicting_dep/package.json new file mode 100644 index 0000000000..1a4c42ca3e --- /dev/null +++ b/tests/specs/npm/workspace_conflicting_dep/package.json @@ -0,0 +1,6 @@ +{ + "workspaces": [ + "./a", + "./b" + ] +} diff --git a/tests/specs/npm/workspace_dep_aliases/__test__.jsonc b/tests/specs/npm/workspace_dep_aliases/__test__.jsonc new file mode 100644 index 0000000000..82d59906e6 --- /dev/null +++ b/tests/specs/npm/workspace_dep_aliases/__test__.jsonc @@ -0,0 +1,24 @@ +{ + "tempDir": true, + "tests": { + "conflicting_deps": { + "envs": { + "DENO_FUTURE": "1" + }, + "steps": [ + { + "args": "install", + "output": "[WILDCARD]" + }, + { + "args": "run ./a/index.js", + "output": "1 + 2 = 3\n" + }, + { + "args": "run ./b/index.js", + "output": "1 + 2 = 3\n" + } + ] + } + } +} diff --git a/tests/specs/npm/workspace_dep_aliases/a/index.js b/tests/specs/npm/workspace_dep_aliases/a/index.js new file mode 100644 index 0000000000..4354706600 --- /dev/null +++ b/tests/specs/npm/workspace_dep_aliases/a/index.js @@ -0,0 +1,3 @@ +import { sum } from "sumPkg"; + +console.log(`1 + 2 = ${sum(1, 2)}`); diff --git a/tests/specs/npm/workspace_dep_aliases/a/package.json b/tests/specs/npm/workspace_dep_aliases/a/package.json new file mode 100644 index 0000000000..a8c6a04f4a --- /dev/null +++ b/tests/specs/npm/workspace_dep_aliases/a/package.json @@ -0,0 +1,7 @@ +{ + "name": "@denotest/a", + "version": "1.0.0", + "dependencies": { + "sumPkg": "npm:@denotest/add@0.5.0" + } +} diff --git a/tests/specs/npm/workspace_dep_aliases/b/index.js b/tests/specs/npm/workspace_dep_aliases/b/index.js new file mode 100644 index 0000000000..5645c0876f --- /dev/null +++ b/tests/specs/npm/workspace_dep_aliases/b/index.js @@ -0,0 +1,3 @@ +import { add } from "addPkg"; + +console.log(`1 + 2 = ${add(1, 2)}`); diff --git a/tests/specs/npm/workspace_dep_aliases/b/package.json b/tests/specs/npm/workspace_dep_aliases/b/package.json new file mode 100644 index 0000000000..9abb40985d --- /dev/null +++ b/tests/specs/npm/workspace_dep_aliases/b/package.json @@ -0,0 +1,7 @@ +{ + "name": "@denotest/b", + "version": "1.0.0", + "dependencies": { + "addPkg": "npm:@denotest/add@1.0.0" + } +} diff --git a/tests/specs/npm/workspace_dep_aliases/package.json b/tests/specs/npm/workspace_dep_aliases/package.json new file mode 100644 index 0000000000..1a4c42ca3e --- /dev/null +++ b/tests/specs/npm/workspace_dep_aliases/package.json @@ -0,0 +1,6 @@ +{ + "workspaces": [ + "./a", + "./b" + ] +}