fix(node): Fix `--allow-scripts` with no `deno.json` (#24533)

We would resolve the wrong package.json, resulting in an inability to
run CJS (or other node-mode) scripts

10 files changed, 82 insertions, 20 deletions

diff --git a/cli/args/mod.rs b/cli/args/mod.rs
index aaa8185af..8b1b8e0c3 100644
--- a/cli/args/mod.rs
+++ b/cli/args/mod.rs
@@ -722,15 +722,15 @@ pub enum NpmProcessStateKind {
   Byonm,
 }
 
-const RESOLUTION_STATE_ENV_VAR_NAME: &str =
+pub(crate) const NPM_RESOLUTION_STATE_ENV_VAR_NAME: &str =
   "DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE";
 
 static NPM_PROCESS_STATE: Lazy<Option<NpmProcessState>> = Lazy::new(|| {
-  let state = std::env::var(RESOLUTION_STATE_ENV_VAR_NAME).ok()?;
+  let state = std::env::var(NPM_RESOLUTION_STATE_ENV_VAR_NAME).ok()?;
   let state: NpmProcessState = serde_json::from_str(&state).ok()?;
   // remove the environment variable so that sub processes
   // that are spawned do not also use this.
-  std::env::remove_var(RESOLUTION_STATE_ENV_VAR_NAME);
+  std::env::remove_var(NPM_RESOLUTION_STATE_ENV_VAR_NAME);
   Some(state)
 });
 
diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs
index 1020a57e9..a18ad4d7f 100644
--- a/cli/npm/managed/mod.rs
+++ b/cli/npm/managed/mod.rs
@@ -518,22 +518,25 @@ impl ManagedCliNpmResolver {
   }
 }
 
