From fd5de5d3906a34ac867a60e90960f582a545a866 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Fri, 1 Nov 2024 12:56:24 -0700 Subject: [PATCH 1/2] prefer package.json for npm defs if not farther --- cli/tools/registry/pm.rs | 22 +++++++++++++++++- .../package_json_and_deno_json/__test__.jsonc | 23 +++++++++++++++++++ .../subdir/deno.json | 4 ++++ .../package_json_and_deno_json/subdir/mod.ts | 0 .../subdir/prefer_if_closer_deno.json.out | 9 ++++++++ 5 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 tests/specs/add/package_json_and_deno_json/subdir/deno.json create mode 100644 tests/specs/add/package_json_and_deno_json/subdir/mod.ts create mode 100644 tests/specs/add/package_json_and_deno_json/subdir/prefer_if_closer_deno.json.out diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index d1be901d6779f0..57b3f5d08d5265 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -1,5 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; @@ -333,6 +334,14 @@ fn load_configs( Ok((cli_factory, npm_config, deno_config)) } +fn path_distance(a: &Path, b: &Path) -> usize { + let diff = pathdiff::diff_paths(a, b); + let Some(diff) = diff else { + return usize::MAX; + }; + diff.components().count() +} + pub async fn add( flags: Arc, add_flags: AddFlags, @@ -357,6 +366,17 @@ pub async fn add( } } + let start_dir = cli_factory.cli_options()?.start_dir.dir_path(); + let prefer_npm_config = match (npm_config.as_ref(), deno_config.as_ref()) { + (Some(npm), Some(deno)) => { + let npm_distance = path_distance(&npm.path, &start_dir); + let deno_distance = path_distance(&deno.path, &start_dir); + npm_distance <= deno_distance + } + (Some(_), None) => true, + (None, _) => false, + }; + let http_client = cli_factory.http_client_provider(); let deps_http_cache = cli_factory.global_http_cache()?; let mut deps_file_fetcher = FileFetcher::new( @@ -455,7 +475,7 @@ pub async fn add( selected_package.selected_version ); - if selected_package.package_name.starts_with("npm:") { + if selected_package.package_name.starts_with("npm:") && prefer_npm_config { if let Some(npm) = &mut npm_config { npm.add(selected_package, dev); } else { diff --git a/tests/specs/add/package_json_and_deno_json/__test__.jsonc b/tests/specs/add/package_json_and_deno_json/__test__.jsonc index 0beee02d156bdd..8d67a07c823e58 100644 --- a/tests/specs/add/package_json_and_deno_json/__test__.jsonc +++ b/tests/specs/add/package_json_and_deno_json/__test__.jsonc @@ -41,6 +41,29 @@ "output": "good\n" } ] + }, + "only_prefers_package_json_if_closer": { + "steps": [ + { + "cwd": "./subdir", + "args": "add npm:@denotest/esm-basic jsr:@denotest/add npm:@denotest/say-hello", + "output": "[WILDCARD]" + }, + { + "args": [ + "eval", + "console.log(Deno.readTextFileSync('package.json').trim())" + ], + "output": "{}\n" + }, + { + "args": [ + "eval", + "console.log(Deno.readTextFileSync('./subdir/deno.json').trim())" + ], + "output": "subdir/prefer_if_closer_deno.json.out" + } + ] } } } diff --git a/tests/specs/add/package_json_and_deno_json/subdir/deno.json b/tests/specs/add/package_json_and_deno_json/subdir/deno.json new file mode 100644 index 00000000000000..cc54bbd8a2931a --- /dev/null +++ b/tests/specs/add/package_json_and_deno_json/subdir/deno.json @@ -0,0 +1,4 @@ +{ + "name": "@test/subdir", + "exports": "./mod.ts" +} diff --git a/tests/specs/add/package_json_and_deno_json/subdir/mod.ts b/tests/specs/add/package_json_and_deno_json/subdir/mod.ts new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/specs/add/package_json_and_deno_json/subdir/prefer_if_closer_deno.json.out b/tests/specs/add/package_json_and_deno_json/subdir/prefer_if_closer_deno.json.out new file mode 100644 index 00000000000000..8a5819d5039902 --- /dev/null +++ b/tests/specs/add/package_json_and_deno_json/subdir/prefer_if_closer_deno.json.out @@ -0,0 +1,9 @@ +{ + "name": "@test/subdir", + "exports": "./mod.ts", + "imports": { + "@denotest/add": "jsr:@denotest/add@^1.0.0", + "@denotest/esm-basic": "npm:@denotest/esm-basic@^1.0.0", + "@denotest/say-hello": "npm:@denotest/say-hello@^1.0.0" + } +} From fb63b7107bc78da265698c4a06c4a1ac15c2c6a9 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Fri, 1 Nov 2024 14:20:11 -0700 Subject: [PATCH 2/2] add comment --- cli/tools/registry/pm.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index 57b3f5d08d5265..4e038799833916 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -367,6 +367,10 @@ pub async fn add( } let start_dir = cli_factory.cli_options()?.start_dir.dir_path(); + + // only prefer to add npm deps to `package.json` if there isn't a closer deno.json. + // example: if deno.json is in the CWD and package.json is in the parent, we should add + // npm deps to deno.json, since it's closer let prefer_npm_config = match (npm_config.as_ref(), deno_config.as_ref()) { (Some(npm), Some(deno)) => { let npm_distance = path_distance(&npm.path, &start_dir);