FUTURE: deno install changes

cli/args/flags.rs

@@ -914,14 +914,14 @@ impl Flags {
       }
       Task(_) | Check(_) | Coverage(_) | Cache(_) | Info(_) | Eval(_)
       | Test(_) | Bench(_) | Repl(_) | Compile(_) | Publish(_) => {
-        std::env::current_dir().ok()
+        Some(current_dir.to_path_buf())
       }
       Add(_) | Bundle(_) | Completions(_) | Doc(_) | Fmt(_) | Init(_)
       | Uninstall(_) | Jupyter(_) | Lsp | Lint(_) | Types | Upgrade(_)
       | Vendor(_) => None,
       Install(_) => {
         if *DENO_FUTURE {
-          std::env::current_dir().ok()
+          Some(current_dir.to_path_buf())
         } else {
           None
         }
@@ -1342,20 +1342,6 @@ fn clap_root() -> Command {
     .after_help(ENV_VARIABLES_HELP)
 }
 
-fn add_args(cmd: Command, include_packages: bool) -> Command {
-  if include_packages {
-    cmd.arg(
-      Arg::new("packages")
-        .help("List of packages to add")
-        .required(true)
-        .num_args(1..)
-        .action(ArgAction::Append),
-    )
-  } else {
-    cmd
-  }
-}
-
 fn add_subcommand() -> Command {
   Command::new("add")
     .about("Add dependencies")
@@ -1369,7 +1355,15 @@ You can add multiple dependencies at once:
   deno add @std/path @std/assert
 ",
     )
-    .defer(|cmd| add_args(cmd, true))
+    .defer(|cmd| {
+      cmd.arg(
+        Arg::new("packages")
+          .help("List of packages to add")
+          .required(true)
+          .num_args(1..)
+          .action(ArgAction::Append),
+      )
+    })
 }
 
 fn bench_subcommand() -> Command {
@@ -2160,8 +2154,7 @@ The installation root is determined, in order of precedence:
 These must be added to the path manually if required.")
     .defer(|cmd| {
       let cmd = runtime_args(cmd, true, true).arg(check_arg(true));
-      let cmd = install_args(cmd, true);
-      add_args(cmd, false)
+      install_args(cmd, true)
     })
 }
 
