Skip to content

Commit

Permalink
fix(cli): add prefix hint when installing a package globally
Browse files Browse the repository at this point in the history
  • Loading branch information
mbrdg committed Nov 16, 2024
1 parent 1cab4f0 commit 84ceac0
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 2 deletions.
45 changes: 44 additions & 1 deletion cli/tools/installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use crate::args::resolve_no_prompt;
use crate::args::AddFlags;
use crate::args::CaData;
use crate::args::CacheSetting;
use crate::args::ConfigFlag;
use crate::args::Flags;
use crate::args::InstallFlags;
Expand All @@ -13,8 +14,11 @@ use crate::args::TypeCheckMode;
use crate::args::UninstallFlags;
use crate::args::UninstallKind;
use crate::factory::CliFactory;
use crate::file_fetcher::FileFetcher;
use crate::graph_container::ModuleGraphContainer;
use crate::http_util::HttpClientProvider;
use crate::jsr::JsrFetchResolver;
use crate::npm::NpmFetchResolver;
use crate::util::fs::canonicalize_path_maybe_not_exists;

use deno_core::anyhow::bail;
Expand Down Expand Up @@ -354,12 +358,51 @@ async fn install_global(
) -> Result<(), AnyError> {
// ensure the module is cached
let factory = CliFactory::from_flags(flags.clone());

let http_client = factory.http_client_provider();
let deps_http_cache = factory.global_http_cache()?;
let mut deps_file_fetcher = FileFetcher::new(
deps_http_cache.clone(),
CacheSetting::ReloadAll,
true,
http_client.clone(),
Default::default(),
None,
);

let npmrc = factory.cli_options().unwrap().npmrc();

deps_file_fetcher.set_download_log_level(log::Level::Trace);
let deps_file_fetcher = Arc::new(deps_file_fetcher);
let jsr_resolver = Arc::new(JsrFetchResolver::new(deps_file_fetcher.clone()));
let npm_resolver = Arc::new(NpmFetchResolver::new(
deps_file_fetcher.clone(),
npmrc.clone(),
));

let entry_text = install_flags_global.module_url.as_str();
let req = super::registry::AddRmPackageReq::parse(entry_text);

// found a package requirement but missing the prefix
if let Ok(Err(package_req)) = req {
if jsr_resolver.req_to_nv(&package_req).await.is_some() {
bail!(
"{entry_text} is missing a prefix. Did you mean `{}`?",
crate::colors::yellow(format!("deno install -g jsr:{package_req}"))
);
} else if npm_resolver.req_to_nv(&package_req).await.is_some() {
bail!(
"{entry_text} is missing a prefix. Did you mean `{}`?",
crate::colors::yellow(format!("deno install -g npm:{package_req}"))
);
}
}

factory
.main_module_graph_container()
.await?
.load_and_type_check_files(&[install_flags_global.module_url.clone()])
.await?;
let http_client = factory.http_client_provider();

// create the install shim
create_install_shim(http_client, &flags, install_flags_global).await
Expand Down
1 change: 1 addition & 0 deletions cli/tools/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ pub use pm::add;
pub use pm::cache_top_level_deps;
pub use pm::remove;
pub use pm::AddCommandName;
pub use pm::AddRmPackageReq;
use publish_order::PublishOrderGraph;
use unfurl::SpecifierUnfurler;

Expand Down
2 changes: 1 addition & 1 deletion cli/tools/registry/pm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ enum AddRmPackageReqValue {
}

#[derive(Debug, PartialEq, Eq)]
struct AddRmPackageReq {
pub struct AddRmPackageReq {
alias: String,
value: AddRmPackageReqValue,
}
Expand Down
57 changes: 57 additions & 0 deletions tests/integration/install_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,3 +329,60 @@ fn check_local_by_default2() {
.skip_output_check()
.assert_exit_code(0);
}

#[test]
fn show_prefix_hint_on_global_install() {
let context = TestContextBuilder::new()
.add_npm_env_vars()
.add_jsr_env_vars()
.use_http_server()
.use_temp_cwd()
.build();
let temp_dir = context.temp_dir();
let temp_dir_str = temp_dir.path().to_string();

let env_vars = [
("HOME", temp_dir_str.as_str()),
("USERPROFILE", temp_dir_str.as_str()),
("DENO_INSTALL_ROOT", ""),
];

for pkg_req in ["npm:@denotest/bin", "jsr:@denotest/add"] {
let name = pkg_req.split_once('/').unwrap().1;
let pkg = pkg_req.split_once(':').unwrap().1;

// try with prefix and ensure that the installation succeeds
context
.new_command()
.args_vec(["install", "-g", "--name", name, pkg_req])
.envs(env_vars)
.run()
.skip_output_check()
.assert_exit_code(0);

// try without the prefix and ensure that the installation fails with the appropriate error
// message
let output = context
.new_command()
.args_vec(["install", "-g", "--name", name, pkg])
.envs(env_vars)
.run();
output.assert_exit_code(1);

let output_text = output.combined_output();
let expected_text =
format!("error: {pkg} is missing a prefix. Did you mean `deno install -g {pkg_req}`?");
assert_contains!(output_text, &expected_text);
}

// try a pckage not in npm and jsr to make sure the appropriate error message still appears
let output = context
.new_command()
.args_vec(["install", "-g", "package-that-does-not-exist"])
.envs(env_vars)
.run();
output.assert_exit_code(1);

let output_text = output.combined_output();
assert_contains!(output_text, "error: Module not found");
}

0 comments on commit 84ceac0

Please sign in to comment.