+fn npm_process_state(
+  snapshot: ValidSerializedNpmResolutionSnapshot,
+  node_modules_path: Option<&Path>,
+) -> String {
+  serde_json::to_string(&NpmProcessState {
+    kind: NpmProcessStateKind::Snapshot(snapshot.into_serialized()),
+    local_node_modules_path: node_modules_path
+      .map(|p| p.to_string_lossy().to_string()),
+  })
+  .unwrap()
+}
+
 impl NpmResolver for ManagedCliNpmResolver {
   /// Gets the state of npm for the process.
   fn get_npm_process_state(&self) -> String {
-    serde_json::to_string(&NpmProcessState {
-      kind: NpmProcessStateKind::Snapshot(
-        self
-          .resolution
-          .serialized_valid_snapshot()
-          .into_serialized(),
-      ),
-      local_node_modules_path: self
-        .fs_resolver
-        .node_modules_path()
-        .map(|p| p.to_string_lossy().to_string()),
-    })
-    .unwrap()
+    npm_process_state(
+      self.resolution.serialized_valid_snapshot(),
+      self.fs_resolver.node_modules_path().map(|p| p.as_path()),
+    )
   }
 
   fn resolve_package_folder_from_package(
diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs
index 937b2a899..cbd820320 100644
--- a/cli/npm/managed/resolvers/local.rs
+++ b/cli/npm/managed/resolvers/local.rs
@@ -282,8 +282,12 @@ fn resolve_baseline_custom_commands(
   custom_commands
     .insert("npm".to_string(), Rc::new(crate::task_runner::NpmCommand));
 
-  custom_commands
-    .insert("node".to_string(), Rc::new(crate::task_runner::NodeCommand));
+  custom_commands.insert(
+    "node".to_string(),
+    Rc::new(crate::task_runner::NodeCommand {
+      force_node_modules_dir: true,
+    }),
+  );
 
   custom_commands.insert(
     "node-gyp".to_string(),
@@ -687,7 +691,16 @@ async fn sync_resolution_with_fs(
       &deno_local_registry_dir,
     )?;
     let init_cwd = lifecycle_scripts.initial_cwd.as_deref().unwrap();
+    let process_state = crate::npm::managed::npm_process_state(
+      snapshot.as_valid_serialized(),
+      Some(root_node_modules_dir_path),
+    );
 
+    let mut env_vars = crate::task_runner::real_env_vars();
+    env_vars.insert(
+      crate::args::NPM_RESOLUTION_STATE_ENV_VAR_NAME.to_string(),
+      process_state,
+    );
     for (package, package_path, scripts_run_path) in packages_with_scripts {
       // add custom commands for binaries from the package's dependencies. this will take precedence over the
       // baseline commands, so if the package relies on a bin that conflicts with one higher in the dependency tree, the
@@ -710,7 +723,7 @@ async fn sync_resolution_with_fs(
               task_name: script_name,
               script,
               cwd: &package_path,
-              env_vars: crate::task_runner::real_env_vars(),
+              env_vars: env_vars.clone(),
               custom_commands: custom_commands.clone(),
               init_cwd,
               argv: &[],
diff --git a/cli/task_runner.rs b/cli/task_runner.rs
index e8937590d..7653594e5 100644
--- a/cli/task_runner.rs
+++ b/cli/task_runner.rs
@@ -164,7 +164,9 @@ impl ShellCommand for NpmCommand {
   }
 }
 
-pub struct NodeCommand;
+pub struct NodeCommand {
+  pub force_node_modules_dir: bool,
+}
 
 impl ShellCommand for NodeCommand {
   fn execute(
@@ -191,6 +193,9 @@ impl ShellCommand for NodeCommand {
       .execute(context);
     }
     args.extend(["run", "-A"].into_iter().map(|s| s.to_string()));
+    if self.force_node_modules_dir {
+      args.push("--node-modules-dir=true".to_string());
+    }
     args.extend(context.args.iter().cloned());
 
     let mut state = context.state;
@@ -303,6 +308,7 @@ impl ShellCommand for NodeModulesFileRunCommand {
     let mut args = vec![
       "run".to_string(),
       "--ext=js".to_string(),
+      "--node-modules-dir=true".to_string(),
       "-A".to_string(),
       self.path.to_string_lossy().to_string(),
     ];
diff --git a/tests/registry/npm/@denotest/lifecycle-scripts-cjs/1.0.0/package.json b/tests/registry/npm/@denotest/lifecycle-scripts-cjs/1.0.0/package.json
new file mode 100644
index 000000000..1924ddef8
--- /dev/null
+++ b/tests/registry/npm/@denotest/lifecycle-scripts-cjs/1.0.0/package.json
@@ -0,0 +1,11 @@
+{
+  "name": "@denotest/lifecycle-scripts-cjs",
+  "version": "1.0.0",
+  "scripts": {
+    "preinstall": "echo preinstall && node preinstall.js",
+    "install": "echo install && cli-cjs 'hello from install script'"
+  },
+  "dependencies": {
+    "@denotest/bin": "1.0.0"
+  }
+}
\ No newline at end of file
diff --git a/tests/registry/npm/@denotest/lifecycle-scripts-cjs/1.0.0/preinstall.js b/tests/registry/npm/@denotest/lifecycle-scripts-cjs/1.0.0/preinstall.js
new file mode 100644
index 000000000..b5511f990
--- /dev/null
+++ b/tests/registry/npm/@denotest/lifecycle-scripts-cjs/1.0.0/preinstall.js
@@ -0,0 +1,5 @@
+const inspect = require('util').inspect;
+
+inspect({
+  "preinstall": "script"
+});
\ No newline at end of file
diff --git a/tests/registry/npm/@denotest/node-lifecycle-scripts/1.0.0/preinstall.js b/tests/registry/npm/@denotest/node-lifecycle-scripts/1.0.0/preinstall.js
index e3a59c753..f375b5fb3 100644
--- a/tests/registry/npm/@denotest/node-lifecycle-scripts/1.0.0/preinstall.js
+++ b/tests/registry/npm/@denotest/node-lifecycle-scripts/1.0.0/preinstall.js
@@ -1,4 +1,5 @@
 if ("Deno" in globalThis && typeof globalThis.Deno === 'object') {
+  require('./helper.js');
   console.log('deno preinstall.js');
 } else {
   console.log('node preinstall.js');
diff --git a/tests/specs/npm/lifecycle_scripts/__test__.jsonc b/tests/specs/npm/lifecycle_scripts/__test__.jsonc
index 5639c83cd..b862b8126 100644
--- a/tests/specs/npm/lifecycle_scripts/__test__.jsonc
+++ b/tests/specs/npm/lifecycle_scripts/__test__.jsonc
@@ -119,6 +119,19 @@
           "output": ""
         }
       ]
+    },
+    "lifecycle_scripts_no_deno_json": {
+      "tempDir": true,
+      "steps": [
+        {
+          "args": ["eval", "Deno.removeSync('deno.json')"],
+          "output": ""
+        },
+        {
+          "args": "cache --allow-scripts --node-modules-dir=true no_deno_json.js",
+          "output": "no_deno_json.out"
+        }
+      ]
     }
   }
 }
diff --git a/tests/specs/npm/lifecycle_scripts/no_deno_json.js b/tests/specs/npm/lifecycle_scripts/no_deno_json.js
new file mode 100644
index 000000000..2f26eb7e5
--- /dev/null
+++ b/tests/specs/npm/lifecycle_scripts/no_deno_json.js
@@ -0,0 +1 @@
+import {} from "npm:@denotest/lifecycle-scripts-cjs@1.0.0";
diff --git a/tests/specs/npm/lifecycle_scripts/no_deno_json.out b/tests/specs/npm/lifecycle_scripts/no_deno_json.out
new file mode 100644
index 000000000..9609c0790
--- /dev/null
+++ b/tests/specs/npm/lifecycle_scripts/no_deno_json.out
@@ -0,0 +1,9 @@
+Download http://localhost:4260/@denotest/lifecycle-scripts-cjs
+Download http://localhost:4260/@denotest/bin
+Download http://localhost:4260/@denotest/lifecycle-scripts-cjs/1.0.0.tgz
+Download http://localhost:4260/@denotest/bin/1.0.0.tgz
+Initialize @denotest/lifecycle-scripts-cjs@1.0.0
+Initialize @denotest/bin@1.0.0
+preinstall
+install
+hello from install script