@@ -3986,8 +3979,7 @@ fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) {
     let args = cmd_values.collect();
 
     flags.subcommand = DenoSubcommand::Install(InstallFlags {
-      // TODO(bartlomieju): remove once `deno install` supports both local and
-      // global installs
+      // TODO(bartlomieju): remove for 2.0
       global,
       kind: InstallKind::Global(InstallFlagsGlobal {
         name,

cli/args/mod.rs

@@ -721,7 +721,7 @@ pub struct CliOptions {
   maybe_node_modules_folder: Option<PathBuf>,
   maybe_vendor_folder: Option<PathBuf>,
   maybe_config_file: Option<ConfigFile>,
-  pub maybe_package_json: Option<PackageJson>,
+  maybe_package_json: Option<PackageJson>,
   maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
   overrides: CliOptionOverrides,
   maybe_workspace_config: Option<WorkspaceConfig>,

cli/tools/registry/pm.rs

@@ -1,5 +1,6 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
+use std::borrow::Cow;
 use std::path::PathBuf;
 use std::sync::Arc;
 
@@ -28,23 +29,43 @@ use crate::file_fetcher::FileFetcher;
 use crate::jsr::JsrFetchResolver;
 use crate::npm::NpmFetchResolver;
 
-enum ConfigFile {
-  Deno(deno_config::ConfigFile),
+enum DenoConfigFormat {
+  Json,
+  Jsonc,
+}
+
+impl DenoConfigFormat {
+  fn from_specifier(spec: &ModuleSpecifier) -> Result<Self, AnyError> {
+    let file_name = spec
+      .path_segments()
+      .ok_or_else(|| anyhow!("Empty path in deno config specifier: {spec}"))?
+      .last()
+      .unwrap();
+    match file_name {
+      "deno.json" => Ok(Self::Json),
+      "deno.jsonc" => Ok(Self::Jsonc),
+      _ => bail!("Unsupported deno config file: {file_name}"),
+    }
+  }
+}
+
+enum DenoOrPackageJson {
+  Deno(deno_config::ConfigFile, DenoConfigFormat),
   Npm(deno_node::PackageJson, Option<FmtOptionsConfig>),
 }
 
-impl ConfigFile {
-  fn specifier(&self) -> ModuleSpecifier {
+impl DenoOrPackageJson {
+  fn specifier(&self) -> Cow<ModuleSpecifier> {
     match self {
-      Self::Deno(d) => d.specifier.clone(),
-      Self::Npm(n, ..) => n.specifier(),
+      Self::Deno(d, ..) => Cow::Borrowed(&d.specifier),
+      Self::Npm(n, ..) => Cow::Owned(n.specifier()),
     }
   }
 
   /// Returns the existing imports/dependencies from the config.
   fn existing_imports(&self) -> Result<IndexMap<String, String>, AnyError> {
     match self {
-      ConfigFile::Deno(deno) => {
+      DenoOrPackageJson::Deno(deno, ..) => {
         if let Some(imports) = deno.json.imports.clone() {
           match serde_json::from_value(imports) {
             Ok(map) => Ok(map),
@@ -56,35 +77,38 @@ impl ConfigFile {
           Ok(Default::default())
         }
       }
-      ConfigFile::Npm(npm, ..) => {
+      DenoOrPackageJson::Npm(npm, ..) => {
         Ok(npm.dependencies.clone().unwrap_or_default())
       }
     }
   }
 
   fn fmt_options(&self) -> FmtOptionsConfig {
     match self {
-      ConfigFile::Deno(deno) => deno
+      DenoOrPackageJson::Deno(deno, ..) => deno
         .to_fmt_config()
         .ok()
         .flatten()
         .map(|f| f.options)
         .unwrap_or_default(),
-      ConfigFile::Npm(_, config) => config.clone().unwrap_or_default(),
+      DenoOrPackageJson::Npm(_, config) => config.clone().unwrap_or_default(),
     }
   }
 
   fn imports_key(&self) -> &'static str {
     match self {
-      ConfigFile::Deno(_) => "imports",
-      ConfigFile::Npm(..) => "dependencies",
+      DenoOrPackageJson::Deno(..) => "imports",
+      DenoOrPackageJson::Npm(..) => "dependencies",
     }
   }
 
   fn file_name(&self) -> &'static str {
     match self {
-      ConfigFile::Deno(_) => "deno.json",
-      ConfigFile::Npm(..) => "package.json",
+      DenoOrPackageJson::Deno(_, format) => match format {
+        DenoConfigFormat::Json => "deno.json",
+        DenoConfigFormat::Jsonc => "deno.jsonc",
+      },
+      DenoOrPackageJson::Npm(..) => "package.json",
     }
   }
 
@@ -97,35 +121,39 @@ impl ConfigFile {
   /// creates a `deno.json` file - in this case
   /// we also return a new `CliFactory` that knows about
   /// the new config
-  async fn from_flags(flags: Flags) -> Result<(Self, CliFactory), AnyError> {
+  fn from_flags(flags: Flags) -> Result<(Self, CliFactory), AnyError> {
     let factory = CliFactory::from_flags(flags.clone())?;
     let options = factory.cli_options().clone();
 
     match (options.maybe_config_file(), options.maybe_package_json()) {
       // when both are present, for now,
       // default to deno.json
-      (Some(deno), Some(_) | None) => {
-        Ok((ConfigFile::Deno(deno.clone()), factory))
-      }
+      (Some(deno), Some(_) | None) => Ok((
+        DenoOrPackageJson::Deno(
+          deno.clone(),
+          DenoConfigFormat::from_specifier(&deno.specifier)?,
+        ),
+        factory,
+      )),
       (None, Some(package_json)) if options.enable_future_features() => {
-        Ok((ConfigFile::Npm(package_json.clone(), None), factory))
+        Ok((DenoOrPackageJson::Npm(package_json.clone(), None), factory))
       }
       (None, Some(_) | None) => {
-        tokio::fs::write(options.initial_cwd().join("deno.json"), "{}\n")
-          .await
+        std::fs::write(options.initial_cwd().join("deno.json"), "{}\n")
           .context("Failed to create deno.json file")?;
         log::info!("Created deno.json configuration file.");
         let new_factory = CliFactory::from_flags(flags.clone())?;
         let new_options = new_factory.cli_options().clone();
         Ok((
-          ConfigFile::Deno(
+          DenoOrPackageJson::Deno(
             new_options
               .maybe_config_file()
               .as_ref()
               .ok_or_else(|| {
                 anyhow!("config not found, but it was just created")
               })?
               .clone(),
+            DenoConfigFormat::Json,
           ),
           new_factory,
         ))
@@ -134,24 +162,12 @@ impl ConfigFile {
   }
 }
 
-/// removes the `npm:` or `jsr:` prefix from a
-/// specifier (e.g. for inserting into
-/// `package.json`)
-fn strip_specifier_prefix(specifier: &str) -> &str {
-  let specifier = specifier.strip_prefix("npm:").unwrap_or(specifier);
-  specifier.strip_prefix("jsr:").unwrap_or(specifier)
-}
-
 fn package_json_dependency_entry(
   selected: SelectedPackage,
 ) -> (String, String) {
-  if selected.package_name.starts_with("npm:") {
-    (
-      strip_specifier_prefix(&selected.package_name).into(),
-      selected.version_req,
-    )
-  } else if selected.package_name.starts_with("jsr:") {
-    let jsr_package = strip_specifier_prefix(&selected.package_name);
+  if let Some(npm_package) = selected.package_name.strip_prefix("npm:") {
+    (npm_package.into(), selected.version_req)
+  } else if let Some(jsr_package) = selected.package_name.strip_prefix("jsr:") {
     let jsr_package = jsr_package.strip_prefix('@').unwrap_or(jsr_package);
     let scope_replaced = jsr_package.replace('/', "__");
     let version_req =
@@ -164,7 +180,7 @@ fn package_json_dependency_entry(
 
 pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> {
   let (config_file, cli_factory) =
-    ConfigFile::from_flags(flags.clone()).await?;
+    DenoOrPackageJson::from_flags(flags.clone())?;
 
   let config_specifier = config_file.specifier();
   if config_specifier.scheme() != "file" {

tests/specs/install/future_install_global/__test__.jsonc

@@ -10,7 +10,7 @@
     },
     {
       "args": "run -A assert.js",
-      "output": "assert.out"
+      "output": ""
     }
   ]
 }

tests/specs/install/future_install_local_add_deno/__test__.jsonc

@@ -9,9 +9,16 @@
       "output": "install.out"
     },
     {
-      "args": "run --allow-read=./deno.json --cached-only main.js",
-      "exitCode": 0,
-      "output": "main.out"
+      // make sure the dep got cached
+      "args": "run --cached-only main.js",
+      "output": ""
+    },
+    {
+      "args": [
+        "eval",
+        "console.log(Deno.readTextFileSync('deno.json').trim())"
+      ],
+      "output": "deno.json.out"
     }
   ]
 }

tests/specs/install/future_install_local_add_deno/deno.json.out

@@ -0,0 +1,5 @@
+{
+  "imports": {
+    "@denotest/esm-basic": "npm:@denotest/esm-basic@^1.0.0"
+  }
+}

tests/specs/install/future_install_local_add_deno/main.js

@@ -1,7 +1,2 @@
 import { setValue } from "@denotest/esm-basic";
 setValue(5);
-
-const denoJson = JSON.parse(await Deno.readTextFile("./deno.json"));
-if (!denoJson.imports || !denoJson.imports["@denotest/esm-basic"]) {
-  throw new Error("deno.json missing dep!");
-}

tests/specs/install/future_install_local_add_npm/__test__.jsonc

@@ -9,9 +9,17 @@
       "output": "install.out"
     },
     {
-      "args": "run --allow-read=./package.json --cached-only main.js",
+      // make sure the dep got cached
+      "args": "run --cached-only main.js",
       "exitCode": 0,
-      "output": "main.out"
+      "output": ""
+    },
+    {
+      "args": [
+        "eval",
+        "console.log(Deno.readTextFileSync('package.json').trim())"
+      ],
+      "output": "package.json.out"
     }
   ]
 }

tests/specs/install/future_install_local_add_npm/main.js

@@ -1,10 +1,2 @@
 import { setValue } from "@denotest/esm-basic";
 setValue(5);
-
-const packageJson = JSON.parse(await Deno.readTextFile("./package.json"));
-
-if (
-  !packageJson.dependencies || !packageJson.dependencies["@denotest/esm-basic"]
-) {
-  throw new Error("Package json missing dep!");
-}

tests/specs/install/future_install_local_add_npm/package.json.out

@@ -0,0 +1,3 @@
+{
+  "dependencies": { "@denotest/esm-basic": "^1.0.0" }
+}

tests/specs/install/future_install_local_deno/__test__.jsonc

@@ -9,9 +9,9 @@
       "output": "install.out"
     },
     {
+      // ensure deps are actually cached
       "args": "run --cached-only main.js",
-      "exitCode": 0,
-      "output": "main.out"
+      "output": ""
     }
   ]
 }

tests/specs/install/no_future_install_global/__test__.jsonc

@@ -7,7 +7,7 @@
     },
     {
       "args": "run -A ./assert.js",
-      "output": "assert.out"
+      "output": ""
     }
   ]
 }