Repository: denoland/deno
Pull request: fix(cli): Respect implied BYONM from DENO_FUTURE in `deno task`

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.

Changed files:
filename: cli/npm/managed/resolvers/local.rs
patch: @@ -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(),

filename: cli/task_runner.rs
patch: @@ -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(),
     ];

filename: tests/specs/npm/lifecycle_scripts/__test__.jsonc
patch: @@ -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"
+        }
+      ]
     }
   }
 }

filename: tests/specs/task/byonm/__test__.jsonc
patch: @@ -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"
+      }]
+    }
+  }
 }