feat(config): Node modules option for 2.0 (#25299)

Cargo.lock

@@ -1375,9 +1375,9 @@ dependencies = [
 
 [[package]]
 name = "deno_config"
-version = "0.30.1"
+version = "0.31.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9657dbcc5210407fd9a1b1571310f2fe25c6dd6be2195c964d19f43d70045a95"
+checksum = "efba70c2fbec59e6d0c6040376644803f2cebbb4d55a651cbab4794e390a8592"
 dependencies = [
  "anyhow",
  "deno_package_json",

cli/Cargo.toml

@@ -65,7 +65,7 @@ winres.workspace = true
 [dependencies]
 deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] }
 deno_cache_dir = { workspace = true }
-deno_config = { version = "=0.30.1", features = ["workspace", "sync"] }
+deno_config = { version = "=0.31.0", features = ["workspace", "sync"] }
 deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
 deno_doc = { version = "0.147.0", features = ["html", "syntect"] }
 deno_emit = "=0.44.0"

cli/args/flags.rs

@@ -10,6 +10,7 @@ use clap::ColorChoice;
 use clap::Command;
 use clap::ValueHint;
 use color_print::cstr;
+use deno_config::deno_json::NodeModulesMode;
 use deno_config::glob::FilePatterns;
 use deno_config::glob::PathOrPatternSet;
 use deno_core::anyhow::bail;
@@ -602,6 +603,7 @@ pub struct Flags {
   pub type_check_mode: TypeCheckMode,
   pub config_flag: ConfigFlag,
   pub node_modules_dir: Option<bool>,
+  pub node_modules_mode: Option<NodeModulesMode>,
   pub vendor: Option<bool>,
   pub enable_op_summary_metrics: bool,
   pub enable_testing_features: bool,
@@ -2363,7 +2365,7 @@ TypeScript compiler cache: Subdirectory containing TS compiler output.",
       .arg(no_lock_arg())
       .arg(config_arg())
       .arg(import_map_arg())
-      .arg(node_modules_dir_arg())
+      .args(node_modules_args())
       .arg(vendor_arg())
       .arg(
         Arg::new("json")
@@ -3178,7 +3180,7 @@ Remote modules and multiple modules may also be specified:
       .arg(config_arg())
       .arg(import_map_arg())
       .arg(lock_arg())
-      .arg(node_modules_dir_arg())
+      .args(node_modules_args())
       .arg(vendor_arg())
       .arg(reload_arg())
       .arg(ca_file_arg())
@@ -3240,7 +3242,7 @@ fn compile_args_without_check_args(app: Command) -> Command {
     .arg(import_map_arg())
     .arg(no_remote_arg())
     .arg(no_npm_arg())
-    .arg(node_modules_dir_arg())
+    .args(node_modules_args())
     .arg(vendor_arg())
     .arg(config_arg())
     .arg(no_config_arg())
@@ -3916,16 +3918,49 @@ fn no_npm_arg() -> Arg {
     .help_heading(DEPENDENCY_MANAGEMENT_HEADING)
 }
 
-fn node_modules_dir_arg() -> Arg {
-  Arg::new("node-modules-dir")
-    .long("node-modules-dir")
-    .num_args(0..=1)
-    .value_parser(value_parser!(bool))
-    .value_name("DIRECTORY")
-    .default_missing_value("true")
-    .require_equals(true)
-    .help("Enables or disables the use of a local node_modules folder for npm packages")
-    .help_heading(DEPENDENCY_MANAGEMENT_HEADING)
+fn node_modules_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
+  if *DENO_FUTURE {
+    let value = matches.remove_one::<NodeModulesMode>("node-modules");
+    if let Some(mode) = value {
+      flags.node_modules_mode = Some(mode);
+    }
+  } else {
+    flags.node_modules_dir = matches.remove_one::<bool>("node-modules-dir");
+  }
+}
+
+fn node_modules_args() -> Vec<Arg> {
+  if *DENO_FUTURE {
+    vec![
+      Arg::new("node-modules")
+        .long("node-modules")
+        .num_args(0..=1)
+        .value_parser(NodeModulesMode::parse)
+        .value_name("MODE")
+        .require_equals(true)
+        .help("Sets the node modules management mode for npm packages")
+        .help_heading(DEPENDENCY_MANAGEMENT_HEADING),
+      Arg::new("node-modules-dir")
+        .long("node-modules-dir")
+        .num_args(0..=1)
+        .value_parser(clap::builder::UnknownArgumentValueParser::suggest_arg(
+          "--node-modules",
+        ))
+        .require_equals(true),
+    ]
+  } else {
+    vec![
+      Arg::new("node-modules-dir")
+        .long("node-modules-dir")
+        .num_args(0..=1)
+        .value_parser(value_parser!(bool))
+        .value_name("ENABLED")
+        .default_missing_value("true")
+        .require_equals(true)
+        .help("Enables or disables the use of a local node_modules folder for npm packages")
+        .help_heading(DEPENDENCY_MANAGEMENT_HEADING)
+    ]
+  }
 }
 
 fn vendor_arg() -> Arg {
@@ -5315,7 +5350,7 @@ fn node_modules_and_vendor_dir_arg_parse(
   flags: &mut Flags,
   matches: &mut ArgMatches,
 ) {
-  flags.node_modules_dir = matches.remove_one::<bool>("node-modules-dir");
+  node_modules_arg_parse(flags, matches);
   flags.vendor = matches.remove_one::<bool>("vendor");
 }
 

cli/args/mod.rs

@@ -8,6 +8,7 @@ mod lockfile;
 mod package_json;
 
 use deno_ast::SourceMapOption;
+use deno_config::deno_json::NodeModulesMode;
 use deno_config::workspace::CreateResolverOptions;
 use deno_config::workspace::PackageJsonDepResolution;
 use deno_config::workspace::VendorEnablement;
@@ -819,6 +820,7 @@ impl CliOptions {
     let maybe_node_modules_folder = resolve_node_modules_folder(
       &initial_cwd,
       &flags,
+      &start_dir.workspace,
       root_folder.deno_json.as_deref(),
       root_folder.pkg_json.as_deref(),
       &deno_dir_provider,
@@ -917,6 +919,10 @@ impl CliOptions {
     };
 
     for diagnostic in start_dir.workspace.diagnostics() {
+      // TODO(2.0): remove
+      if matches!(diagnostic.kind, deno_config::workspace::WorkspaceDiagnosticKind::DeprecatedNodeModulesDirOption(_)) && !*DENO_FUTURE {
+        continue;
+      }
       log::warn!("{} {}", colors::yellow("Warning"), diagnostic);
     }
 
@@ -1255,11 +1261,29 @@ impl CliOptions {
     }
   }
 
-  pub fn node_modules_dir_enablement(&self) -> Option<bool> {
-    self
-      .flags
-      .node_modules_dir
-      .or_else(|| self.workspace().node_modules_dir())
+  pub fn node_modules_mode(&self) -> Result<Option<NodeModulesMode>, AnyError> {
+    if *DENO_FUTURE {
+      if let Some(flag) = self.flags.node_modules_mode {
+        return Ok(Some(flag));
+      }
+      self.workspace().node_modules_mode().map_err(Into::into)
+    } else {
+      Ok(
+        self
+          .flags
+          .node_modules_dir
+          .or_else(|| self.workspace().node_modules_dir())
+          .map(|enabled| {
+            if enabled && self.byonm_enabled() {
+              NodeModulesMode::LocalManual
+            } else if enabled {
+              NodeModulesMode::LocalAuto
+            } else {
+              NodeModulesMode::GlobalAuto
+            }
+          }),
+      )
+    }
   }
 
   pub fn vendor_dir_path(&self) -> Option<&PathBuf> {
@@ -1611,9 +1635,19 @@ impl CliOptions {
       || self.workspace().has_unstable("bare-node-builtins")
   }
 
+  fn byonm_enabled(&self) -> bool {
+    // check if enabled via unstable
+    self.flags.unstable_config.byonm
+      || NPM_PROCESS_STATE
+        .as_ref()
+        .map(|s| matches!(s.kind, NpmProcessStateKind::Byonm))
+        .unwrap_or(false)
+      || self.workspace().has_unstable("byonm")
+  }
+
   pub fn use_byonm(&self) -> bool {
     if self.enable_future_features()
-      && self.node_modules_dir_enablement().is_none()
+      && self.node_modules_mode().ok().flatten().is_none()
       && self.maybe_node_modules_folder.is_some()
       && self
         .workspace()
@@ -1624,13 +1658,7 @@ impl CliOptions {
       return true;
     }
 
-    // check if enabled via unstable
-    self.flags.unstable_config.byonm
-      || NPM_PROCESS_STATE
-        .as_ref()
-        .map(|s| matches!(s.kind, NpmProcessStateKind::Byonm))
-        .unwrap_or(false)
-      || self.workspace().has_unstable("byonm")
+    self.byonm_enabled()
   }
 
   pub fn unstable_sloppy_imports(&self) -> bool {
@@ -1762,15 +1790,28 @@ impl CliOptions {
 fn resolve_node_modules_folder(
   cwd: &Path,
   flags: &Flags,
+  workspace: &Workspace,
   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
-    .or_else(|| maybe_config_file.and_then(|c| c.json.node_modules_dir))
-    .or(flags.vendor)
-    .or_else(|| maybe_config_file.and_then(|c| c.json.vendor));
+  let use_node_modules_dir = if *DENO_FUTURE {
+    if let Some(mode) = flags.node_modules_mode {
+      Some(mode.uses_node_modules_dir())
+    } else {
+      workspace
+        .node_modules_mode()?
+        .map(|m| m.uses_node_modules_dir())
+        .or(flags.vendor)
+        .or_else(|| maybe_config_file.and_then(|c| c.json.vendor))
+    }
+  } else {
+    flags
+      .node_modules_dir
+      .or_else(|| maybe_config_file.and_then(|c| c.json.node_modules_dir))
+      .or(flags.vendor)
+      .or_else(|| maybe_config_file.and_then(|c| c.json.vendor))
+  };
   let path = if use_node_modules_dir == Some(false) {
     return Ok(None);
   } else if let Some(state) = &*NPM_PROCESS_STATE {

cli/graph_util.rs

@@ -541,7 +541,12 @@ impl ModuleGraphBuilder {
   ) -> Result<(), AnyError> {
     // ensure an "npm install" is done if the user has explicitly
     // opted into using a node_modules directory
-    if self.options.node_modules_dir_enablement() == Some(true) {
+    if self
+      .options
+      .node_modules_mode()?
+      .map(|m| m.uses_node_modules_dir())
+      .unwrap_or(false)
+    {
       if let Some(npm_resolver) = self.npm_resolver.as_managed() {
         npm_resolver.ensure_top_level_package_json_install().await?;
       }

cli/lsp/config.rs

@@ -5,6 +5,7 @@ use deno_config::deno_json::DenoJsonCache;
 use deno_config::deno_json::FmtConfig;
 use deno_config::deno_json::FmtOptionsConfig;
 use deno_config::deno_json::LintConfig;
+use deno_config::deno_json::NodeModulesMode;
 use deno_config::deno_json::TestConfig;
 use deno_config::deno_json::TsConfig;
 use deno_config::fs::DenoConfigFs;
@@ -1390,8 +1391,16 @@ impl ConfigData {
     let byonm = std::env::var("DENO_UNSTABLE_BYONM").is_ok()
       || member_dir.workspace.has_unstable("byonm")
       || (*DENO_FUTURE
-        && member_dir.workspace.package_jsons().next().is_some()
-        && member_dir.workspace.node_modules_dir().is_none());
+        && matches!(
+          member_dir.workspace.node_modules_mode().unwrap_or_default(),
+          Some(NodeModulesMode::LocalManual)
+        ))
+      || (
+        *DENO_FUTURE
+          && member_dir.workspace.package_jsons().next().is_some()
+          && member_dir.workspace.node_modules_dir().is_none()
+        // TODO(2.0): remove
+      );
     if byonm {
       lsp_log!("  Enabled 'bring your own node_modules'.");
     }

cli/tools/run/mod.rs

@@ -194,7 +194,12 @@ pub async fn eval_command(
 pub async fn maybe_npm_install(factory: &CliFactory) -> Result<(), AnyError> {
   // ensure an "npm install" is done if the user has explicitly
   // opted into using a managed node_modules directory
-  if factory.cli_options()?.node_modules_dir_enablement() == Some(true) {
+  if factory
+    .cli_options()?
+    .node_modules_mode()?
+    .map(|m| m.uses_node_modules_dir())
+    .unwrap_or(false)
+  {
     if let Some(npm_resolver) = factory.npm_resolver().await?.as_managed() {
       npm_resolver.ensure_top_level_package_json_install().await?;
     }

cli/tools/vendor/mod.rs

@@ -88,7 +88,10 @@ pub async fn vendor(
   let graph = output.graph;
   let npm_package_count = graph.npm_packages.len();
   let try_add_node_modules_dir = npm_package_count > 0
-    && cli_options.node_modules_dir_enablement().unwrap_or(true);
+    && cli_options
+      .node_modules_mode()?
+      .map(|m| m.uses_node_modules_dir())
+      .unwrap_or(true);
 
   log::info!(
     concat!("Vendored {} {} into {} directory.",),

ext/node/polyfills/child_process.ts

@@ -145,7 +145,6 @@ export function fork(
   args = [
     "run",
     ...op_bootstrap_unstable_args(),
-    "--node-modules-dir",
     "-A",
     ...stringifiedV8Flags,
     ...execArgv,

tests/specs/lockfile/frozen_lockfile/__test__.jsonc

@@ -58,18 +58,22 @@
       ]
     },
     "error_when_package_json_changed": {
+      "envs": {
+        "DENO_FUTURE": "1"
+      },
       "steps": [
         {
-          "envs": {
-            "DENO_FUTURE": "1"
-          },
+          "args": [
+            "eval",
+            "Deno.writeTextFileSync('deno.json', `{ \"nodeModules\": \"local-auto\" }`)"
+          ],
+          "output": "[WILDCARD]"
+        },
+        {
           "args": "cache add.ts",
           "output": "[WILDCARD]"
         },
         {
-          "envs": {
-            "DENO_FUTURE": "1"
-          },
           "args": [
             "eval",
             "Deno.writeTextFileSync(\"package.json\", JSON.stringify({ dependencies: { \"@denotest/bin\": \"0.7.0\" } }))"
@@ -77,17 +81,11 @@
           "output": ""
         },
         {
-          "envs": {
-            "DENO_FUTURE": "1"
-          },
           "args": "cache --frozen add.ts",
           "output": "frozen_package_json_changed.out",
           "exitCode": 1
         },
         {
-          "envs": {
-            "DENO_FUTURE": "1"
-          },
           "args": "install --frozen",
           "output": "frozen_package_json_changed_install.out",
           "exitCode": 1

tests/specs/npm/bin_entries_prefer_closer/deno.json

@@ -1,3 +1,3 @@
 {
-  "nodeModulesDir": true
+  "nodeModules": "local-auto"
 }

tests/specs/npm/future_node_modules_dir_setting/__test__.jsonc

@@ -10,11 +10,11 @@
     "exitCode": 1
   }, {
     // this should override byonm
-    "args": "run --node-modules-dir=false --quiet main.ts",
+    "args": "run --node-modules=global-auto --quiet main.ts",
     "output": "main.out"
   }, {
     // same with this
-    "args": "run --node-modules-dir --quiet main.ts",
+    "args": "run --node-modules=local-auto --quiet main.ts",
     "output": "main.out"
   }]
 }

tests/specs/npm/lifecycle_scripts/__test__.jsonc

@@ -78,6 +78,13 @@
         "DENO_FUTURE": "1"
       },
       "steps": [
+        {
+          "args": [
+            "eval",
+            "Deno.removeSync('./deno.json')"
+          ],
+          "output": "[WILDCARD]"
+        },
         {
           "args": [
             "eval",