From 28c395037079bc601e7645541dc746787ab6e285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 9 Apr 2024 20:12:22 +0200 Subject: [PATCH 01/32] wip --- cli/args/flags.rs | 75 ++++++++++++++++++++++++++---------------- cli/tools/installer.rs | 15 ++++++++- package.json | 14 ++++++++ 3 files changed, 75 insertions(+), 29 deletions(-) create mode 100644 package.json diff --git a/cli/args/flags.rs b/cli/args/flags.rs index c03acb02a95dad..ae9b257fe29c01 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -38,6 +38,7 @@ use crate::args::resolve_no_prompt; use crate::util::fs::canonicalize_path; use super::flags_net; +use super::DENO_FUTURE; #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct FileFlags { @@ -2078,25 +2079,36 @@ The installation root is determined, in order of precedence: - $HOME/.deno These must be added to the path manually if required.") - .defer(|cmd| runtime_args(cmd, true, true).arg(Arg::new("cmd").required(true).num_args(1..).value_hint(ValueHint::FilePath)) - .arg(check_arg(true)) - .arg( + .defer(|cmd| { + let cmd = + runtime_args(cmd, true, true) + .arg(check_arg(true)); + + let cmd = if *DENO_FUTURE { + cmd.arg(Arg::new("cmd").required_if_eq("global", "true").num_args(1..).value_hint(ValueHint::FilePath)) + } else { + cmd.arg(Arg::new("cmd").required(true).num_args(1..).value_hint(ValueHint::FilePath)) + }; + + cmd.arg( Arg::new("name") .long("name") .short('n') .help("Executable file name") - .required(false)) + .required(false) + ) .arg( Arg::new("root") .long("root") .help("Installation root") - .value_hint(ValueHint::DirPath)) + .value_hint(ValueHint::DirPath) + ) .arg( Arg::new("force") .long("force") .short('f') .help("Forcefully overwrite existing installation") - .action(ArgAction::SetTrue)) + .action(ArgAction::SetTrue) ) .arg( Arg::new("global") @@ -2106,6 +2118,7 @@ These must be added to the path manually if required.") .action(ArgAction::SetTrue) ) .arg(env_file_arg()) + }) } fn jupyter_subcommand() -> Command { @@ -3870,29 +3883,35 @@ fn info_parse(flags: &mut Flags, matches: &mut ArgMatches) { fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) { runtime_args_parse(flags, matches, true, true); - - let root = matches.remove_one::("root"); - - let force = matches.get_flag("force"); + let global = matches.get_flag("global"); - let name = matches.remove_one::("name"); - let mut cmd_values = matches.remove_many::("cmd").unwrap(); - - let module_url = cmd_values.next().unwrap(); - let args = cmd_values.collect(); - - flags.subcommand = DenoSubcommand::Install(InstallFlags { - // TODO(bartlomieju): remove once `deno install` supports both local and - // global installs - global, - kind: InstallKind::Global(InstallFlagsGlobal { - name, - module_url, - args, - root, - force, - }), - }); + if global { + let root = matches.remove_one::("root"); + let force = matches.get_flag("force"); + let name = matches.remove_one::("name"); + let mut cmd_values = matches.remove_many::("cmd").unwrap_or_default(); + + let module_url = cmd_values.next().unwrap(); + let args = cmd_values.collect(); + + flags.subcommand = DenoSubcommand::Install(InstallFlags { + // TODO(bartlomieju): remove once `deno install` supports both local and + // global installs + global, + kind: InstallKind::Global(InstallFlagsGlobal { + name, + module_url, + args, + root, + force, + }), + }); + } else { + flags.subcommand = DenoSubcommand::Install(InstallFlags { + global, + kind: InstallKind::Local, + }) + } } fn jupyter_parse(flags: &mut Flags, matches: &mut ArgMatches) { diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index f3eba7b8a87455..c0cb55a381f299 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -253,6 +253,19 @@ pub fn uninstall(uninstall_flags: UninstallFlags) -> Result<(), AnyError> { Ok(()) } +async fn install_local(flags: Flags) -> Result<(), AnyError> { + let factory = CliFactory::from_flags(flags)?; + let cli_options = factory.cli_options(); + let npm_resolver = factory.npm_resolver().await?; + + eprintln!("{}", npm_resolver.as_managed().is_some()); + if let Some(npm_resolver) = npm_resolver.as_managed() { + npm_resolver.ensure_top_level_package_json_install().await?; + npm_resolver.resolve_pending().await?; + } + Ok(()) +} + pub async fn install_command( flags: Flags, install_flags: InstallFlags, @@ -263,7 +276,7 @@ pub async fn install_command( let install_flags_global = match install_flags.kind { InstallKind::Global(flags) => flags, - InstallKind::Local => unreachable!(), + InstallKind::Local => return install_local(flags).await, }; // ensure the module is cached diff --git a/package.json b/package.json new file mode 100644 index 00000000000000..877e29086245e8 --- /dev/null +++ b/package.json @@ -0,0 +1,14 @@ +{ + "name": "deno", + "version": "1.0.0", + "description": "[![](https://img.shields.io/crates/v/deno.svg)](https://crates.io/crates/deno) [![Twitter badge][]][Twitter link] [![Discord badge][]][Discord link] [![YouTube badge][]][YouTube link]", + "main": "index.js", + "directories": { + "test": "tests" + }, + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC" +} From 0c1c21257b44575f6173b8b6f0008df2df6b5bcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 22 Apr 2024 23:36:45 +0200 Subject: [PATCH 02/32] works with DENO_FUTURE=1 --- cli/args/flags.rs | 9 ++++++++- cli/args/mod.rs | 2 +- cli/factory.rs | 3 ++- cli/tools/installer.rs | 1 - 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index ae9b257fe29c01..d7fd66a4423e64 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -917,8 +917,15 @@ impl Flags { std::env::current_dir().ok() } Add(_) | Bundle(_) | Completions(_) | Doc(_) | Fmt(_) | Init(_) - | Install(_) | Uninstall(_) | Jupyter(_) | Lsp | Lint(_) | Types + | Uninstall(_) | Jupyter(_) | Lsp | Lint(_) | Types | Upgrade(_) | Vendor(_) => None, + Install(_) => { + if *DENO_FUTURE { + std::env::current_dir().ok() + } else { + None + } + }, } } diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 3b5d79ef3cb797..6db73c0f1dade1 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -721,7 +721,7 @@ pub struct CliOptions { maybe_node_modules_folder: Option, maybe_vendor_folder: Option, maybe_config_file: Option, - maybe_package_json: Option, + pub maybe_package_json: Option, maybe_lockfile: Option>>, overrides: CliOptionOverrides, maybe_workspace_config: Option, diff --git a/cli/factory.rs b/cli/factory.rs index a9f6bf87e90348..074eaa1e4cebae 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -408,7 +408,8 @@ impl CliFactory { .npm_resolver .get_or_try_init_async(async { let fs = self.fs(); - create_cli_npm_resolver(if self.options.use_byonm() { + // For `deno install` we want to force the managed resolver so it can set up `node_modules/` directory. + create_cli_npm_resolver(if self.options.use_byonm() && !matches!(self.options.sub_command(), DenoSubcommand::Install(_)) { CliNpmResolverCreateOptions::Byonm(CliNpmResolverByonmCreateOptions { fs: fs.clone(), root_node_modules_dir: match self.options.node_modules_dir_path() { diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index c0cb55a381f299..dacad9e3d8ebc3 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -258,7 +258,6 @@ async fn install_local(flags: Flags) -> Result<(), AnyError> { let cli_options = factory.cli_options(); let npm_resolver = factory.npm_resolver().await?; - eprintln!("{}", npm_resolver.as_managed().is_some()); if let Some(npm_resolver) = npm_resolver.as_managed() { npm_resolver.ensure_top_level_package_json_install().await?; npm_resolver.resolve_pending().await?; From 6f35a70ffa8c3c5096cc88103ef304dd4ac39cfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 22 Apr 2024 23:38:04 +0200 Subject: [PATCH 03/32] revert --- Cargo.lock | 6 +++--- package.json | 14 -------------- 2 files changed, 3 insertions(+), 17 deletions(-) delete mode 100644 package.json diff --git a/Cargo.lock b/Cargo.lock index c21efbbc8af69c..ab73634ae1f2bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5900,12 +5900,12 @@ dependencies = [ [[package]] name = "socket2" -version = "0.5.6" +version = "0.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05ffd9c0a93b7543e062e759284fcf5f5e3b098501104bfbdde4d404db792871" +checksum = "7b5fac59a5cb5dd637972e5fca70daf0523c9067fcdc4842f053dae04a18f8e9" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.48.0", ] [[package]] diff --git a/package.json b/package.json deleted file mode 100644 index 877e29086245e8..00000000000000 --- a/package.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "name": "deno", - "version": "1.0.0", - "description": "[![](https://img.shields.io/crates/v/deno.svg)](https://crates.io/crates/deno) [![Twitter badge][]][Twitter link] [![Discord badge][]][Discord link] [![YouTube badge][]][YouTube link]", - "main": "index.js", - "directories": { - "test": "tests" - }, - "scripts": { - "test": "echo \"Error: no test specified\" && exit 1" - }, - "author": "", - "license": "ISC" -} From 394bd8ee7f3c3c2b7e5ca4ed2cd575108c063477 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Thu, 25 Apr 2024 09:39:44 -0400 Subject: [PATCH 04/32] Fix lint --- cli/tools/installer.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index dacad9e3d8ebc3..1e468ffb19c9e7 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -255,7 +255,6 @@ pub fn uninstall(uninstall_flags: UninstallFlags) -> Result<(), AnyError> { async fn install_local(flags: Flags) -> Result<(), AnyError> { let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); let npm_resolver = factory.npm_resolver().await?; if let Some(npm_resolver) = npm_resolver.as_managed() { From f043fbe5712783d571b5ba334dbbc3a5d8b4851e Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Thu, 25 Apr 2024 09:42:18 -0400 Subject: [PATCH 05/32] Fmt --- cli/args/flags.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index d7fd66a4423e64..6adffcbd91c77c 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -917,15 +917,15 @@ impl Flags { std::env::current_dir().ok() } Add(_) | Bundle(_) | Completions(_) | Doc(_) | Fmt(_) | Init(_) - | Uninstall(_) | Jupyter(_) | Lsp | Lint(_) | Types - | Upgrade(_) | Vendor(_) => None, + | Uninstall(_) | Jupyter(_) | Lsp | Lint(_) | Types | Upgrade(_) + | Vendor(_) => None, Install(_) => { if *DENO_FUTURE { std::env::current_dir().ok() } else { None } - }, + } } } @@ -2087,7 +2087,7 @@ The installation root is determined, in order of precedence: These must be added to the path manually if required.") .defer(|cmd| { - let cmd = + let cmd = runtime_args(cmd, true, true) .arg(check_arg(true)); @@ -2096,7 +2096,7 @@ These must be added to the path manually if required.") } else { cmd.arg(Arg::new("cmd").required(true).num_args(1..).value_hint(ValueHint::FilePath)) }; - + cmd.arg( Arg::new("name") .long("name") @@ -3890,13 +3890,14 @@ fn info_parse(flags: &mut Flags, matches: &mut ArgMatches) { fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) { runtime_args_parse(flags, matches, true, true); - + let global = matches.get_flag("global"); if global { let root = matches.remove_one::("root"); let force = matches.get_flag("force"); let name = matches.remove_one::("name"); - let mut cmd_values = matches.remove_many::("cmd").unwrap_or_default(); + let mut cmd_values = + matches.remove_many::("cmd").unwrap_or_default(); let module_url = cmd_values.next().unwrap(); let args = cmd_values.collect(); From 7209ccd40fa794a9cd2a3b31c2b72e325e339ec6 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Thu, 25 Apr 2024 10:11:24 -0400 Subject: [PATCH 06/32] Don't require global flag if !DENO_FUTURE --- cli/args/flags.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 6adffcbd91c77c..603de835e70721 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -3892,7 +3892,7 @@ fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) { runtime_args_parse(flags, matches, true, true); let global = matches.get_flag("global"); - if global { + if global || !*DENO_FUTURE { let root = matches.remove_one::("root"); let force = matches.get_flag("force"); let name = matches.remove_one::("name"); From 49c77d29c3789d05b1947e1ebec5cf6952f3e934 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Thu, 25 Apr 2024 11:45:01 -0400 Subject: [PATCH 07/32] Add integration tests --- .../future_install_global/__test__.jsonc | 12 ++++++++++++ .../install/future_install_global/install.out | 7 +++++++ .../install/future_install_global/main.js | 3 +++ .../install/future_install_global/main.out | 0 .../future_install_global/package.json | 7 +++++++ .../__test__.jsonc | 19 +++++++++++++++++++ .../future_install_node_modules/install.out | 4 ++++ .../future_install_node_modules/main.js | 3 +++ .../future_install_node_modules/main.out | 0 .../future_install_node_modules/package.json | 6 ++++++ .../no_future_install_global/__test__.jsonc | 9 +++++++++ .../no_future_install_global/install.out | 7 +++++++ .../install/no_future_install_global/main.js | 3 +++ .../install/no_future_install_global/main.out | 0 .../no_future_install_global/package.json | 7 +++++++ 15 files changed, 87 insertions(+) create mode 100644 tests/specs/install/future_install_global/__test__.jsonc create mode 100644 tests/specs/install/future_install_global/install.out create mode 100644 tests/specs/install/future_install_global/main.js create mode 100644 tests/specs/install/future_install_global/main.out create mode 100644 tests/specs/install/future_install_global/package.json create mode 100644 tests/specs/install/future_install_node_modules/__test__.jsonc create mode 100644 tests/specs/install/future_install_node_modules/install.out create mode 100644 tests/specs/install/future_install_node_modules/main.js create mode 100644 tests/specs/install/future_install_node_modules/main.out create mode 100644 tests/specs/install/future_install_node_modules/package.json create mode 100644 tests/specs/install/no_future_install_global/__test__.jsonc create mode 100644 tests/specs/install/no_future_install_global/install.out create mode 100644 tests/specs/install/no_future_install_global/main.js create mode 100644 tests/specs/install/no_future_install_global/main.out create mode 100644 tests/specs/install/no_future_install_global/package.json diff --git a/tests/specs/install/future_install_global/__test__.jsonc b/tests/specs/install/future_install_global/__test__.jsonc new file mode 100644 index 00000000000000..3bfbec2579bb63 --- /dev/null +++ b/tests/specs/install/future_install_global/__test__.jsonc @@ -0,0 +1,12 @@ +{ + "tempDir": true, + "envs": { + "DENO_FUTURE": "1" + }, + "steps": [ + { + "args": "install --global --root ./bins ./main.js", + "output": "install.out" + } + ] +} diff --git a/tests/specs/install/future_install_global/install.out b/tests/specs/install/future_install_global/install.out new file mode 100644 index 00000000000000..09cf2cf0250988 --- /dev/null +++ b/tests/specs/install/future_install_global/install.out @@ -0,0 +1,7 @@ +Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Initialize @denotest/esm-basic@1.0.0 +✅ Successfully installed deno-cli-test[WILDCARD] +/[WILDCARD]/bins/bin/deno-cli-test[WILDCARD] +[WILDLINE] +[WILDLINE] diff --git a/tests/specs/install/future_install_global/main.js b/tests/specs/install/future_install_global/main.js new file mode 100644 index 00000000000000..2ba55540b5d9d9 --- /dev/null +++ b/tests/specs/install/future_install_global/main.js @@ -0,0 +1,3 @@ +import { setValue } from "@denotest/esm-basic"; + +setValue(5); diff --git a/tests/specs/install/future_install_global/main.out b/tests/specs/install/future_install_global/main.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/specs/install/future_install_global/package.json b/tests/specs/install/future_install_global/package.json new file mode 100644 index 00000000000000..f3b6cb7be1a466 --- /dev/null +++ b/tests/specs/install/future_install_global/package.json @@ -0,0 +1,7 @@ +{ + "name": "deno-test-bin", + "dependencies": { + "@denotest/esm-basic": "*" + }, + "type": "module" +} diff --git a/tests/specs/install/future_install_node_modules/__test__.jsonc b/tests/specs/install/future_install_node_modules/__test__.jsonc new file mode 100644 index 00000000000000..324e50749d600e --- /dev/null +++ b/tests/specs/install/future_install_node_modules/__test__.jsonc @@ -0,0 +1,19 @@ +{ + "tempDir": true, + "envs": { + "DENO_FUTURE": "1" + }, + "steps": [ + { + "cleanDenoDir": true, + "args": "install", + "output": "install.out" + }, + { + "cleanDenoDir": true, + "args": "run --cached-only main.js", + "exitCode": 0, + "output": "main.out" + } + ] +} diff --git a/tests/specs/install/future_install_node_modules/install.out b/tests/specs/install/future_install_node_modules/install.out new file mode 100644 index 00000000000000..1cb06afcc5d818 --- /dev/null +++ b/tests/specs/install/future_install_node_modules/install.out @@ -0,0 +1,4 @@ +⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. +Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Initialize @denotest/esm-basic@1.0.0 diff --git a/tests/specs/install/future_install_node_modules/main.js b/tests/specs/install/future_install_node_modules/main.js new file mode 100644 index 00000000000000..2ba55540b5d9d9 --- /dev/null +++ b/tests/specs/install/future_install_node_modules/main.js @@ -0,0 +1,3 @@ +import { setValue } from "@denotest/esm-basic"; + +setValue(5); diff --git a/tests/specs/install/future_install_node_modules/main.out b/tests/specs/install/future_install_node_modules/main.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/specs/install/future_install_node_modules/package.json b/tests/specs/install/future_install_node_modules/package.json new file mode 100644 index 00000000000000..8e46ec6ebe329c --- /dev/null +++ b/tests/specs/install/future_install_node_modules/package.json @@ -0,0 +1,6 @@ +{ + "dependencies": { + "@denotest/esm-basic": "*" + }, + "type": "module" +} diff --git a/tests/specs/install/no_future_install_global/__test__.jsonc b/tests/specs/install/no_future_install_global/__test__.jsonc new file mode 100644 index 00000000000000..e4b8b6c13d9ddf --- /dev/null +++ b/tests/specs/install/no_future_install_global/__test__.jsonc @@ -0,0 +1,9 @@ +{ + "tempDir": true, + "steps": [ + { + "args": "install --root ./bins ./main.js", + "output": "install.out" + } + ] +} diff --git a/tests/specs/install/no_future_install_global/install.out b/tests/specs/install/no_future_install_global/install.out new file mode 100644 index 00000000000000..6dcda89ca442da --- /dev/null +++ b/tests/specs/install/no_future_install_global/install.out @@ -0,0 +1,7 @@ +⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. +Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +✅ Successfully installed deno-cli-test[WILDCARD] +/[WILDCARD]/bins/bin/deno-cli-test[WILDCARD] +[WILDLINE] +[WILDLINE] diff --git a/tests/specs/install/no_future_install_global/main.js b/tests/specs/install/no_future_install_global/main.js new file mode 100644 index 00000000000000..6268d713625799 --- /dev/null +++ b/tests/specs/install/no_future_install_global/main.js @@ -0,0 +1,3 @@ +import { setValue } from "npm:@denotest/esm-basic"; + +setValue(5); diff --git a/tests/specs/install/no_future_install_global/main.out b/tests/specs/install/no_future_install_global/main.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/specs/install/no_future_install_global/package.json b/tests/specs/install/no_future_install_global/package.json new file mode 100644 index 00000000000000..f3b6cb7be1a466 --- /dev/null +++ b/tests/specs/install/no_future_install_global/package.json @@ -0,0 +1,7 @@ +{ + "name": "deno-test-bin", + "dependencies": { + "@denotest/esm-basic": "*" + }, + "type": "module" +} From 5dc90f6e8d43765baf61b70e76a28c4f7522cd5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 25 Apr 2024 17:49:58 +0200 Subject: [PATCH 08/32] reset CI From 7a6b4c40f26eb2808b7bc31e8ae6b577c33f6a64 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Thu, 25 Apr 2024 12:35:53 -0400 Subject: [PATCH 09/32] Fix output on windows --- tests/specs/install/future_install_global/install.out | 4 +--- tests/specs/install/no_future_install_global/install.out | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/specs/install/future_install_global/install.out b/tests/specs/install/future_install_global/install.out index 09cf2cf0250988..09134524c161d1 100644 --- a/tests/specs/install/future_install_global/install.out +++ b/tests/specs/install/future_install_global/install.out @@ -2,6 +2,4 @@ Download http://localhost:4545/npm/registry/@denotest/esm-basic Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz Initialize @denotest/esm-basic@1.0.0 ✅ Successfully installed deno-cli-test[WILDCARD] -/[WILDCARD]/bins/bin/deno-cli-test[WILDCARD] -[WILDLINE] -[WILDLINE] +[WILDCARD] diff --git a/tests/specs/install/no_future_install_global/install.out b/tests/specs/install/no_future_install_global/install.out index 6dcda89ca442da..3187283333c89c 100644 --- a/tests/specs/install/no_future_install_global/install.out +++ b/tests/specs/install/no_future_install_global/install.out @@ -2,6 +2,4 @@ Download http://localhost:4545/npm/registry/@denotest/esm-basic Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz ✅ Successfully installed deno-cli-test[WILDCARD] -/[WILDCARD]/bins/bin/deno-cli-test[WILDCARD] -[WILDLINE] -[WILDLINE] +[WILDCARD] From 59e798dadb195065ed9ba78ba09055227d02c04f Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Fri, 26 Apr 2024 11:10:33 -0400 Subject: [PATCH 10/32] Npm flag + wire up install flags --- cli/args/flags.rs | 46 ++++++++++++++++++++++++++++++++++++++---- cli/tools/installer.rs | 14 ++++++++++--- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 603de835e70721..ddfe637a7a652e 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -48,6 +48,7 @@ pub struct FileFlags { #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct AddFlags { + pub npm: bool, pub packages: Vec, } @@ -196,7 +197,7 @@ pub struct InstallFlagsGlobal { #[derive(Clone, Debug, Eq, PartialEq)] pub enum InstallKind { #[allow(unused)] - Local, + Local(Option), Global(InstallFlagsGlobal), } @@ -1338,6 +1339,19 @@ fn clap_root() -> Command { .after_help(ENV_VARIABLES_HELP) } +fn add_subcommand_args() -> impl IntoIterator { + [ + Arg::new("npm") + .help("Look up packages in npm if unspecified") + .action(ArgAction::SetTrue), + Arg::new("packages") + .help("List of packages to add") + .required(true) + .num_args(1..) + .action(ArgAction::Append), + ] +} + fn add_subcommand() -> Command { Command::new("add") .about("Add dependencies") @@ -2055,6 +2069,15 @@ TypeScript compiler cache: Subdirectory containing TS compiler output.", )) } +fn future_install_subcommand() -> Command { + Command::new("install") + .about("Install dependencies") + .long_about( + "Installs dependencies either in the local project or globally to a bin directory. + +") +} + fn install_subcommand() -> Command { Command::new("install") .about("Install script as an executable") @@ -3575,8 +3598,18 @@ fn unsafely_ignore_certificate_errors_arg() -> Arg { } fn add_parse(flags: &mut Flags, matches: &mut ArgMatches) { - let packages = matches.remove_many::("packages").unwrap().collect(); - flags.subcommand = DenoSubcommand::Add(AddFlags { packages }); + flags.subcommand = DenoSubcommand::Add(add_parse_inner(matches, None)); +} + +fn add_parse_inner( + matches: &mut ArgMatches, + packages: Option>, +) -> AddFlags { + let npm: bool = matches.get_flag("npm"); + let packages = packages + .unwrap_or_else(|| matches.remove_many::("packages").unwrap()) + .collect(); + AddFlags { packages, npm } } fn bench_parse(flags: &mut Flags, matches: &mut ArgMatches) { @@ -3915,9 +3948,12 @@ fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) { }), }); } else { + let local_flags = matches + .remove_many("packages") + .map(|packages| add_parse_inner(matches, Some(packages))); flags.subcommand = DenoSubcommand::Install(InstallFlags { global, - kind: InstallKind::Local, + kind: InstallKind::Local(local_flags), }) } } @@ -9634,6 +9670,7 @@ mod tests { Flags { subcommand: DenoSubcommand::Add(AddFlags { packages: svec!["@david/which"], + ..Default::default() }), ..Flags::default() } @@ -9645,6 +9682,7 @@ mod tests { Flags { subcommand: DenoSubcommand::Add(AddFlags { packages: svec!["@david/which", "@luca/hello"], + ..Default::default() }), ..Flags::default() } diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index 1e468ffb19c9e7..10a654d9ed25a7 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -1,6 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use crate::args::resolve_no_prompt; +use crate::args::AddFlags; use crate::args::CaData; use crate::args::Flags; use crate::args::InstallFlags; @@ -253,10 +254,15 @@ pub fn uninstall(uninstall_flags: UninstallFlags) -> Result<(), AnyError> { Ok(()) } -async fn install_local(flags: Flags) -> Result<(), AnyError> { +async fn install_local( + flags: Flags, + maybe_add_flags: Option, +) -> Result<(), AnyError> { + if let Some(add_flags) = maybe_add_flags { + return super::registry::add(flags, add_flags).await; + } let factory = CliFactory::from_flags(flags)?; let npm_resolver = factory.npm_resolver().await?; - if let Some(npm_resolver) = npm_resolver.as_managed() { npm_resolver.ensure_top_level_package_json_install().await?; npm_resolver.resolve_pending().await?; @@ -274,7 +280,9 @@ pub async fn install_command( let install_flags_global = match install_flags.kind { InstallKind::Global(flags) => flags, - InstallKind::Local => return install_local(flags).await, + InstallKind::Local(maybe_add_flags) => { + return install_local(flags, maybe_add_flags).await + } }; // ensure the module is cached From 6414516071beeb5b192af9132fad215c2c2beee9 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Fri, 26 Apr 2024 13:29:06 -0400 Subject: [PATCH 11/32] Factor out a separate future install command --- cli/args/flags.rs | 143 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 100 insertions(+), 43 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index ddfe637a7a652e..b29a04c0cff82c 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1322,7 +1322,11 @@ fn clap_root() -> Command { .subcommand(fmt_subcommand()) .subcommand(init_subcommand()) .subcommand(info_subcommand()) - .subcommand(install_subcommand()) + .subcommand(if *DENO_FUTURE { + future_install_subcommand() + } else { + install_subcommand() + }) .subcommand(jupyter_subcommand()) .subcommand(uninstall_subcommand()) .subcommand(lsp_subcommand()) @@ -2069,20 +2073,109 @@ TypeScript compiler cache: Subdirectory containing TS compiler output.", )) } +fn install_args(cmd: Command, deno_future: bool) -> Command { + let cmd = if deno_future { + cmd.arg( + Arg::new("cmd") + .required_if_eq("global", "true") + .num_args(1..) + .value_hint(ValueHint::FilePath), + ) + } else { + cmd.arg( + Arg::new("cmd") + .required(true) + .num_args(1..) + .value_hint(ValueHint::FilePath), + ) + }; + cmd + .arg( + Arg::new("name") + .long("name") + .short('n') + .help("Executable file name") + .required(false), + ) + .arg( + Arg::new("root") + .long("root") + .help("Installation root") + .value_hint(ValueHint::DirPath), + ) + .arg( + Arg::new("force") + .long("force") + .short('f') + .help("Forcefully overwrite existing installation") + .action(ArgAction::SetTrue), + ) + .arg( + Arg::new("global") + .long("global") + .short('g') + .help("Install a package or script as a globally available executable") + .action(ArgAction::SetTrue), + ) + .arg(env_file_arg()) +} + fn future_install_subcommand() -> Command { Command::new("install") .about("Install dependencies") .long_about( - "Installs dependencies either in the local project or globally to a bin directory. - -") +"Installs dependencies either in the local project or globally to a bin directory. + +Local installation +------------------- +If the --global flag is not set, adds dependencies to the local project's configuration +(package.json / deno.json) and installs them in the package cache. If no dependency +is specified, installs all dependencies listed in package.json. + + deno install + deno install @std/bytes + deno install npm:chalk + +Global installation +------------------- +If the --global flag is set, installs a script as an executable in the installation root's bin directory. + + deno install --global --allow-net --allow-read jsr:@std/http/file-server + deno install -g https://examples.deno.land/color-logging.ts + +To change the executable name, use -n/--name: + + deno install -g --allow-net --allow-read -n serve jsr:@std/http/file-server + +The executable name is inferred by default: + - Attempt to take the file stem of the URL path. The above example would + become 'file_server'. + - If the file stem is something generic like 'main', 'mod', 'index' or 'cli', + and the path has no parent, take the file name of the parent path. Otherwise + settle with the generic name. + - If the resulting name has an '@...' suffix, strip it. + +To change the installation root, use --root: + + deno install -g --allow-net --allow-read --root /usr/local jsr:@std/http/file-server + +The installation root is determined, in order of precedence: + - --root option + - DENO_INSTALL_ROOT environment variable + - $HOME/.deno + +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); + }) } fn install_subcommand() -> Command { Command::new("install") .about("Install script as an executable") .long_about( - "Installs a script as an executable in the installation root's bin directory. +"Installs a script as an executable in the installation root's bin directory. deno install --global --allow-net --allow-read jsr:@std/http/file-server deno install -g https://examples.deno.land/color-logging.ts @@ -2110,44 +2203,8 @@ 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 = if *DENO_FUTURE { - cmd.arg(Arg::new("cmd").required_if_eq("global", "true").num_args(1..).value_hint(ValueHint::FilePath)) - } else { - cmd.arg(Arg::new("cmd").required(true).num_args(1..).value_hint(ValueHint::FilePath)) - }; - - cmd.arg( - Arg::new("name") - .long("name") - .short('n') - .help("Executable file name") - .required(false) - ) - .arg( - Arg::new("root") - .long("root") - .help("Installation root") - .value_hint(ValueHint::DirPath) - ) - .arg( - Arg::new("force") - .long("force") - .short('f') - .help("Forcefully overwrite existing installation") - .action(ArgAction::SetTrue) - ) - .arg( - Arg::new("global") - .long("global") - .short('g') - .help("Install a package or script as a globally available executable") - .action(ArgAction::SetTrue) - ) - .arg(env_file_arg()) + let cmd = runtime_args(cmd, true, true).arg(check_arg(true)); + install_args(cmd, false) }) } From b3d259cc91509a8eb9b097e03823ae726b68b2d6 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Fri, 26 Apr 2024 13:30:33 -0400 Subject: [PATCH 12/32] Include add args --- cli/args/flags.rs | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index b29a04c0cff82c..11b215bd72074b 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1343,17 +1343,21 @@ fn clap_root() -> Command { .after_help(ENV_VARIABLES_HELP) } -fn add_subcommand_args() -> impl IntoIterator { - [ - Arg::new("npm") - .help("Look up packages in npm if unspecified") - .action(ArgAction::SetTrue), - Arg::new("packages") - .help("List of packages to add") - .required(true) - .num_args(1..) - .action(ArgAction::Append), - ] +fn add_args(cmd: Command) -> Command { + cmd + .arg( + Arg::new("npm") + .long("npm") + .help("Look up packages in npm if unspecified") + .action(ArgAction::SetTrue), + ) + .arg( + Arg::new("packages") + .help("List of packages to add") + .required(true) + .num_args(1..) + .action(ArgAction::Append), + ) } fn add_subcommand() -> Command { @@ -1369,15 +1373,7 @@ You can add multiple dependencies at once: deno add @std/path @std/assert ", ) - .defer(|cmd| { - cmd.arg( - Arg::new("packages") - .help("List of packages to add") - .required(true) - .num_args(1..) - .action(ArgAction::Append), - ) - }) + .defer(|cmd| add_args(cmd)) } fn bench_subcommand() -> Command { @@ -2168,6 +2164,7 @@ 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) }) } From e875d3b356a572cfad4ccbb6e2c1d67235ad7052 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Fri, 26 Apr 2024 17:35:50 -0400 Subject: [PATCH 13/32] Fix args --- cli/args/flags.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 11b215bd72074b..93ae44a1290f21 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1343,21 +1343,24 @@ fn clap_root() -> Command { .after_help(ENV_VARIABLES_HELP) } -fn add_args(cmd: Command) -> Command { - cmd - .arg( - Arg::new("npm") - .long("npm") - .help("Look up packages in npm if unspecified") - .action(ArgAction::SetTrue), - ) - .arg( +fn add_args(cmd: Command, include_packages: bool) -> Command { + let cmd = cmd.arg( + Arg::new("npm") + .long("npm") + .help("Look up packages in npm if unspecified") + .action(ArgAction::SetTrue), + ); + 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 { @@ -1373,7 +1376,7 @@ You can add multiple dependencies at once: deno add @std/path @std/assert ", ) - .defer(|cmd| add_args(cmd)) + .defer(|cmd| add_args(cmd, true)) } fn bench_subcommand() -> Command { @@ -2072,7 +2075,7 @@ TypeScript compiler cache: Subdirectory containing TS compiler output.", fn install_args(cmd: Command, deno_future: bool) -> Command { let cmd = if deno_future { cmd.arg( - Arg::new("cmd") + Arg::new("packages") .required_if_eq("global", "true") .num_args(1..) .value_hint(ValueHint::FilePath), @@ -2164,7 +2167,7 @@ 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) + add_args(cmd, false) }) } From 7fb272d9cb17fcd2953bf44cce13eeb76fdc7a73 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Sat, 27 Apr 2024 01:16:05 -0400 Subject: [PATCH 14/32] Deno add / install works on package.json --- cli/tools/registry/pm.rs | 205 ++++++++++++++++++++++++++++++--------- 1 file changed, 161 insertions(+), 44 deletions(-) diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index 699b476cb9b68f..d0b4827fe3233f 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -1,19 +1,22 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; use deno_ast::TextChange; use deno_config::FmtOptionsConfig; +use deno_core::anyhow::anyhow; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::futures::FutureExt; use deno_core::futures::StreamExt; use deno_core::serde_json; +use deno_core::ModuleSpecifier; +use deno_runtime::deno_node; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; +use indexmap::IndexMap; use jsonc_parser::ast::ObjectProp; use jsonc_parser::ast::Value; @@ -25,22 +28,133 @@ use crate::file_fetcher::FileFetcher; use crate::jsr::JsrFetchResolver; use crate::npm::NpmFetchResolver; +enum ConfigFile { + Deno(deno_config::ConfigFile), + Npm(deno_node::PackageJson, Option), +} + +impl ConfigFile { + fn specifier(&self) -> ModuleSpecifier { + match self { + Self::Deno(d) => d.specifier.clone(), + Self::Npm(n, ..) => n.specifier(), + } + } + + /// Returns the existing imports/dependencies from the config. + fn existing_imports(&self) -> Result, AnyError> { + match self { + ConfigFile::Deno(deno) => { + if let Some(imports) = deno.json.imports.clone() { + match serde_json::from_value(imports) { + Ok(map) => Ok(map), + Err(err) => { + bail!("Malformed \"imports\" configuration: {err}") + } + } + } else { + Ok(Default::default()) + } + } + ConfigFile::Npm(npm, ..) => { + Ok(npm.dependencies.clone().unwrap_or_default()) + } + } + } + + fn fmt_options(&self) -> FmtOptionsConfig { + match self { + ConfigFile::Deno(deno) => deno + .to_fmt_config() + .ok() + .flatten() + .map(|f| f.options) + .unwrap_or_default(), + ConfigFile::Npm(_, config) => config.clone().unwrap_or_default(), + } + } + + fn imports_key(&self) -> &'static str { + match self { + ConfigFile::Deno(_) => "imports", + ConfigFile::Npm(..) => "dependencies", + } + } + + fn file_name(&self) -> &'static str { + match self { + ConfigFile::Deno(_) => "deno.json", + ConfigFile::Npm(..) => "package.json", + } + } + + fn is_npm(&self) -> bool { + matches!(self, Self::Npm(..)) + } + + /// Get the preferred config file to operate on + /// given the flags. If no config file is present, + /// 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, + add_flags: &AddFlags, + ) -> 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, it's hard to come up with a consistent + // and easy to explain behavior. for now, + // only choose the `package.json` if the `--npm` flag is + // passed + (Some(deno), Some(package_json)) if add_flags.npm => Ok(( + ConfigFile::Npm( + package_json.clone(), + deno.to_fmt_config()?.map(|f| f.options), + ), + factory, + )), + (Some(deno), Some(_) | None) => { + Ok((ConfigFile::Deno(deno.clone()), factory)) + } + (None, Some(package_json)) => { + Ok((ConfigFile::Npm(package_json.clone(), None), factory)) + } + (None, None) => { + tokio::fs::write(options.initial_cwd().join("deno.json"), "{}\n") + .await + .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( + new_options + .maybe_config_file() + .as_ref() + .ok_or_else(|| { + anyhow!("config not found, but it was just created") + })? + .clone(), + ), + new_factory, + )) + } + } + } +} + pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { - let cli_factory = CliFactory::from_flags(flags.clone())?; - let cli_options = cli_factory.cli_options(); - - let Some(config_file) = cli_options.maybe_config_file() else { - tokio::fs::write(cli_options.initial_cwd().join("deno.json"), "{}\n") - .await - .context("Failed to create deno.json file")?; - log::info!("Created deno.json configuration file."); - return add(flags, add_flags).boxed_local().await; - }; + let (config_file, cli_factory) = + ConfigFile::from_flags(flags, &add_flags).await?; - if config_file.specifier.scheme() != "file" { + let config_specifier = config_file.specifier(); + if config_specifier.scheme() != "file" { bail!("Can't add dependencies to a remote configuration file"); } - let config_file_path = config_file.specifier.to_file_path().unwrap(); + let config_file_path = config_specifier.to_file_path().unwrap(); let http_client = cli_factory.http_client(); @@ -48,11 +162,16 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { let mut package_reqs = Vec::with_capacity(add_flags.packages.len()); for package_name in add_flags.packages.iter() { - let req = if package_name.starts_with("npm:") { - let pkg_req = NpmPackageReqReference::from_str(package_name) - .with_context(|| { - format!("Failed to parse package required: {}", package_name) - })?; + let req = if package_name.starts_with("npm:") + || (add_flags.npm && !package_name.starts_with("jsr")) + { + let pkg_req = NpmPackageReqReference::from_str(&format!( + "npm:{}", + package_name.strip_prefix("npm:").unwrap_or(package_name) + )) + .with_context(|| { + format!("Failed to parse package required: {}", package_name) + })?; AddPackageReq::Npm(pkg_req) } else { let pkg_req = JsrPackageReqReference::from_str(&format!( @@ -128,16 +247,9 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { _ => bail!("Failed updating config file due to no object."), }; - let mut existing_imports = - if let Some(imports) = config_file.json.imports.clone() { - match serde_json::from_value::>(imports) { - Ok(i) => i, - Err(_) => bail!("Malformed \"imports\" configuration"), - } - } else { - HashMap::default() - }; + let mut existing_imports = config_file.existing_imports()?; + let is_npm = config_file.is_npm(); for selected_package in selected_packages { log::info!( "Add {} - {}@{}", @@ -145,13 +257,19 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { selected_package.package_name, selected_package.version_req ); - existing_imports.insert( - selected_package.import_name, - format!( - "{}@{}", - selected_package.package_name, selected_package.version_req - ), - ); + + if is_npm { + existing_imports + .insert(selected_package.package_name, selected_package.version_req) + } else { + existing_imports.insert( + selected_package.import_name, + format!( + "{}@{}", + selected_package.package_name, selected_package.version_req + ), + ) + }; } let mut import_list: Vec<(String, String)> = existing_imports.into_iter().collect(); @@ -159,18 +277,15 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { import_list.sort_by(|(k1, _), (k2, _)| k1.cmp(k2)); let generated_imports = generate_imports(import_list); - let fmt_config_options = config_file - .to_fmt_config() - .ok() - .flatten() - .map(|config| config.options) - .unwrap_or_default(); + let fmt_config_options = config_file.fmt_options(); let new_text = update_config_file_content( obj, &config_file_contents, generated_imports, fmt_config_options, + config_file.imports_key(), + config_file.file_name(), ); tokio::fs::write(&config_file_path, new_text) @@ -259,10 +374,12 @@ fn update_config_file_content( config_file_contents: &str, generated_imports: String, fmt_options: FmtOptionsConfig, + imports_key: &str, + file_name: &str, ) -> String { let mut text_changes = vec![]; - match obj.get("imports") { + match obj.get(imports_key) { Some(ObjectProp { value: Value::Object(lit), .. @@ -282,10 +399,10 @@ fn update_config_file_content( // "": ":@" // } // } - new_text: format!("\"imports\": {{\n {} }}", generated_imports), + new_text: format!("\"{imports_key}\": {{\n {generated_imports} }}"), }) } - // we verified the shape of `imports` above + // we verified the shape of `imports`/`dependencies` above Some(_) => unreachable!(), } @@ -293,7 +410,7 @@ fn update_config_file_content( deno_ast::apply_text_changes(config_file_contents, text_changes); crate::tools::fmt::format_json( - &PathBuf::from("deno.json"), + &PathBuf::from(file_name), &new_text, &fmt_options, ) From 6ef7c984ccae56b2029f22e5c8c74122f509d9a2 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Mon, 29 Apr 2024 10:21:33 -0700 Subject: [PATCH 15/32] Fix arg --- cli/args/flags.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 93ae44a1290f21..7e7e0465546497 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -2075,7 +2075,7 @@ TypeScript compiler cache: Subdirectory containing TS compiler output.", fn install_args(cmd: Command, deno_future: bool) -> Command { let cmd = if deno_future { cmd.arg( - Arg::new("packages") + Arg::new("cmd") .required_if_eq("global", "true") .num_args(1..) .value_hint(ValueHint::FilePath), @@ -4006,7 +4006,7 @@ fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) { }); } else { let local_flags = matches - .remove_many("packages") + .remove_many("cmd") .map(|packages| add_parse_inner(matches, Some(packages))); flags.subcommand = DenoSubcommand::Install(InstallFlags { global, From c584b8ffc6116179f4ef28663b24b52e1052a43c Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Mon, 29 Apr 2024 10:23:05 -0700 Subject: [PATCH 16/32] Handle dependency entry for package.json properly --- cli/tools/registry/pm.rs | 44 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index d0b4827fe3233f..dbdf8c3f4f1354 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -146,9 +146,37 @@ 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); + let jsr_package = jsr_package.strip_prefix("@").unwrap_or(jsr_package); + let scope_replaced = jsr_package.replace('/', "__"); + let version_req = + format!("npm:@jsr/{scope_replaced}@{}", selected.version_req); + (selected.import_name, version_req) + } else { + (selected.package_name, selected.version_req) + } +} + pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { let (config_file, cli_factory) = - ConfigFile::from_flags(flags, &add_flags).await?; + ConfigFile::from_flags(flags.clone(), &add_flags).await?; let config_specifier = config_file.specifier(); if config_specifier.scheme() != "file" { @@ -259,8 +287,8 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { ); if is_npm { - existing_imports - .insert(selected_package.package_name, selected_package.version_req) + let (name, version) = package_json_dependency_entry(selected_package); + existing_imports.insert(name, version) } else { existing_imports.insert( selected_package.import_name, @@ -292,7 +320,15 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { .await .context("Failed to update configuration file")?; - // TODO(bartlomieju): we should now cache the imports from the config file. + // TODO(bartlomieju): we should now cache the imports from the deno.json. + + // make a new CliFactory to pick up the updated config file + let cli_factory = CliFactory::from_flags(flags)?; + let npm_resolver = cli_factory.npm_resolver().await?; + if let Some(npm_resolver) = npm_resolver.as_managed() { + npm_resolver.ensure_top_level_package_json_install().await?; + npm_resolver.resolve_pending().await?; + } Ok(()) } From 4cb4217ec334e8666db06cfbcc04f1f362c02689 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Mon, 29 Apr 2024 11:10:13 -0700 Subject: [PATCH 17/32] Appease linter --- cli/tools/registry/pm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index dbdf8c3f4f1354..eef7010f72dc4f 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -164,7 +164,7 @@ fn package_json_dependency_entry( ) } else if selected.package_name.starts_with("jsr:") { let jsr_package = strip_specifier_prefix(&selected.package_name); - let jsr_package = jsr_package.strip_prefix("@").unwrap_or(jsr_package); + let jsr_package = jsr_package.strip_prefix('@').unwrap_or(jsr_package); let scope_replaced = jsr_package.replace('/', "__"); let version_req = format!("npm:@jsr/{scope_replaced}@{}", selected.version_req); From 0d806a168e3ab9a22d3842304299f9f750d4f00b Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Mon, 29 Apr 2024 11:10:54 -0700 Subject: [PATCH 18/32] Add test for `deno install npm:` under DENO_FUTURE --- .../future_install_local_npm/__test__.jsonc | 19 +++++++++++++++++++ .../future_install_local_npm/install.out | 5 +++++ .../install/future_install_local_npm/main.js | 2 ++ .../install/future_install_local_npm/main.out | 0 .../future_install_local_npm/package.json | 3 +++ 5 files changed, 29 insertions(+) create mode 100644 tests/specs/install/future_install_local_npm/__test__.jsonc create mode 100644 tests/specs/install/future_install_local_npm/install.out create mode 100644 tests/specs/install/future_install_local_npm/main.js create mode 100644 tests/specs/install/future_install_local_npm/main.out create mode 100644 tests/specs/install/future_install_local_npm/package.json diff --git a/tests/specs/install/future_install_local_npm/__test__.jsonc b/tests/specs/install/future_install_local_npm/__test__.jsonc new file mode 100644 index 00000000000000..bbb86d69cdd11b --- /dev/null +++ b/tests/specs/install/future_install_local_npm/__test__.jsonc @@ -0,0 +1,19 @@ +{ + "tempDir": true, + "envs": { + "DENO_FUTURE": "1" + }, + "steps": [ + { + "cleanDenoDir": true, + "args": "install npm:@denotest/esm-basic", + "output": "install.out" + }, + { + "cleanDenoDir": true, + "args": "run --cached-only main.js", + "exitCode": 0, + "output": "main.out" + } + ] +} diff --git a/tests/specs/install/future_install_local_npm/install.out b/tests/specs/install/future_install_local_npm/install.out new file mode 100644 index 00000000000000..e0cf4b51a4afae --- /dev/null +++ b/tests/specs/install/future_install_local_npm/install.out @@ -0,0 +1,5 @@ +⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. +Add @denotest/esm-basic - npm:@denotest/esm-basic@^1.0.0 +Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Initialize @denotest/esm-basic@1.0.0 diff --git a/tests/specs/install/future_install_local_npm/main.js b/tests/specs/install/future_install_local_npm/main.js new file mode 100644 index 00000000000000..c29b48b4ed5967 --- /dev/null +++ b/tests/specs/install/future_install_local_npm/main.js @@ -0,0 +1,2 @@ +import { setValue } from "@denotest/esm-basic"; +setValue(5); diff --git a/tests/specs/install/future_install_local_npm/main.out b/tests/specs/install/future_install_local_npm/main.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/specs/install/future_install_local_npm/package.json b/tests/specs/install/future_install_local_npm/package.json new file mode 100644 index 00000000000000..18a1e415e56220 --- /dev/null +++ b/tests/specs/install/future_install_local_npm/package.json @@ -0,0 +1,3 @@ +{ + "dependencies": {} +} From 1ed7eecacec6ba4260f7f1435b516eb4ab031b04 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Mon, 29 Apr 2024 12:39:40 -0700 Subject: [PATCH 19/32] Cache import map deps --- cli/module_loader.rs | 37 +++++++++++++++++++++++++++++++++++++ cli/tools/installer.rs | 8 +++----- cli/tools/registry/pm.rs | 7 ++----- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/cli/module_loader.rs b/cli/module_loader.rs index a6c8d133810d06..f07aa21a737064 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -8,6 +8,7 @@ use crate::cache::CodeCache; use crate::cache::ModuleInfoCache; use crate::cache::ParsedSourceCache; use crate::emit::Emitter; +use crate::factory::CliFactory; use crate::graph_util::graph_lock_or_exit; use crate::graph_util::CreateGraphOptions; use crate::graph_util::ModuleGraphBuilder; @@ -64,6 +65,42 @@ use std::rc::Rc; use std::str; use std::sync::Arc; +pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> { + let npm_resolver = factory.npm_resolver().await?; + if let Some(npm_resolver) = npm_resolver.as_managed() { + npm_resolver.ensure_top_level_package_json_install().await?; + npm_resolver.resolve_pending().await?; + } + // cache as many entries in the import map as we can + if let Some(import_map) = factory.maybe_import_map().await? { + let roots = import_map + .imports() + .entries() + .filter_map(|entry| { + if entry.key.ends_with('/') { + None + } else if let Some(value) = entry.value { + Some(value.clone()) + } else { + None + } + }) + .collect(); + factory + .module_load_preparer() + .await? + .prepare_module_load( + roots, + false, + factory.cli_options().ts_type_lib_window(), + deno_runtime::permissions::PermissionsContainer::allow_all(), + ) + .await?; + } + + Ok(()) +} + pub struct ModuleLoadPreparer { options: Arc, graph_container: Arc, diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index 10a654d9ed25a7..9ff17ccc304740 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -261,12 +261,10 @@ async fn install_local( if let Some(add_flags) = maybe_add_flags { return super::registry::add(flags, add_flags).await; } + let factory = CliFactory::from_flags(flags)?; - let npm_resolver = factory.npm_resolver().await?; - if let Some(npm_resolver) = npm_resolver.as_managed() { - npm_resolver.ensure_top_level_package_json_install().await?; - npm_resolver.resolve_pending().await?; - } + crate::module_loader::load_top_level_deps(&factory).await?; + Ok(()) } diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index eef7010f72dc4f..ad9dfd6d9b1189 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -324,11 +324,8 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { // make a new CliFactory to pick up the updated config file let cli_factory = CliFactory::from_flags(flags)?; - let npm_resolver = cli_factory.npm_resolver().await?; - if let Some(npm_resolver) = npm_resolver.as_managed() { - npm_resolver.ensure_top_level_package_json_install().await?; - npm_resolver.resolve_pending().await?; - } + // cache deps + crate::module_loader::load_top_level_deps(&cli_factory).await?; Ok(()) } From c60aca51e17adcaac1f3b682a261464aa960aeed Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Mon, 29 Apr 2024 13:44:28 -0700 Subject: [PATCH 20/32] Test caching (+ some renames) --- .../__test__.jsonc | 18 ++++++++++++++++++ .../future_install_local_add_deno/deno.json | 1 + .../future_install_local_add_deno/install.out | 4 ++++ .../main.js | 0 .../main.out | 0 .../__test__.jsonc | 0 .../install.out | 0 .../future_install_local_add_npm/main.js | 2 ++ .../future_install_local_add_npm/main.out | 0 .../package.json | 0 .../future_install_local_deno/__test__.jsonc | 18 ++++++++++++++++++ .../future_install_local_deno/deno.json | 8 ++++++++ .../future_install_local_deno/install.out | 11 +++++++++++ .../install/future_install_local_deno/main.js | 2 ++ .../install/future_install_local_deno/main.out | 0 15 files changed, 64 insertions(+) create mode 100644 tests/specs/install/future_install_local_add_deno/__test__.jsonc create mode 100644 tests/specs/install/future_install_local_add_deno/deno.json create mode 100644 tests/specs/install/future_install_local_add_deno/install.out rename tests/specs/install/{future_install_local_npm => future_install_local_add_deno}/main.js (100%) rename tests/specs/install/{future_install_local_npm => future_install_local_add_deno}/main.out (100%) rename tests/specs/install/{future_install_local_npm => future_install_local_add_npm}/__test__.jsonc (100%) rename tests/specs/install/{future_install_local_npm => future_install_local_add_npm}/install.out (100%) create mode 100644 tests/specs/install/future_install_local_add_npm/main.js create mode 100644 tests/specs/install/future_install_local_add_npm/main.out rename tests/specs/install/{future_install_local_npm => future_install_local_add_npm}/package.json (100%) create mode 100644 tests/specs/install/future_install_local_deno/__test__.jsonc create mode 100644 tests/specs/install/future_install_local_deno/deno.json create mode 100644 tests/specs/install/future_install_local_deno/install.out create mode 100644 tests/specs/install/future_install_local_deno/main.js create mode 100644 tests/specs/install/future_install_local_deno/main.out diff --git a/tests/specs/install/future_install_local_add_deno/__test__.jsonc b/tests/specs/install/future_install_local_add_deno/__test__.jsonc new file mode 100644 index 00000000000000..49313f90d2dcd9 --- /dev/null +++ b/tests/specs/install/future_install_local_add_deno/__test__.jsonc @@ -0,0 +1,18 @@ +{ + "tempDir": true, + "envs": { + "DENO_FUTURE": "1" + }, + "steps": [ + { + "cleanDenoDir": true, + "args": "install npm:@denotest/esm-basic", + "output": "install.out" + }, + { + "args": "run --cached-only main.js", + "exitCode": 0, + "output": "main.out" + } + ] +} diff --git a/tests/specs/install/future_install_local_add_deno/deno.json b/tests/specs/install/future_install_local_add_deno/deno.json new file mode 100644 index 00000000000000..0967ef424bce67 --- /dev/null +++ b/tests/specs/install/future_install_local_add_deno/deno.json @@ -0,0 +1 @@ +{} diff --git a/tests/specs/install/future_install_local_add_deno/install.out b/tests/specs/install/future_install_local_add_deno/install.out new file mode 100644 index 00000000000000..1ed999c762596a --- /dev/null +++ b/tests/specs/install/future_install_local_add_deno/install.out @@ -0,0 +1,4 @@ +⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. +Add @denotest/esm-basic - npm:@denotest/esm-basic@^1.0.0 +Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz diff --git a/tests/specs/install/future_install_local_npm/main.js b/tests/specs/install/future_install_local_add_deno/main.js similarity index 100% rename from tests/specs/install/future_install_local_npm/main.js rename to tests/specs/install/future_install_local_add_deno/main.js diff --git a/tests/specs/install/future_install_local_npm/main.out b/tests/specs/install/future_install_local_add_deno/main.out similarity index 100% rename from tests/specs/install/future_install_local_npm/main.out rename to tests/specs/install/future_install_local_add_deno/main.out diff --git a/tests/specs/install/future_install_local_npm/__test__.jsonc b/tests/specs/install/future_install_local_add_npm/__test__.jsonc similarity index 100% rename from tests/specs/install/future_install_local_npm/__test__.jsonc rename to tests/specs/install/future_install_local_add_npm/__test__.jsonc diff --git a/tests/specs/install/future_install_local_npm/install.out b/tests/specs/install/future_install_local_add_npm/install.out similarity index 100% rename from tests/specs/install/future_install_local_npm/install.out rename to tests/specs/install/future_install_local_add_npm/install.out diff --git a/tests/specs/install/future_install_local_add_npm/main.js b/tests/specs/install/future_install_local_add_npm/main.js new file mode 100644 index 00000000000000..c29b48b4ed5967 --- /dev/null +++ b/tests/specs/install/future_install_local_add_npm/main.js @@ -0,0 +1,2 @@ +import { setValue } from "@denotest/esm-basic"; +setValue(5); diff --git a/tests/specs/install/future_install_local_add_npm/main.out b/tests/specs/install/future_install_local_add_npm/main.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/specs/install/future_install_local_npm/package.json b/tests/specs/install/future_install_local_add_npm/package.json similarity index 100% rename from tests/specs/install/future_install_local_npm/package.json rename to tests/specs/install/future_install_local_add_npm/package.json diff --git a/tests/specs/install/future_install_local_deno/__test__.jsonc b/tests/specs/install/future_install_local_deno/__test__.jsonc new file mode 100644 index 00000000000000..8237af02e382df --- /dev/null +++ b/tests/specs/install/future_install_local_deno/__test__.jsonc @@ -0,0 +1,18 @@ +{ + "tempDir": true, + "envs": { + "DENO_FUTURE": "1" + }, + "steps": [ + { + "cleanDenoDir": true, + "args": "install", + "output": "install.out" + }, + { + "args": "run --cached-only main.js", + "exitCode": 0, + "output": "main.out" + } + ] +} diff --git a/tests/specs/install/future_install_local_deno/deno.json b/tests/specs/install/future_install_local_deno/deno.json new file mode 100644 index 00000000000000..ba1b6cc3e06add --- /dev/null +++ b/tests/specs/install/future_install_local_deno/deno.json @@ -0,0 +1,8 @@ +{ + "imports": { + "@std/fs/": "https://deno.land/std@0.224.0/fs/", + "@denotest/esm-basic": "npm:@denotest/esm-basic@^1.0.0", + "@denotest/no_module_graph": "jsr:@denotest/no_module_graph@*", + "test-http": "http://localhost:4545/v1/extensionless" + } +} diff --git a/tests/specs/install/future_install_local_deno/install.out b/tests/specs/install/future_install_local_deno/install.out new file mode 100644 index 00000000000000..c52e7bebb1bff4 --- /dev/null +++ b/tests/specs/install/future_install_local_deno/install.out @@ -0,0 +1,11 @@ +⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. +Download http://localhost:4545/v1/extensionless +Download http://localhost:4545/subdir/mod1.ts +Download http://localhost:4545/subdir/subdir2/mod2.ts +Download http://localhost:4545/subdir/print_hello.ts +Download http://127.0.0.1:4250/@denotest/no_module_graph/meta.json +Download http://127.0.0.1:4250/@denotest/no_module_graph/0.2.0_meta.json +Download http://127.0.0.1:4250/@denotest/no_module_graph/0.2.0/mod.ts +Download http://127.0.0.1:4250/@denotest/no_module_graph/0.2.0/TestClass.ts +Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz diff --git a/tests/specs/install/future_install_local_deno/main.js b/tests/specs/install/future_install_local_deno/main.js new file mode 100644 index 00000000000000..c29b48b4ed5967 --- /dev/null +++ b/tests/specs/install/future_install_local_deno/main.js @@ -0,0 +1,2 @@ +import { setValue } from "@denotest/esm-basic"; +setValue(5); diff --git a/tests/specs/install/future_install_local_deno/main.out b/tests/specs/install/future_install_local_deno/main.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 From 72d0677dbf110f378e66f4f5fcb76ade87a4421e Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Mon, 29 Apr 2024 13:52:28 -0700 Subject: [PATCH 21/32] Appease clippy --- cli/module_loader.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cli/module_loader.rs b/cli/module_loader.rs index f07aa21a737064..7d8cb130b296c8 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -79,10 +79,8 @@ pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> { .filter_map(|entry| { if entry.key.ends_with('/') { None - } else if let Some(value) = entry.value { - Some(value.clone()) } else { - None + entry.value.cloned() } }) .collect(); From f096499215a1338836ef9e539269ab6bba5032ef Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Mon, 29 Apr 2024 17:14:03 -0700 Subject: [PATCH 22/32] Fix test --- tests/specs/install/future_install_local_deno/deno.json | 2 +- tests/specs/install/future_install_local_deno/install.out | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/specs/install/future_install_local_deno/deno.json b/tests/specs/install/future_install_local_deno/deno.json index ba1b6cc3e06add..dbcf1c220315ef 100644 --- a/tests/specs/install/future_install_local_deno/deno.json +++ b/tests/specs/install/future_install_local_deno/deno.json @@ -2,7 +2,7 @@ "imports": { "@std/fs/": "https://deno.land/std@0.224.0/fs/", "@denotest/esm-basic": "npm:@denotest/esm-basic@^1.0.0", - "@denotest/no_module_graph": "jsr:@denotest/no_module_graph@*", + "@denotest/add": "jsr:@denotest/add", "test-http": "http://localhost:4545/v1/extensionless" } } diff --git a/tests/specs/install/future_install_local_deno/install.out b/tests/specs/install/future_install_local_deno/install.out index c52e7bebb1bff4..54b22fede57bc7 100644 --- a/tests/specs/install/future_install_local_deno/install.out +++ b/tests/specs/install/future_install_local_deno/install.out @@ -3,9 +3,8 @@ Download http://localhost:4545/v1/extensionless Download http://localhost:4545/subdir/mod1.ts Download http://localhost:4545/subdir/subdir2/mod2.ts Download http://localhost:4545/subdir/print_hello.ts -Download http://127.0.0.1:4250/@denotest/no_module_graph/meta.json -Download http://127.0.0.1:4250/@denotest/no_module_graph/0.2.0_meta.json -Download http://127.0.0.1:4250/@denotest/no_module_graph/0.2.0/mod.ts -Download http://127.0.0.1:4250/@denotest/no_module_graph/0.2.0/TestClass.ts +Download http://127.0.0.1:4250/@denotest/add/meta.json +Download http://127.0.0.1:4250/@denotest/add/1.0.0_meta.json +Download http://127.0.0.1:4250/@denotest/add/1.0.0/mod.ts Download http://localhost:4545/npm/registry/@denotest/esm-basic Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz From 9ac6330c9df692866f4f1a6e22553bbefecbf9dc Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Tue, 30 Apr 2024 15:03:08 -0700 Subject: [PATCH 23/32] Remove --npm for now, gate behavior change behind deno_future --- cli/args/flags.rs | 6 +----- cli/tools/registry/pm.rs | 32 ++++++++++---------------------- 2 files changed, 11 insertions(+), 27 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 7e7e0465546497..90cd048aa47bd6 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -48,7 +48,6 @@ pub struct FileFlags { #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct AddFlags { - pub npm: bool, pub packages: Vec, } @@ -3662,11 +3661,10 @@ fn add_parse_inner( matches: &mut ArgMatches, packages: Option>, ) -> AddFlags { - let npm: bool = matches.get_flag("npm"); let packages = packages .unwrap_or_else(|| matches.remove_many::("packages").unwrap()) .collect(); - AddFlags { packages, npm } + AddFlags { packages } } fn bench_parse(flags: &mut Flags, matches: &mut ArgMatches) { @@ -9727,7 +9725,6 @@ mod tests { Flags { subcommand: DenoSubcommand::Add(AddFlags { packages: svec!["@david/which"], - ..Default::default() }), ..Flags::default() } @@ -9739,7 +9736,6 @@ mod tests { Flags { subcommand: DenoSubcommand::Add(AddFlags { packages: svec!["@david/which", "@luca/hello"], - ..Default::default() }), ..Flags::default() } diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index ad9dfd6d9b1189..3007b9406c080f 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -97,32 +97,20 @@ 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, - add_flags: &AddFlags, - ) -> Result<(Self, CliFactory), AnyError> { + async 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, it's hard to come up with a consistent - // and easy to explain behavior. for now, - // only choose the `package.json` if the `--npm` flag is - // passed - (Some(deno), Some(package_json)) if add_flags.npm => Ok(( - ConfigFile::Npm( - package_json.clone(), - deno.to_fmt_config()?.map(|f| f.options), - ), - factory, - )), + // when both are present, for now, + // default to deno.json (Some(deno), Some(_) | None) => { Ok((ConfigFile::Deno(deno.clone()), factory)) } - (None, Some(package_json)) => { + (None, Some(package_json)) if options.enable_future_features() => { Ok((ConfigFile::Npm(package_json.clone(), None), factory)) } - (None, None) => { + (None, Some(_) | None) => { tokio::fs::write(options.initial_cwd().join("deno.json"), "{}\n") .await .context("Failed to create deno.json file")?; @@ -176,7 +164,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(), &add_flags).await?; + ConfigFile::from_flags(flags.clone()).await?; let config_specifier = config_file.specifier(); if config_specifier.scheme() != "file" { @@ -190,9 +178,7 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { let mut package_reqs = Vec::with_capacity(add_flags.packages.len()); for package_name in add_flags.packages.iter() { - let req = if package_name.starts_with("npm:") - || (add_flags.npm && !package_name.starts_with("jsr")) - { + let req = if package_name.starts_with("npm:") { let pkg_req = NpmPackageReqReference::from_str(&format!( "npm:{}", package_name.strip_prefix("npm:").unwrap_or(package_name) @@ -325,7 +311,9 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { // make a new CliFactory to pick up the updated config file let cli_factory = CliFactory::from_flags(flags)?; // cache deps - crate::module_loader::load_top_level_deps(&cli_factory).await?; + if cli_factory.cli_options().enable_future_features() { + crate::module_loader::load_top_level_deps(&cli_factory).await?; + } Ok(()) } From f6ad0c0e9345c0c71d384eac6e270cebc16e9866 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Wed, 1 May 2024 11:21:37 -0700 Subject: [PATCH 24/32] Test cleanup --- .../install/future_install_global/__test__.jsonc | 6 +++++- tests/specs/install/future_install_global/assert.js | 11 +++++++++++ .../main.out => future_install_global/assert.out} | 0 tests/specs/install/future_install_global/install.out | 2 +- .../future_install_local_add_deno/__test__.jsonc | 2 +- .../install/future_install_local_add_deno/main.js | 5 +++++ .../future_install_local_add_npm/__test__.jsonc | 2 +- .../install/future_install_local_add_npm/main.js | 8 ++++++++ tests/specs/install/future_install_local_deno/main.js | 4 ++++ .../install/future_install_node_modules/package.json | 3 +-- .../install/no_future_install_global/__test__.jsonc | 6 +++++- .../specs/install/no_future_install_global/assert.js | 11 +++++++++++ .../specs/install/no_future_install_global/assert.out | 0 .../install/no_future_install_global/install.out | 2 +- .../install/no_future_install_global/package.json | 3 +-- 15 files changed, 55 insertions(+), 10 deletions(-) create mode 100644 tests/specs/install/future_install_global/assert.js rename tests/specs/install/{no_future_install_global/main.out => future_install_global/assert.out} (100%) create mode 100644 tests/specs/install/no_future_install_global/assert.js create mode 100644 tests/specs/install/no_future_install_global/assert.out diff --git a/tests/specs/install/future_install_global/__test__.jsonc b/tests/specs/install/future_install_global/__test__.jsonc index 3bfbec2579bb63..fcae89f416ea23 100644 --- a/tests/specs/install/future_install_global/__test__.jsonc +++ b/tests/specs/install/future_install_global/__test__.jsonc @@ -5,8 +5,12 @@ }, "steps": [ { - "args": "install --global --root ./bins ./main.js", + "args": "install --global --root ./bins --name deno-test-bin ./main.js", "output": "install.out" + }, + { + "args": "run -A assert.js", + "output": "assert.out" } ] } diff --git a/tests/specs/install/future_install_global/assert.js b/tests/specs/install/future_install_global/assert.js new file mode 100644 index 00000000000000..c7a6a06807e5d6 --- /dev/null +++ b/tests/specs/install/future_install_global/assert.js @@ -0,0 +1,11 @@ +const dirs = Deno.readDir("./bins/bin"); + +let found = false; +for await (const entry of dirs) { + if (entry.name.includes("deno-test-bin")) { + found = true; + } +} +if (!found) { + throw new Error("Failed to find test bin"); +} diff --git a/tests/specs/install/no_future_install_global/main.out b/tests/specs/install/future_install_global/assert.out similarity index 100% rename from tests/specs/install/no_future_install_global/main.out rename to tests/specs/install/future_install_global/assert.out diff --git a/tests/specs/install/future_install_global/install.out b/tests/specs/install/future_install_global/install.out index 09134524c161d1..633718af08bce3 100644 --- a/tests/specs/install/future_install_global/install.out +++ b/tests/specs/install/future_install_global/install.out @@ -1,5 +1,5 @@ Download http://localhost:4545/npm/registry/@denotest/esm-basic Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz Initialize @denotest/esm-basic@1.0.0 -✅ Successfully installed deno-cli-test[WILDCARD] +✅ Successfully installed deno-test-bin[WILDCARD] [WILDCARD] diff --git a/tests/specs/install/future_install_local_add_deno/__test__.jsonc b/tests/specs/install/future_install_local_add_deno/__test__.jsonc index 49313f90d2dcd9..7139bcc020711a 100644 --- a/tests/specs/install/future_install_local_add_deno/__test__.jsonc +++ b/tests/specs/install/future_install_local_add_deno/__test__.jsonc @@ -10,7 +10,7 @@ "output": "install.out" }, { - "args": "run --cached-only main.js", + "args": "run --allow-read=./deno.json --cached-only main.js", "exitCode": 0, "output": "main.out" } diff --git a/tests/specs/install/future_install_local_add_deno/main.js b/tests/specs/install/future_install_local_add_deno/main.js index c29b48b4ed5967..195d338ec7b3ce 100644 --- a/tests/specs/install/future_install_local_add_deno/main.js +++ b/tests/specs/install/future_install_local_add_deno/main.js @@ -1,2 +1,7 @@ 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!"); +} diff --git a/tests/specs/install/future_install_local_add_npm/__test__.jsonc b/tests/specs/install/future_install_local_add_npm/__test__.jsonc index bbb86d69cdd11b..04086476c3058d 100644 --- a/tests/specs/install/future_install_local_add_npm/__test__.jsonc +++ b/tests/specs/install/future_install_local_add_npm/__test__.jsonc @@ -11,7 +11,7 @@ }, { "cleanDenoDir": true, - "args": "run --cached-only main.js", + "args": "run --allow-read=./package.json --cached-only main.js", "exitCode": 0, "output": "main.out" } diff --git a/tests/specs/install/future_install_local_add_npm/main.js b/tests/specs/install/future_install_local_add_npm/main.js index c29b48b4ed5967..fba24025c0545f 100644 --- a/tests/specs/install/future_install_local_add_npm/main.js +++ b/tests/specs/install/future_install_local_add_npm/main.js @@ -1,2 +1,10 @@ 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!"); +} diff --git a/tests/specs/install/future_install_local_deno/main.js b/tests/specs/install/future_install_local_deno/main.js index c29b48b4ed5967..5c09f1f20afc81 100644 --- a/tests/specs/install/future_install_local_deno/main.js +++ b/tests/specs/install/future_install_local_deno/main.js @@ -1,2 +1,6 @@ import { setValue } from "@denotest/esm-basic"; +import { add } from "@denotest/add"; +import { returnsHi } from "test-http"; setValue(5); +returnsHi(); +add(2, 2); diff --git a/tests/specs/install/future_install_node_modules/package.json b/tests/specs/install/future_install_node_modules/package.json index 8e46ec6ebe329c..54ca824d645c5f 100644 --- a/tests/specs/install/future_install_node_modules/package.json +++ b/tests/specs/install/future_install_node_modules/package.json @@ -1,6 +1,5 @@ { "dependencies": { "@denotest/esm-basic": "*" - }, - "type": "module" + } } diff --git a/tests/specs/install/no_future_install_global/__test__.jsonc b/tests/specs/install/no_future_install_global/__test__.jsonc index e4b8b6c13d9ddf..6c7e28af0e79e3 100644 --- a/tests/specs/install/no_future_install_global/__test__.jsonc +++ b/tests/specs/install/no_future_install_global/__test__.jsonc @@ -2,8 +2,12 @@ "tempDir": true, "steps": [ { - "args": "install --root ./bins ./main.js", + "args": "install --root ./bins --name deno-test-bin ./main.js", "output": "install.out" + }, + { + "args": "run -A ./assert.js", + "output": "assert.out" } ] } diff --git a/tests/specs/install/no_future_install_global/assert.js b/tests/specs/install/no_future_install_global/assert.js new file mode 100644 index 00000000000000..c7a6a06807e5d6 --- /dev/null +++ b/tests/specs/install/no_future_install_global/assert.js @@ -0,0 +1,11 @@ +const dirs = Deno.readDir("./bins/bin"); + +let found = false; +for await (const entry of dirs) { + if (entry.name.includes("deno-test-bin")) { + found = true; + } +} +if (!found) { + throw new Error("Failed to find test bin"); +} diff --git a/tests/specs/install/no_future_install_global/assert.out b/tests/specs/install/no_future_install_global/assert.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/specs/install/no_future_install_global/install.out b/tests/specs/install/no_future_install_global/install.out index 3187283333c89c..bccc365af7d8cc 100644 --- a/tests/specs/install/no_future_install_global/install.out +++ b/tests/specs/install/no_future_install_global/install.out @@ -1,5 +1,5 @@ ⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. Download http://localhost:4545/npm/registry/@denotest/esm-basic Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz -✅ Successfully installed deno-cli-test[WILDCARD] +✅ Successfully installed deno-test-bin[WILDCARD] [WILDCARD] diff --git a/tests/specs/install/no_future_install_global/package.json b/tests/specs/install/no_future_install_global/package.json index f3b6cb7be1a466..9f465ad503decc 100644 --- a/tests/specs/install/no_future_install_global/package.json +++ b/tests/specs/install/no_future_install_global/package.json @@ -2,6 +2,5 @@ "name": "deno-test-bin", "dependencies": { "@denotest/esm-basic": "*" - }, - "type": "module" + } } From 15eb0728e0d135ae84b96cbf17c19032f0835ff7 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Wed, 1 May 2024 11:24:59 -0700 Subject: [PATCH 25/32] Rm last --npm --- cli/args/flags.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 90cd048aa47bd6..07de184df6cc47 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1343,12 +1343,6 @@ fn clap_root() -> Command { } fn add_args(cmd: Command, include_packages: bool) -> Command { - let cmd = cmd.arg( - Arg::new("npm") - .long("npm") - .help("Look up packages in npm if unspecified") - .action(ArgAction::SetTrue), - ); if include_packages { cmd.arg( Arg::new("packages") From f19518acf2e21a4ded7a1f48763e90b43632637d Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Tue, 7 May 2024 08:01:46 -0700 Subject: [PATCH 26/32] Add "i" alias for install --- cli/args/flags.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 07de184df6cc47..0262c3bb12a824 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -2114,6 +2114,7 @@ fn install_args(cmd: Command, deno_future: bool) -> Command { fn future_install_subcommand() -> Command { Command::new("install") + .visible_alias("i") .about("Install dependencies") .long_about( "Installs dependencies either in the local project or globally to a bin directory. From b8e7dec64d55669b4b5cc316f8eb7fb7bbcc2868 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Tue, 7 May 2024 08:40:53 -0700 Subject: [PATCH 27/32] Fix tests --- tests/specs/install/future_install_global/install.out | 4 ++-- .../install/future_install_local_add_deno/__test__.jsonc | 1 - tests/specs/install/future_install_local_add_deno/install.out | 4 ++-- .../specs/install/future_install_local_add_npm/__test__.jsonc | 2 -- tests/specs/install/future_install_local_add_npm/install.out | 4 ++-- tests/specs/install/future_install_local_deno/__test__.jsonc | 1 - tests/specs/install/future_install_local_deno/install.out | 4 ++-- .../specs/install/future_install_node_modules/__test__.jsonc | 2 -- tests/specs/install/future_install_node_modules/install.out | 4 ++-- tests/specs/install/no_future_install_global/install.out | 4 ++-- 10 files changed, 12 insertions(+), 18 deletions(-) diff --git a/tests/specs/install/future_install_global/install.out b/tests/specs/install/future_install_global/install.out index 633718af08bce3..adb8b459898827 100644 --- a/tests/specs/install/future_install_global/install.out +++ b/tests/specs/install/future_install_global/install.out @@ -1,5 +1,5 @@ -Download http://localhost:4545/npm/registry/@denotest/esm-basic -Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Download http://localhost:4260/@denotest/esm-basic +Download http://localhost:4260/@denotest/esm-basic/1.0.0.tgz Initialize @denotest/esm-basic@1.0.0 ✅ Successfully installed deno-test-bin[WILDCARD] [WILDCARD] diff --git a/tests/specs/install/future_install_local_add_deno/__test__.jsonc b/tests/specs/install/future_install_local_add_deno/__test__.jsonc index 7139bcc020711a..039afefa1bf24c 100644 --- a/tests/specs/install/future_install_local_add_deno/__test__.jsonc +++ b/tests/specs/install/future_install_local_add_deno/__test__.jsonc @@ -5,7 +5,6 @@ }, "steps": [ { - "cleanDenoDir": true, "args": "install npm:@denotest/esm-basic", "output": "install.out" }, diff --git a/tests/specs/install/future_install_local_add_deno/install.out b/tests/specs/install/future_install_local_add_deno/install.out index 1ed999c762596a..df58284fc81ce5 100644 --- a/tests/specs/install/future_install_local_add_deno/install.out +++ b/tests/specs/install/future_install_local_add_deno/install.out @@ -1,4 +1,4 @@ ⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. Add @denotest/esm-basic - npm:@denotest/esm-basic@^1.0.0 -Download http://localhost:4545/npm/registry/@denotest/esm-basic -Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Download http://localhost:4260/@denotest/esm-basic +Download http://localhost:4260/@denotest/esm-basic/1.0.0.tgz diff --git a/tests/specs/install/future_install_local_add_npm/__test__.jsonc b/tests/specs/install/future_install_local_add_npm/__test__.jsonc index 04086476c3058d..6913404a77bcfc 100644 --- a/tests/specs/install/future_install_local_add_npm/__test__.jsonc +++ b/tests/specs/install/future_install_local_add_npm/__test__.jsonc @@ -5,12 +5,10 @@ }, "steps": [ { - "cleanDenoDir": true, "args": "install npm:@denotest/esm-basic", "output": "install.out" }, { - "cleanDenoDir": true, "args": "run --allow-read=./package.json --cached-only main.js", "exitCode": 0, "output": "main.out" diff --git a/tests/specs/install/future_install_local_add_npm/install.out b/tests/specs/install/future_install_local_add_npm/install.out index e0cf4b51a4afae..d9c23abf008408 100644 --- a/tests/specs/install/future_install_local_add_npm/install.out +++ b/tests/specs/install/future_install_local_add_npm/install.out @@ -1,5 +1,5 @@ ⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. Add @denotest/esm-basic - npm:@denotest/esm-basic@^1.0.0 -Download http://localhost:4545/npm/registry/@denotest/esm-basic -Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Download http://localhost:4260/@denotest/esm-basic +Download http://localhost:4260/@denotest/esm-basic/1.0.0.tgz Initialize @denotest/esm-basic@1.0.0 diff --git a/tests/specs/install/future_install_local_deno/__test__.jsonc b/tests/specs/install/future_install_local_deno/__test__.jsonc index 8237af02e382df..4af2939ee7bc50 100644 --- a/tests/specs/install/future_install_local_deno/__test__.jsonc +++ b/tests/specs/install/future_install_local_deno/__test__.jsonc @@ -5,7 +5,6 @@ }, "steps": [ { - "cleanDenoDir": true, "args": "install", "output": "install.out" }, diff --git a/tests/specs/install/future_install_local_deno/install.out b/tests/specs/install/future_install_local_deno/install.out index 54b22fede57bc7..efb77d8f2257f1 100644 --- a/tests/specs/install/future_install_local_deno/install.out +++ b/tests/specs/install/future_install_local_deno/install.out @@ -6,5 +6,5 @@ Download http://localhost:4545/subdir/print_hello.ts Download http://127.0.0.1:4250/@denotest/add/meta.json Download http://127.0.0.1:4250/@denotest/add/1.0.0_meta.json Download http://127.0.0.1:4250/@denotest/add/1.0.0/mod.ts -Download http://localhost:4545/npm/registry/@denotest/esm-basic -Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Download http://localhost:4260/@denotest/esm-basic +Download http://localhost:4260/@denotest/esm-basic/1.0.0.tgz diff --git a/tests/specs/install/future_install_node_modules/__test__.jsonc b/tests/specs/install/future_install_node_modules/__test__.jsonc index 324e50749d600e..4af2939ee7bc50 100644 --- a/tests/specs/install/future_install_node_modules/__test__.jsonc +++ b/tests/specs/install/future_install_node_modules/__test__.jsonc @@ -5,12 +5,10 @@ }, "steps": [ { - "cleanDenoDir": true, "args": "install", "output": "install.out" }, { - "cleanDenoDir": true, "args": "run --cached-only main.js", "exitCode": 0, "output": "main.out" diff --git a/tests/specs/install/future_install_node_modules/install.out b/tests/specs/install/future_install_node_modules/install.out index 1cb06afcc5d818..22574688aa8ca5 100644 --- a/tests/specs/install/future_install_node_modules/install.out +++ b/tests/specs/install/future_install_node_modules/install.out @@ -1,4 +1,4 @@ ⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. -Download http://localhost:4545/npm/registry/@denotest/esm-basic -Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Download http://localhost:4260/@denotest/esm-basic +Download http://localhost:4260/@denotest/esm-basic/1.0.0.tgz Initialize @denotest/esm-basic@1.0.0 diff --git a/tests/specs/install/no_future_install_global/install.out b/tests/specs/install/no_future_install_global/install.out index bccc365af7d8cc..f3a394c6ff6669 100644 --- a/tests/specs/install/no_future_install_global/install.out +++ b/tests/specs/install/no_future_install_global/install.out @@ -1,5 +1,5 @@ ⚠️ `deno install` behavior will change in Deno 2. To preserve the current behavior use the `-g` or `--global` flag. -Download http://localhost:4545/npm/registry/@denotest/esm-basic -Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Download http://localhost:4260/@denotest/esm-basic +Download http://localhost:4260/@denotest/esm-basic/1.0.0.tgz ✅ Successfully installed deno-test-bin[WILDCARD] [WILDCARD] From 9e1afedddc4c26bd554bcd67b0ecd795323e952a Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Tue, 7 May 2024 12:30:33 -0700 Subject: [PATCH 28/32] Cleanup --- cli/args/flags.rs | 30 ++++++++++++------------------ cli/args/mod.rs | 2 +- cli/tools/registry/pm.rs | 18 +++--------------- 3 files changed, 16 insertions(+), 34 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 0262c3bb12a824..1585ae5d0be7d7 100644 --- a/cli/args/flags.rs +++ b/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 { @@ -2161,7 +2155,7 @@ 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) + cmd }) } diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 6db73c0f1dade1..3b5d79ef3cb797 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -721,7 +721,7 @@ pub struct CliOptions { maybe_node_modules_folder: Option, maybe_vendor_folder: Option, maybe_config_file: Option, - pub maybe_package_json: Option, + maybe_package_json: Option, maybe_lockfile: Option>>, overrides: CliOptionOverrides, maybe_workspace_config: Option, diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index 3007b9406c080f..5a8c49bef419b5 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -134,24 +134,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 = From 6b4486b769670942129e58e73eb754d439e99e2b Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Tue, 7 May 2024 12:51:46 -0700 Subject: [PATCH 29/32] Cleanup add logic --- cli/tools/registry/pm.rs | 74 +++++++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 23 deletions(-) diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index 5a8c49bef419b5..e37ee3d826227f 100644 --- a/cli/tools/registry/pm.rs +++ b/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 { + 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), } -impl ConfigFile { - fn specifier(&self) -> ModuleSpecifier { +impl DenoOrPackageJson { + fn specifier(&self) -> Cow { 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, 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,7 +77,7 @@ impl ConfigFile { Ok(Default::default()) } } - ConfigFile::Npm(npm, ..) => { + DenoOrPackageJson::Npm(npm, ..) => { Ok(npm.dependencies.clone().unwrap_or_default()) } } @@ -64,27 +85,30 @@ impl ConfigFile { 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,28 +121,31 @@ 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() @@ -126,6 +153,7 @@ impl ConfigFile { anyhow!("config not found, but it was just created") })? .clone(), + DenoConfigFormat::Json, ), new_factory, )) @@ -152,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" { From fc66f9c3f19d45615c8a25515560e62d4c6e05ff Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Tue, 7 May 2024 13:04:56 -0700 Subject: [PATCH 30/32] Cleanup tests --- .../install/future_install_global/__test__.jsonc | 2 +- .../specs/install/future_install_global/assert.out | 0 tests/specs/install/future_install_global/main.out | 0 .../future_install_local_add_deno/__test__.jsonc | 13 ++++++++++--- .../future_install_local_add_deno/deno.json.out | 5 +++++ .../install/future_install_local_add_deno/main.js | 5 ----- .../install/future_install_local_add_deno/main.out | 0 .../future_install_local_add_npm/__test__.jsonc | 12 ++++++++++-- .../install/future_install_local_add_npm/main.js | 8 -------- .../install/future_install_local_add_npm/main.out | 0 .../future_install_local_add_npm/package.json.out | 3 +++ .../future_install_local_deno/__test__.jsonc | 4 ++-- .../install/future_install_local_deno/main.out | 0 .../future_install_node_modules/__test__.jsonc | 4 ++-- .../install/future_install_node_modules/main.out | 0 .../install/no_future_install_global/__test__.jsonc | 2 +- .../install/no_future_install_global/assert.out | 0 17 files changed, 34 insertions(+), 24 deletions(-) delete mode 100644 tests/specs/install/future_install_global/assert.out delete mode 100644 tests/specs/install/future_install_global/main.out create mode 100644 tests/specs/install/future_install_local_add_deno/deno.json.out delete mode 100644 tests/specs/install/future_install_local_add_deno/main.out delete mode 100644 tests/specs/install/future_install_local_add_npm/main.out create mode 100644 tests/specs/install/future_install_local_add_npm/package.json.out delete mode 100644 tests/specs/install/future_install_local_deno/main.out delete mode 100644 tests/specs/install/future_install_node_modules/main.out delete mode 100644 tests/specs/install/no_future_install_global/assert.out diff --git a/tests/specs/install/future_install_global/__test__.jsonc b/tests/specs/install/future_install_global/__test__.jsonc index fcae89f416ea23..be6fcab9720144 100644 --- a/tests/specs/install/future_install_global/__test__.jsonc +++ b/tests/specs/install/future_install_global/__test__.jsonc @@ -10,7 +10,7 @@ }, { "args": "run -A assert.js", - "output": "assert.out" + "output": "" } ] } diff --git a/tests/specs/install/future_install_global/assert.out b/tests/specs/install/future_install_global/assert.out deleted file mode 100644 index e69de29bb2d1d6..00000000000000 diff --git a/tests/specs/install/future_install_global/main.out b/tests/specs/install/future_install_global/main.out deleted file mode 100644 index e69de29bb2d1d6..00000000000000 diff --git a/tests/specs/install/future_install_local_add_deno/__test__.jsonc b/tests/specs/install/future_install_local_add_deno/__test__.jsonc index 039afefa1bf24c..eaafafbdd323b7 100644 --- a/tests/specs/install/future_install_local_add_deno/__test__.jsonc +++ b/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" } ] } diff --git a/tests/specs/install/future_install_local_add_deno/deno.json.out b/tests/specs/install/future_install_local_add_deno/deno.json.out new file mode 100644 index 00000000000000..0172071c39e3a0 --- /dev/null +++ b/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" + } +} diff --git a/tests/specs/install/future_install_local_add_deno/main.js b/tests/specs/install/future_install_local_add_deno/main.js index 195d338ec7b3ce..c29b48b4ed5967 100644 --- a/tests/specs/install/future_install_local_add_deno/main.js +++ b/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!"); -} diff --git a/tests/specs/install/future_install_local_add_deno/main.out b/tests/specs/install/future_install_local_add_deno/main.out deleted file mode 100644 index e69de29bb2d1d6..00000000000000 diff --git a/tests/specs/install/future_install_local_add_npm/__test__.jsonc b/tests/specs/install/future_install_local_add_npm/__test__.jsonc index 6913404a77bcfc..19ded2ab56fa81 100644 --- a/tests/specs/install/future_install_local_add_npm/__test__.jsonc +++ b/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" } ] } diff --git a/tests/specs/install/future_install_local_add_npm/main.js b/tests/specs/install/future_install_local_add_npm/main.js index fba24025c0545f..c29b48b4ed5967 100644 --- a/tests/specs/install/future_install_local_add_npm/main.js +++ b/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!"); -} diff --git a/tests/specs/install/future_install_local_add_npm/main.out b/tests/specs/install/future_install_local_add_npm/main.out deleted file mode 100644 index e69de29bb2d1d6..00000000000000 diff --git a/tests/specs/install/future_install_local_add_npm/package.json.out b/tests/specs/install/future_install_local_add_npm/package.json.out new file mode 100644 index 00000000000000..ad8518e791a975 --- /dev/null +++ b/tests/specs/install/future_install_local_add_npm/package.json.out @@ -0,0 +1,3 @@ +{ + "dependencies": { "@denotest/esm-basic": "^1.0.0" } +} diff --git a/tests/specs/install/future_install_local_deno/__test__.jsonc b/tests/specs/install/future_install_local_deno/__test__.jsonc index 4af2939ee7bc50..eb780d96246ee7 100644 --- a/tests/specs/install/future_install_local_deno/__test__.jsonc +++ b/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": "" } ] } diff --git a/tests/specs/install/future_install_local_deno/main.out b/tests/specs/install/future_install_local_deno/main.out deleted file mode 100644 index e69de29bb2d1d6..00000000000000 diff --git a/tests/specs/install/future_install_node_modules/__test__.jsonc b/tests/specs/install/future_install_node_modules/__test__.jsonc index 4af2939ee7bc50..eb780d96246ee7 100644 --- a/tests/specs/install/future_install_node_modules/__test__.jsonc +++ b/tests/specs/install/future_install_node_modules/__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": "" } ] } diff --git a/tests/specs/install/future_install_node_modules/main.out b/tests/specs/install/future_install_node_modules/main.out deleted file mode 100644 index e69de29bb2d1d6..00000000000000 diff --git a/tests/specs/install/no_future_install_global/__test__.jsonc b/tests/specs/install/no_future_install_global/__test__.jsonc index 6c7e28af0e79e3..ca351ee0dfe6a8 100644 --- a/tests/specs/install/no_future_install_global/__test__.jsonc +++ b/tests/specs/install/no_future_install_global/__test__.jsonc @@ -7,7 +7,7 @@ }, { "args": "run -A ./assert.js", - "output": "assert.out" + "output": "" } ] } diff --git a/tests/specs/install/no_future_install_global/assert.out b/tests/specs/install/no_future_install_global/assert.out deleted file mode 100644 index e69de29bb2d1d6..00000000000000 From d3901df998c3d76ec1c7a059f6f388cde05ecc5e Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Tue, 7 May 2024 13:06:34 -0700 Subject: [PATCH 31/32] Appease clippy --- cli/args/flags.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 1585ae5d0be7d7..5fa4704b44f481 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -2154,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); - cmd + install_args(cmd, true) }) } From 8959a586ffdf3a9f84b46c8f5d9e743dac196453 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker Date: Tue, 7 May 2024 13:17:19 -0700 Subject: [PATCH 32/32] Update todo --- cli/args/flags.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 5fa4704b44f481..0ef02be3927d59 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -3979,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,