Skip to content

Commit

Permalink
add raw_specifier for showing helpful error message easier when it …
Browse files Browse the repository at this point in the history
…failed to resolve CJS module
  • Loading branch information
Hajime-san committed Nov 10, 2024
1 parent 73fbd61 commit 81962e8
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 3 deletions.
12 changes: 9 additions & 3 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,9 +815,15 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
}

let referrer = self.0.resolve_referrer(referrer)?;
let specifier = self.0.inner_resolve(specifier, &referrer)?;
ensure_not_jsr_non_jsr_remote_import(&specifier, &referrer)?;
Ok(specifier)
let resolved_specifier = self.0.inner_resolve(specifier, &referrer)?;
// set the raw specifier so that we can use it later
self
.0
.shared
.cjs_tracker
.set_raw_speficier(&resolved_specifier, specifier);
ensure_not_jsr_non_jsr_remote_import(&resolved_specifier, &referrer)?;
Ok(resolved_specifier)
}

fn get_host_defined_options<'s>(
Expand Down
64 changes: 64 additions & 0 deletions cli/resolver.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use async_trait::async_trait;
use dashmap::mapref::one::Ref;
use dashmap::DashMap;
use dashmap::DashSet;
use deno_ast::MediaType;
Expand Down Expand Up @@ -382,6 +383,49 @@ impl NpmModuleLoader {
if let Some(referrer) = &maybe_referrer {
msg.push_str(" imported from ");
msg.push_str(referrer.as_str());

// TODO(Hajime-san): Use `with_added_extension` when it becomes stable.
//
// This extended implementation for `Path` defines an ad-hoc method with the same name,
// since `with_added_extension` is currently only available in the nightly version.
// This implementation should be replaced when it becomes stable.
// https://github.com/rust-lang/rust/issues/127292
trait PathExt {
fn _with_added_extension(&self, extension: &str) -> PathBuf;
}

impl PathExt for Path {
fn _with_added_extension(&self, extension: &str) -> PathBuf {
let mut path = self.to_path_buf();

let new_extension = match self.extension() {
Some(ext) => {
format!("{}.{}", ext.to_string_lossy(), extension)
}
None => extension.to_string(),
};

path.set_extension(new_extension);
path
}
}

// A package may have CommonJS modules that are not all listed in the package.json exports.
// In this case, it cannot be statically resolved when imported from ESM unless you include the extension of the target file.
// So provide the user with a helpful error message.
let extension = ["js", "cjs"]
.iter()
.find(|e| file_path._with_added_extension(e).is_file());
let raw_specifier = self.cjs_tracker.get_raw_speficier(specifier);
if let (Some(raw_specifier), Some(extension)) =
(raw_specifier, extension)
{
msg.push_str("\nDid you mean to import \"");
let suggested_specifier = Path::new(raw_specifier.as_str())
._with_added_extension(extension);
msg.push_str(&suggested_specifier.to_string_lossy());
msg.push_str("\"?");
}
}
msg
}
Expand Down Expand Up @@ -434,6 +478,8 @@ pub struct CjsTracker {
pkg_json_resolver: Arc<PackageJsonResolver>,
unstable_detect_cjs: bool,
known: DashMap<ModuleSpecifier, ModuleKind>,
/// This field is used to store the raw specifier string due to suggest a helpful error message.
raw_specifier: DashMap<ModuleSpecifier, String>,
}

impl CjsTracker {
Expand All @@ -447,6 +493,7 @@ impl CjsTracker {
pkg_json_resolver,
unstable_detect_cjs: options.unstable_detect_cjs,
known: Default::default(),
raw_specifier: Default::default(),
}
}

Expand Down Expand Up @@ -498,6 +545,23 @@ impl CjsTracker {
self.get_known_kind_with_is_script(specifier, media_type, None)
}

pub fn get_raw_speficier(
&self,
specifier: &ModuleSpecifier,
) -> Option<Ref<'_, Url, String>> {
self.raw_specifier.get(specifier)
}

pub fn set_raw_speficier(
&self,
specifier: &ModuleSpecifier,
raw_specifier: &str,
) {
self
.raw_specifier
.insert(specifier.clone(), raw_specifier.to_string());
}

fn get_known_kind_with_is_script(
&self,
specifier: &ModuleSpecifier,
Expand Down

0 comments on commit 81962e8

Please sign in to comment.