From a4c76add565b9674ef6880de88013948c61a1ce5 Mon Sep 17 00:00:00 2001
From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com>
Date: Fri, 19 Jul 2024 11:59:04 -0700
Subject: [PATCH] fix(cli): Respect implied BYONM from DENO_FUTURE in `deno
 task` (#24652)

Regression from
https://github.com/denoland/deno/commit/04f9db5b2217fe06f88e76146aac6362ff0b0b86

Originally I thought to fix the issue in the PR we needed to explicitly
pass through the `node-modules-dir` flag, but after applying the correct
fix that david pointed out (setting `NPM_PROCESS_STATE`) that wasn't
necessary (or correct).

We had a test for deno task with BYONM, but it only tested with
`"unstable": ["byonm"]` in deno.json, so it didn't catch this.
---
 cli/npm/managed/resolvers/local.rs            |  8 +--
 cli/task_runner.rs                            |  8 +--
 .../npm/lifecycle_scripts/__test__.jsonc      | 13 ++++
 tests/specs/task/byonm/__test__.jsonc         | 68 ++++++++++++++-----
 4 files changed, 67 insertions(+), 30 deletions(-)

diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs
index 2240bf0c02..90a17b1572 100644
--- a/cli/npm/managed/resolvers/local.rs
+++ b/cli/npm/managed/resolvers/local.rs
@@ -282,12 +282,8 @@ 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 {
-      force_node_modules_dir: true,
-    }),
-  );
+  custom_commands
+    .insert("node".to_string(), Rc::new(crate::task_runner::NodeCommand));
 
   custom_commands.insert(
     "node-gyp".to_string(),
diff --git a/cli/task_runner.rs b/cli/task_runner.rs
index 7653594e5a..e8937590db 100644
--- a/cli/task_runner.rs
+++ b/cli/task_runner.rs
@@ -164,9 +164,7 @@ impl ShellCommand for NpmCommand {
   }
 }
 
-pub struct NodeCommand {
-  pub force_node_modules_dir: bool,
-}
+pub struct NodeCommand;
 
 impl ShellCommand for NodeCommand {
   fn execute(
@@ -193,9 +191,6 @@ 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;
@@ -308,7 +303,6 @@ 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/specs/npm/lifecycle_scripts/__test__.jsonc b/tests/specs/npm/lifecycle_scripts/__test__.jsonc
index b862b81260..302b40d1e3 100644
--- a/tests/specs/npm/lifecycle_scripts/__test__.jsonc
+++ b/tests/specs/npm/lifecycle_scripts/__test__.jsonc
@@ -132,6 +132,19 @@
           "output": "no_deno_json.out"
         }
       ]
+    },
+    "lifecycle_scripts_no_deno_json_conflicting_bin": {
+      "tempDir": true,
+      "steps": [
+        {
+          "args": ["eval", "Deno.removeSync('deno.json')"],
+          "output": ""
+        },
+        {
+          "args": "cache --allow-scripts --node-modules-dir=true conflicting_bin.js",
+          "output": "conflicting_bin.out"
+        }
+      ]
     }
   }
 }
diff --git a/tests/specs/task/byonm/__test__.jsonc b/tests/specs/task/byonm/__test__.jsonc
index 670f6767dd..3d62a7887d 100644
--- a/tests/specs/task/byonm/__test__.jsonc
+++ b/tests/specs/task/byonm/__test__.jsonc
@@ -1,20 +1,54 @@
 {
   "tempDir": true,
-  "steps": [{
-    "commandName": "npm",
-    "args": "install",
-    "output": "[WILDCARD]"
-  }, {
-    "args": "task say",
-    "output": "package_json_say.out"
-  }, {
-    "args": "task think",
-    "output": "package_json_think.out"
-  }, {
-    "args": "task deno-say",
-    "output": "deno_json_say.out"
-  }, {
-    "args": "task deno-think",
-    "output": "deno_json_think.out"
-  }]
+  "tests": {
+    "deno_json": {
+      "steps": [{
+        "commandName": "npm",
+        "args": "install",
+        "output": "[WILDCARD]"
+      }, {
+        "args": "task say",
+        "output": "package_json_say.out"
+      }, {
+        "args": "task think",
+        "output": "package_json_think.out"
+      }, {
+        "args": "task deno-say",
+        "output": "deno_json_say.out"
+      }, {
+        "args": "task deno-think",
+        "output": "deno_json_think.out"
+      }]
+    },
+    "no_deno_json": {
+      "steps": [{
+        "args": [
+          "eval",
+          "Deno.removeSync('deno.json')"
+        ],
+        "output": ""
+      }, {
+        "commandName": "npm",
+        "args": "install",
+        "output": "[WILDCARD]"
+      }, {
+        // implied byonm from DENO_FUTURE
+        "envs": {
+          "DENO_FUTURE": "1"
+        },
+        "args": "task say",
+        "output": "package_json_say.out"
+      }, {
+        // byonm flag
+        "args": "task --unstable-byonm say",
+        "output": "package_json_say.out"
+      }, {
+        "args": [
+          "eval",
+          "try { Deno.statSync('node_modules/.deno'); } catch (e) { if (e instanceof Deno.errors.NotFound) { console.log('good'); } else { throw new Error('bad'); } }"
+        ],
+        "output": "good\n"
+      }]
+    }
+  }
 }