diff options
13 files changed, 24 insertions, 88 deletions
diff --git a/cli/args/package_json.rs b/cli/args/package_json.rs index b9f0919d5..2529e54fd 100644 --- a/cli/args/package_json.rs +++ b/cli/args/package_json.rs @@ -1,6 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use std::collections::HashSet; use std::path::PathBuf; use std::sync::Arc; @@ -10,18 +9,16 @@ use deno_package_json::PackageJsonDepValue; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; -use crate::util::path::is_banned_path_char; - #[derive(Debug)] pub struct InstallNpmRemotePkg { - pub alias: String, + pub alias: Option<String>, pub base_dir: PathBuf, pub req: PackageReq, } #[derive(Debug)] pub struct InstallNpmWorkspacePkg { - pub alias: String, + pub alias: Option<String>, pub target_dir: PathBuf, } @@ -43,16 +40,13 @@ impl NpmInstallDepsProvider { let workspace_npm_pkgs = workspace.npm_packages(); for (_, folder) in workspace.config_folders() { - let mut deno_json_aliases = HashSet::new(); - // deal with the deno.json first because it takes precedence during resolution if let Some(deno_json) = &folder.deno_json { // don't bother with externally referenced import maps as users // should inline their import map to get this behaviour if let Some(serde_json::Value::Object(obj)) = &deno_json.json.imports { - deno_json_aliases.reserve(obj.len()); let mut pkg_pkgs = Vec::with_capacity(obj.len()); - for (alias, value) in obj { + for (_alias, value) in obj { let serde_json::Value::String(specifier) = value else { continue; }; @@ -60,11 +54,6 @@ impl NpmInstallDepsProvider { else { continue; }; - // skip any aliases with banned characters - if alias.chars().any(|c| c == '\\' || is_banned_path_char(c)) { - continue; - } - deno_json_aliases.insert(alias.to_lowercase()); let pkg_req = npm_req_ref.into_inner().req; let workspace_pkg = workspace_npm_pkgs .iter() @@ -72,12 +61,12 @@ impl NpmInstallDepsProvider { if let Some(pkg) = workspace_pkg { workspace_pkgs.push(InstallNpmWorkspacePkg { - alias: alias.to_string(), + alias: None, target_dir: pkg.pkg_json.dir_path().to_path_buf(), }); } else { pkg_pkgs.push(InstallNpmRemotePkg { - alias: alias.to_string(), + alias: None, base_dir: deno_json.dir_path(), req: pkg_req, }); @@ -85,7 +74,7 @@ impl NpmInstallDepsProvider { } // sort within each package (more like npm resolution) - pkg_pkgs.sort_by(|a, b| a.alias.cmp(&b.alias)); + pkg_pkgs.sort_by(|a, b| a.req.cmp(&b.req)); remote_pkgs.extend(pkg_pkgs); } } @@ -97,11 +86,6 @@ impl NpmInstallDepsProvider { let Ok(dep) = dep else { continue; }; - if deno_json_aliases.contains(&alias.to_lowercase()) { - // aliases in deno.json take precedence over package.json, so - // since this can't be resolved don't bother installing it - continue; - } match dep { PackageJsonDepValue::Req(pkg_req) => { let workspace_pkg = workspace_npm_pkgs.iter().find(|pkg| { @@ -112,12 +96,12 @@ impl NpmInstallDepsProvider { if let Some(pkg) = workspace_pkg { workspace_pkgs.push(InstallNpmWorkspacePkg { - alias, + alias: Some(alias), target_dir: pkg.pkg_json.dir_path().to_path_buf(), }); } else { pkg_pkgs.push(InstallNpmRemotePkg { - alias, + alias: Some(alias), base_dir: pkg_json.dir_path().to_path_buf(), req: pkg_req, }); @@ -128,7 +112,7 @@ impl NpmInstallDepsProvider { pkg.matches_name_and_version_req(&alias, &version_req) }) { workspace_pkgs.push(InstallNpmWorkspacePkg { - alias, + alias: Some(alias), target_dir: pkg.pkg_json.dir_path().to_path_buf(), }); } diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs index d0fd523e2..472bf2a47 100644 --- a/cli/npm/managed/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -642,13 +642,16 @@ async fn sync_resolution_with_fs( } else { continue; // skip, package not found }; - let alias_clashes = remote.req.name != remote.alias - && newest_packages_by_name.contains_key(&remote.alias); + let Some(remote_alias) = &remote.alias else { + continue; + }; + 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) { + 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) @@ -679,7 +682,7 @@ async fn sync_resolution_with_fs( existing_child_node_modules_dirs.insert(dest_node_modules.clone()); } let mut dest_path = dest_node_modules; - dest_path.push(&remote.alias); + dest_path.push(remote_alias); symlink_package_dir(&local_registry_package_path, &dest_path)?; } else { @@ -689,7 +692,7 @@ async fn sync_resolution_with_fs( { symlink_package_dir( &local_registry_package_path, - &join_package_name(root_node_modules_dir_path, &remote.alias), + &join_package_name(root_node_modules_dir_path, remote_alias), )?; } } @@ -774,9 +777,12 @@ async fn sync_resolution_with_fs( // install correctly for a workspace (potentially in sub directories), // but this is good enough for a first pass for workspace in npm_install_deps_provider.workspace_pkgs() { + let Some(workspace_alias) = &workspace.alias else { + continue; + }; symlink_package_dir( &workspace.target_dir, - &root_node_modules_dir_path.join(&workspace.alias), + &root_node_modules_dir_path.join(workspace_alias), )?; } } diff --git a/tests/specs/install/alias_deno_json/__test__.jsonc b/tests/specs/install/alias_deno_json/__test__.jsonc deleted file mode 100644 index 0398e2eeb..000000000 --- a/tests/specs/install/alias_deno_json/__test__.jsonc +++ /dev/null @@ -1,10 +0,0 @@ -{ - "tempDir": true, - "steps": [{ - "args": "install", - "output": "[WILDCARD]" - }, { - "args": "run --allow-read=. verify.ts", - "output": "true\n" - }] -} diff --git a/tests/specs/install/alias_deno_json/deno.json b/tests/specs/install/alias_deno_json/deno.json deleted file mode 100644 index a1adfb35e..000000000 --- a/tests/specs/install/alias_deno_json/deno.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "imports": { - "alias": "npm:@denotest/add" - } -} diff --git a/tests/specs/install/alias_deno_json/package.json b/tests/specs/install/alias_deno_json/package.json deleted file mode 100644 index 2c63c0851..000000000 --- a/tests/specs/install/alias_deno_json/package.json +++ /dev/null @@ -1,2 +0,0 @@ -{ -} diff --git a/tests/specs/install/alias_deno_json/verify.ts b/tests/specs/install/alias_deno_json/verify.ts deleted file mode 100644 index ea6cadb70..000000000 --- a/tests/specs/install/alias_deno_json/verify.ts +++ /dev/null @@ -1,2 +0,0 @@ -const stat = Deno.statSync(new URL("./node_modules/alias", import.meta.url)); -console.log(stat.isDirectory); diff --git a/tests/specs/install/alias_invalid_path_char/__test__.jsonc b/tests/specs/install/alias_invalid_path_char/__test__.jsonc deleted file mode 100644 index 4a3058635..000000000 --- a/tests/specs/install/alias_invalid_path_char/__test__.jsonc +++ /dev/null @@ -1,10 +0,0 @@ -{ - "tempDir": true, - "steps": [{ - "args": "install", - "output": "[WILDCARD]" - }, { - "args": "run --allow-read=. verify.ts", - "output": ".bin\n.deno\n@denotest\n" - }] -} diff --git a/tests/specs/install/alias_invalid_path_char/deno.jsonc b/tests/specs/install/alias_invalid_path_char/deno.jsonc deleted file mode 100644 index 85befb55e..000000000 --- a/tests/specs/install/alias_invalid_path_char/deno.jsonc +++ /dev/null @@ -1,7 +0,0 @@ -{ - "imports": { - // alias*test is an invalid path char on windows, so - // don't create an alias for this - "alias*test": "npm:@denotest/add" - } -} diff --git a/tests/specs/install/alias_invalid_path_char/package.json b/tests/specs/install/alias_invalid_path_char/package.json deleted file mode 100644 index 2c63c0851..000000000 --- a/tests/specs/install/alias_invalid_path_char/package.json +++ /dev/null @@ -1,2 +0,0 @@ -{ -} diff --git a/tests/specs/install/alias_invalid_path_char/verify.ts b/tests/specs/install/alias_invalid_path_char/verify.ts deleted file mode 100644 index 497e1d8dd..000000000 --- a/tests/specs/install/alias_invalid_path_char/verify.ts +++ /dev/null @@ -1,10 +0,0 @@ -const entries = Array.from( - Deno.readDirSync(new URL("./node_modules", import.meta.url)), -); -const names = entries.map((entry) => entry.name); -names.sort(); - -// won't have the invalid path alias -for (const name of names) { - console.log(name); -} diff --git a/tests/specs/install/alias_pkg_json_and_deno_json_npm_pkg/verify.out b/tests/specs/install/alias_pkg_json_and_deno_json_npm_pkg/verify.out index bc6cb31f7..8c989abd4 100644 --- a/tests/specs/install/alias_pkg_json_and_deno_json_npm_pkg/verify.out +++ b/tests/specs/install/alias_pkg_json_and_deno_json_npm_pkg/verify.out @@ -1,4 +1,4 @@ -@denotest/add +@denotest/esm-basic [Module: null prototype] { add: [Function (anonymous)], default: { add: [Function (anonymous)] } diff --git a/tests/specs/workspaces/nested_deno_pkg_npm_conflict/__test__.jsonc b/tests/specs/workspaces/nested_deno_pkg_npm_conflict/__test__.jsonc index 1bad68fd4..d0d7579b8 100644 --- a/tests/specs/workspaces/nested_deno_pkg_npm_conflict/__test__.jsonc +++ b/tests/specs/workspaces/nested_deno_pkg_npm_conflict/__test__.jsonc @@ -5,9 +5,9 @@ "output": "[WILDCARD]" }, { "args": "run --allow-read main.js", - "output": "4\n0.5.0\n" + "output": "4\n1.0.0\n" }, { "args": "run --allow-read member/main.js", - "output": "3\n1.0.0\n" + "output": "3\n" }] } diff --git a/tests/specs/workspaces/nested_deno_pkg_npm_conflict/member/main.js b/tests/specs/workspaces/nested_deno_pkg_npm_conflict/member/main.js index 0f64cf553..1ca631410 100644 --- a/tests/specs/workspaces/nested_deno_pkg_npm_conflict/member/main.js +++ b/tests/specs/workspaces/nested_deno_pkg_npm_conflict/member/main.js @@ -1,9 +1,3 @@ import { add } from "@denotest/add"; console.log(add(1, 2)); - -console.log( - JSON.parse(Deno.readTextFileSync( - new URL("node_modules/@denotest/add/package.json", import.meta.url), - )).version, -); |