From 105d27bc7db5c0d2fd18cb26f41bd3193be74639 Mon Sep 17 00:00:00 2001 From: Yoshiya Hinosawa Date: Fri, 16 Aug 2024 12:48:48 +0900 Subject: [PATCH] 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 | 21 ++++++++++++++++++- cli/factory.rs | 7 +------ .../@denotest/exec-file/1.0.0/exec-child.js | 2 ++ .../npm/@denotest/exec-file/1.0.0/index.js | 6 ++++++ .../@denotest/exec-file/1.0.0/package.json | 5 +++++ .../__test__.jsonc | 4 ++++ .../npm/exec_file_inside_npm_package/main.out | 1 + .../npm/exec_file_inside_npm_package/main.ts | 1 + tests/testdata/npm/exec_file/main.ts | 1 + 9 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 tests/registry/npm/@denotest/exec-file/1.0.0/exec-child.js create mode 100644 tests/registry/npm/@denotest/exec-file/1.0.0/index.js create mode 100644 tests/registry/npm/@denotest/exec-file/1.0.0/package.json create mode 100644 tests/specs/npm/exec_file_inside_npm_package/__test__.jsonc create mode 100644 tests/specs/npm/exec_file_inside_npm_package/main.out create mode 100644 tests/specs/npm/exec_file_inside_npm_package/main.ts create mode 100644 tests/testdata/npm/exec_file/main.ts diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 696d0e5606..c347ab6fd9 100644 --- a/cli/args/mod.rs +++ b/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, pub disable_deprecated_api_warning: bool, pub verbose_deprecated_api_warning: bool, + pub deno_dir_provider: Arc, } 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, ) -> Result, 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); diff --git a/cli/factory.rs b/cli/factory.rs index 942aefd25a..25f6afa749 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -158,7 +158,6 @@ impl Deferred { #[derive(Default)] struct CliFactoryServices { cli_options: Deferred>, - deno_dir_provider: Deferred>, caches: Deferred>, file_fetcher: Deferred>, global_http_cache: Deferred>, @@ -236,11 +235,7 @@ impl CliFactory { } pub fn deno_dir_provider(&self) -> Result<&Arc, AnyError> { - self.services.deno_dir_provider.get_or_try_init(|| { - Ok(Arc::new(DenoDirProvider::new( - self.cli_options()?.maybe_custom_root().clone(), - ))) - }) + Ok(&self.cli_options()?.deno_dir_provider) } pub fn deno_dir(&self) -> Result<&DenoDir, AnyError> { diff --git a/tests/registry/npm/@denotest/exec-file/1.0.0/exec-child.js b/tests/registry/npm/@denotest/exec-file/1.0.0/exec-child.js new file mode 100644 index 0000000000..13ad51ca2b --- /dev/null +++ b/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()); diff --git a/tests/registry/npm/@denotest/exec-file/1.0.0/index.js b/tests/registry/npm/@denotest/exec-file/1.0.0/index.js new file mode 100644 index 0000000000..0129483aa6 --- /dev/null +++ b/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()); diff --git a/tests/registry/npm/@denotest/exec-file/1.0.0/package.json b/tests/registry/npm/@denotest/exec-file/1.0.0/package.json new file mode 100644 index 0000000000..50f0139860 --- /dev/null +++ b/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" +} diff --git a/tests/specs/npm/exec_file_inside_npm_package/__test__.jsonc b/tests/specs/npm/exec_file_inside_npm_package/__test__.jsonc new file mode 100644 index 0000000000..f816bad869 --- /dev/null +++ b/tests/specs/npm/exec_file_inside_npm_package/__test__.jsonc @@ -0,0 +1,4 @@ +{ + "args": "run -A main.ts", + "output": "main.out" +} diff --git a/tests/specs/npm/exec_file_inside_npm_package/main.out b/tests/specs/npm/exec_file_inside_npm_package/main.out new file mode 100644 index 0000000000..b0e5c237a5 --- /dev/null +++ b/tests/specs/npm/exec_file_inside_npm_package/main.out @@ -0,0 +1 @@ +[WILDCARD]Hello, world! diff --git a/tests/specs/npm/exec_file_inside_npm_package/main.ts b/tests/specs/npm/exec_file_inside_npm_package/main.ts new file mode 100644 index 0000000000..8a90b3a349 --- /dev/null +++ b/tests/specs/npm/exec_file_inside_npm_package/main.ts @@ -0,0 +1 @@ +import "npm:@denotest/exec-file@1.0.0"; diff --git a/tests/testdata/npm/exec_file/main.ts b/tests/testdata/npm/exec_file/main.ts new file mode 100644 index 0000000000..8a90b3a349 --- /dev/null +++ b/tests/testdata/npm/exec_file/main.ts @@ -0,0 +1 @@ +import "npm:@denotest/exec-file@1.0.0";