Skip to content

Commit

Permalink
fix(info): error instead of panic for npm specifiers when using byonm (
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret authored Sep 30, 2024
1 parent c5c1869 commit d7b7877
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 4 deletions.
4 changes: 0 additions & 4 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1144,10 +1144,6 @@ impl CliOptions {
DenoSubcommand::Run(run_flags) => {
if run_flags.is_stdin() {
resolve_url_or_path("./$deno$stdin.ts", self.initial_cwd())?
} else if NpmPackageReqReference::from_str(&run_flags.script)
.is_ok()
{
ModuleSpecifier::parse(&run_flags.script)?
} else {
resolve_url_or_path(&run_flags.script, self.initial_cwd())?
}
Expand Down
7 changes: 7 additions & 0 deletions cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::tools::check::TypeChecker;
use crate::util::file_watcher::WatcherCommunicator;
use crate::util::fs::canonicalize_path;
use deno_config::workspace::JsrPackageConfig;
use deno_core::anyhow::bail;
use deno_graph::source::LoaderChecksum;
use deno_graph::FillFromLockfileOptions;
use deno_graph::JsrLoadError;
Expand Down Expand Up @@ -593,6 +594,12 @@ impl ModuleGraphBuilder {
let initial_package_deps_len = graph.packages.package_deps_sum();
let initial_package_mappings_len = graph.packages.mappings().len();

if roots.iter().any(|r| r.scheme() == "npm")
&& self.npm_resolver.as_byonm().is_some()
{
bail!("Resolving npm specifier entrypoints this way is currently not supported with \"nodeModules\": \"manual\". In the meantime, try with --node-modules-dir=auto instead");
}

graph.build(roots, loader, options).await;

let has_redirects_changed = graph.redirects.len() != initial_redirects_len;
Expand Down
11 changes: 11 additions & 0 deletions tests/specs/info/byonm/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"tempDir": true,
"steps": [{
"args": "install",
"output": "[WILDCARD]"
}, {
"args": "info npm:@denotest/add",
"output": "info.out",
"exitCode": 1
}]
}
6 changes: 6 additions & 0 deletions tests/specs/info/byonm/deno.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"nodeModulesDir": "manual",
"imports": {
"chalk": "npm:@denotest/add"
}
}
1 change: 1 addition & 0 deletions tests/specs/info/byonm/info.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error: Resolving npm specifier entrypoints this way is currently not supported with "nodeModules": "manual". In the meantime, try with --node-modules-dir=auto instead
5 changes: 5 additions & 0 deletions tests/specs/info/byonm/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"@denotest/add": "*"
}
}

0 comments on commit d7b7877

Please sign in to comment.