fix(ext/node): improve shelljs compat with managed npm execution (#24912)

This change improves the Node.js compatibility in managed npm resolution
mode by disabling the discovery of `node_modules` when the
main specifier is inside of `DENO_DIR`.

closes #22732
closes #24589

cli/args/mod.rs

@@ -75,6 +75,7 @@ use std::sync::Arc;
 use thiserror::Error;
 
 use crate::cache;
+use crate::cache::DenoDirProvider;
 use crate::file_fetcher::FileFetcher;
 use crate::util::fs::canonicalize_path_maybe_not_exists;
 use crate::version;
@@ -780,6 +781,7 @@ pub struct CliOptions {
   pub start_dir: Arc<WorkspaceDirectory>,
   pub disable_deprecated_api_warning: bool,
   pub verbose_deprecated_api_warning: bool,
+  pub deno_dir_provider: Arc<DenoDirProvider>,
 }
 
 impl CliOptions {
@@ -810,11 +812,14 @@ impl CliOptions {
 
     let maybe_lockfile = maybe_lockfile.filter(|_| !force_global_cache);
     let root_folder = start_dir.workspace.root_folder_configs();
+    let deno_dir_provider =
+      Arc::new(DenoDirProvider::new(flags.cache_path.clone()));
     let maybe_node_modules_folder = resolve_node_modules_folder(
       &initial_cwd,
       &flags,
       root_folder.deno_json.as_deref(),
       root_folder.pkg_json.as_deref(),
+      &deno_dir_provider,
     )
     .with_context(|| "Resolving node_modules folder.")?;
 
@@ -837,6 +842,7 @@ impl CliOptions {
       start_dir,
       disable_deprecated_api_warning,
       verbose_deprecated_api_warning,
+      deno_dir_provider,
     })
   }
 
@@ -1243,6 +1249,7 @@ impl CliOptions {
       overrides: self.overrides.clone(),
       disable_deprecated_api_warning: self.disable_deprecated_api_warning,
       verbose_deprecated_api_warning: self.verbose_deprecated_api_warning,
+      deno_dir_provider: self.deno_dir_provider.clone(),
     }
   }
 
@@ -1768,6 +1775,7 @@ fn resolve_node_modules_folder(
   flags: &Flags,
   maybe_config_file: Option<&ConfigFile>,
   maybe_package_json: Option<&PackageJson>,
+  deno_dir_provider: &Arc<DenoDirProvider>,
 ) -> Result<Option<PathBuf>, AnyError> {
   let use_node_modules_dir = flags
     .node_modules_dir
@@ -1779,7 +1787,18 @@ fn resolve_node_modules_folder(
   } else if let Some(state) = &*NPM_PROCESS_STATE {
     return Ok(state.local_node_modules_path.as_ref().map(PathBuf::from));
   } else if let Some(package_json_path) = maybe_package_json.map(|c| &c.path) {
-    // always auto-discover the local_node_modules_folder when a package.json exists
+    if let Ok(deno_dir) = deno_dir_provider.get_or_create() {
+      // `deno_dir.root` can be symlink in macOS
+      if let Ok(root) = canonicalize_path_maybe_not_exists(&deno_dir.root) {
+        if package_json_path.starts_with(root) {
+          // if the package.json is in deno_dir, then do not use node_modules
+          // next to it as local node_modules dir
+          return Ok(None);
+        }
+      }
+    }
+    // auto-discover the local_node_modules_folder when a package.json exists
+    // and it's not in deno_dir
     package_json_path.parent().unwrap().join("node_modules")
   } else if use_node_modules_dir.is_none() {
     return Ok(None);

cli/factory.rs

@@ -158,7 +158,6 @@ impl<T> Deferred<T> {
 #[derive(Default)]
 struct CliFactoryServices {
   cli_options: Deferred<Arc<CliOptions>>,
-  deno_dir_provider: Deferred<Arc<DenoDirProvider>>,
   caches: Deferred<Arc<Caches>>,
   file_fetcher: Deferred<Arc<FileFetcher>>,
   global_http_cache: Deferred<Arc<GlobalHttpCache>>,
@@ -236,11 +235,7 @@ impl CliFactory {
   }
 
   pub fn deno_dir_provider(&self) -> Result<&Arc<DenoDirProvider>, AnyError> {
-    self.services.deno_dir_provider.get_or_try_init(|| {
+    Ok(&self.cli_options()?.deno_dir_provider)
-      Ok(Arc::new(DenoDirProvider::new(
-        self.cli_options()?.maybe_custom_root().clone(),
-      )))
-    })
   }
 
   pub fn deno_dir(&self) -> Result<&DenoDir, AnyError> {

tests/registry/npm/@denotest/exec-file/1.0.0/exec-child.js

@@ -0,0 +1,2 @@
+const { Buffer } = require("buffer");
+console.log(Buffer.from("Hello, world!").toString());

tests/registry/npm/@denotest/exec-file/1.0.0/index.js

@@ -0,0 +1,6 @@
+const child_process = require('child_process');
+const path = require('path');
+
+const execArgs = [path.join(__dirname, "exec-child.js")]
+const buf = child_process.execFileSync(process.execPath, execArgs);
+console.log(buf.toString().trim());

tests/registry/npm/@denotest/exec-file/1.0.0/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "@denotest/exec-file",
+  "version": "1.0.0",
+  "main": "index.js"
+}

tests/specs/npm/exec_file_inside_npm_package/__test__.jsonc

@@ -0,0 +1,4 @@
+{
+  "args": "run -A main.ts",
+  "output": "main.out"
+}

tests/specs/npm/exec_file_inside_npm_package/main.out

@@ -0,0 +1 @@
+[WILDCARD]Hello, world!

tests/specs/npm/exec_file_inside_npm_package/main.ts

@@ -0,0 +1 @@
+import "npm:@denotest/exec-file@1.0.0";

tests/testdata/npm/exec_file/main.ts

@@ -0,0 +1 @@
+import "npm:@denotest/exec-file@1.0.0";