fix(add): only add npm deps to package.json if it's at least as close…

… as deno.json (#26683) Fixes #26653
denoland · Nov 5, 2024 · 01c4a2e · 01c4a2e
1 parent c588770
commit 01c4a2e
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 1 deletion.
diff --git 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<Flags>,
   add_flags: AddFlags,
@@ -357,6 +366,21 @@ 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);
+      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 +479,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
@@ -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
@@ -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
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
@@ -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"
+  }
+}