fix(node): improve require esm error messages (#19853)

Part of #19842.

Closes #19583
Closes #16913
This commit is contained in:
David Sherret 2023-07-17 14:00:44 -04:00 committed by GitHub
parent 37241e9b1e
commit 7a9f7f3419
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
21 changed files with 220 additions and 159 deletions

View file

@ -198,7 +198,7 @@ impl<'a> TsResponseImportMapper<'a> {
}
if self.npm_resolver.in_npm_package(specifier) {
if let Ok(pkg_id) = self
if let Ok(Some(pkg_id)) = self
.npm_resolver
.resolve_package_id_from_specifier(specifier)
{
@ -254,7 +254,8 @@ impl<'a> TsResponseImportMapper<'a> {
let root_folder = self
.npm_resolver
.resolve_package_folder_from_specifier(specifier)
.ok()?;
.ok()
.flatten()?;
let package_json_path = root_folder.join("package.json");
let package_json_text = std::fs::read_to_string(&package_json_path).ok()?;
let package_json =

View file

@ -196,19 +196,6 @@ impl NpmCacheDir {
&self,
specifier: &ModuleSpecifier,
registry_url: &Url,
) -> Result<NpmPackageCacheFolderId, AnyError> {
match self
.maybe_resolve_package_folder_id_from_specifier(specifier, registry_url)
{
Some(id) => Ok(id),
None => bail!("could not find npm package for '{}'", specifier),
}
}
fn maybe_resolve_package_folder_id_from_specifier(
&self,
specifier: &ModuleSpecifier,
registry_url: &Url,
) -> Option<NpmPackageCacheFolderId> {
let registry_root_dir = self
.root_dir_url
@ -455,7 +442,7 @@ impl NpmCache {
&self,
specifier: &ModuleSpecifier,
registry_url: &Url,
) -> Result<NpmPackageCacheFolderId, AnyError> {
) -> Option<NpmPackageCacheFolderId> {
self
.cache_dir
.resolve_package_folder_id_from_specifier(specifier, registry_url)

View file

@ -46,12 +46,12 @@ pub trait NpmPackageFsResolver: Send + Sync {
fn resolve_package_folder_from_specifier(
&self,
specifier: &ModuleSpecifier,
) -> Result<PathBuf, AnyError>;
) -> Result<Option<PathBuf>, AnyError>;
fn resolve_package_cache_folder_id_from_specifier(
&self,
specifier: &ModuleSpecifier,
) -> Result<NpmPackageCacheFolderId, AnyError>;
) -> Result<Option<NpmPackageCacheFolderId>, AnyError>;
async fn cache_packages(&self) -> Result<(), AnyError>;

View file

@ -8,6 +8,7 @@ use std::sync::Arc;
use async_trait::async_trait;
use deno_ast::ModuleSpecifier;
use deno_core::anyhow::bail;
use deno_core::error::AnyError;
use deno_core::url::Url;
use deno_npm::resolution::PackageNotFoundFromReferrerError;
@ -97,9 +98,11 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
referrer: &ModuleSpecifier,
mode: NodeResolutionMode,
) -> Result<PathBuf, AnyError> {
let referrer_pkg_id = self
let Some(referrer_pkg_id) = self
.cache
.resolve_package_folder_id_from_specifier(referrer, &self.registry_url)?;
.resolve_package_folder_id_from_specifier(referrer, &self.registry_url) else {
bail!("could not find npm package for '{}'", referrer);
};
let pkg = if mode.is_types() && !name.starts_with("@types/") {
// attempt to resolve the types package first, then fallback to the regular package
match self.resolve_types_package(name, &referrer_pkg_id) {
@ -119,25 +122,30 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
fn resolve_package_folder_from_specifier(
&self,
specifier: &ModuleSpecifier,
) -> Result<PathBuf, AnyError> {
let pkg_folder_id = self.cache.resolve_package_folder_id_from_specifier(
) -> Result<Option<PathBuf>, AnyError> {
let Some(pkg_folder_id) = self.cache.resolve_package_folder_id_from_specifier(
specifier,
&self.registry_url,
)?;
Ok(
) else {
return Ok(None);
};
Ok(Some(
self
.cache
.package_folder_for_id(&pkg_folder_id, &self.registry_url),
)
))
}
fn resolve_package_cache_folder_id_from_specifier(
&self,
specifier: &ModuleSpecifier,
) -> Result<NpmPackageCacheFolderId, AnyError> {
self
.cache
.resolve_package_folder_id_from_specifier(specifier, &self.registry_url)
) -> Result<Option<NpmPackageCacheFolderId>, AnyError> {
Ok(
self.cache.resolve_package_folder_id_from_specifier(
specifier,
&self.registry_url,
),
)
}
async fn cache_packages(&self) -> Result<(), AnyError> {

View file

@ -109,31 +109,27 @@ impl LocalNpmPackageResolver {
fn resolve_folder_for_specifier(
&self,
specifier: &ModuleSpecifier,
) -> Result<PathBuf, AnyError> {
match self.maybe_resolve_folder_for_specifier(specifier) {
// Canonicalize the path so it's not pointing to the symlinked directory
// in `node_modules` directory of the referrer.
Some(path) => canonicalize_path_maybe_not_exists_with_fs(&path, |path| {
self
.fs
.realpath_sync(path)
.map_err(|err| err.into_io_error())
})
.map_err(|err| err.into()),
None => bail!("could not find npm package for '{}'", specifier),
}
}
fn maybe_resolve_folder_for_specifier(
&self,
specifier: &ModuleSpecifier,
) -> Option<PathBuf> {
let relative_url = self.root_node_modules_url.make_relative(specifier)?;
) -> Result<Option<PathBuf>, AnyError> {
let Some(relative_url) = self.root_node_modules_url.make_relative(specifier) else {
return Ok(None);
};
if relative_url.starts_with("../") {
return None;
return Ok(None);
}
// it's within the directory, so use it
specifier.to_file_path().ok()
let Some(path) = specifier.to_file_path().ok() else {
return Ok(None);
};
// Canonicalize the path so it's not pointing to the symlinked directory
// in `node_modules` directory of the referrer.
canonicalize_path_maybe_not_exists_with_fs(&path, |path| {
self
.fs
.realpath_sync(path)
.map_err(|err| err.into_io_error())
})
.map(Some)
.map_err(|err| err.into())
}
}
@ -172,7 +168,9 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
referrer: &ModuleSpecifier,
mode: NodeResolutionMode,
) -> Result<PathBuf, AnyError> {
let local_path = self.resolve_folder_for_specifier(referrer)?;
let Some(local_path) = self.resolve_folder_for_specifier(referrer)? else {
bail!("could not find npm package for '{}'", referrer);
};
let package_root_path = self.resolve_package_root(&local_path);
let mut current_folder = package_root_path.as_path();
loop {
@ -220,22 +218,23 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
fn resolve_package_folder_from_specifier(
&self,
specifier: &ModuleSpecifier,
) -> Result<PathBuf, AnyError> {
let local_path = self.resolve_folder_for_specifier(specifier)?;
) -> Result<Option<PathBuf>, AnyError> {
let Some(local_path) = self.resolve_folder_for_specifier(specifier)? else {
return Ok(None);
};
let package_root_path = self.resolve_package_root(&local_path);
Ok(package_root_path)
Ok(Some(package_root_path))
}
fn resolve_package_cache_folder_id_from_specifier(
&self,
specifier: &ModuleSpecifier,
) -> Result<NpmPackageCacheFolderId, AnyError> {
let folder_path = self.resolve_package_folder_from_specifier(specifier)?;
) -> Result<Option<NpmPackageCacheFolderId>, AnyError> {
let Some(folder_path) = self.resolve_package_folder_from_specifier(specifier)? else {
return Ok(None);
};
let folder_name = folder_path.parent().unwrap().to_string_lossy();
match get_package_folder_id_from_folder_name(&folder_name) {
Some(package_folder_id) => Ok(package_folder_id),
None => bail!("could not resolve package from specifier '{}'", specifier),
}
Ok(get_package_folder_id_from_folder_name(&folder_name))
}
async fn cache_packages(&self) -> Result<(), AnyError> {

View file

@ -131,31 +131,35 @@ impl CliNpmResolver {
pub fn resolve_package_folder_from_specifier(
&self,
specifier: &ModuleSpecifier,
) -> Result<PathBuf, AnyError> {
let path = self
) -> Result<Option<PathBuf>, AnyError> {
let Some(path) = self
.fs_resolver
.resolve_package_folder_from_specifier(specifier)?;
.resolve_package_folder_from_specifier(specifier)? else {
return Ok(None);
};
log::debug!(
"Resolved package folder of {} to {}",
specifier,
path.display()
);
Ok(path)
Ok(Some(path))
}
/// Resolves the package nv from the provided specifier.
pub fn resolve_package_id_from_specifier(
&self,
specifier: &ModuleSpecifier,
) -> Result<NpmPackageId, AnyError> {
let cache_folder_id = self
) -> Result<Option<NpmPackageId>, AnyError> {
let Some(cache_folder_id) = self
.fs_resolver
.resolve_package_cache_folder_id_from_specifier(specifier)?;
Ok(
.resolve_package_cache_folder_id_from_specifier(specifier)? else {
return Ok(None);
};
Ok(Some(
self
.resolution
.resolve_pkg_id_from_pkg_cache_folder_id(&cache_folder_id)?,
)
))
}
/// Attempts to get the package size in bytes.
@ -268,7 +272,7 @@ impl NpmResolver for CliNpmResolver {
fn resolve_package_folder_from_path(
&self,
path: &Path,
) -> Result<PathBuf, AnyError> {
) -> Result<Option<PathBuf>, AnyError> {
let specifier = path_to_specifier(path)?;
self.resolve_package_folder_from_specifier(&specifier)
}
@ -291,7 +295,8 @@ impl NpmResolver for CliNpmResolver {
fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool {
self
.resolve_package_folder_from_specifier(specifier)
.is_ok()
.map(|p| p.is_some())
.unwrap_or(false)
}
fn ensure_read_permission(

View file

@ -88,6 +88,22 @@ itest!(cjs_invalid_name_exports {
http_server: true,
});
itest!(cjs_require_esm_error {
args: "run --allow-read --quiet npm/cjs_require_esm_error/main.ts",
output: "npm/cjs_require_esm_error/main.out",
envs: env_vars_for_npm_tests(),
http_server: true,
exit_code: 1,
});
itest!(cjs_require_esm_mjs_error {
args: "run --allow-read --quiet npm/cjs_require_esm_mjs_error/main.ts",
output: "npm/cjs_require_esm_mjs_error/main.out",
envs: env_vars_for_npm_tests(),
http_server: true,
exit_code: 1,
});
itest!(translate_cjs_to_esm {
args: "run -A --quiet npm/translate_cjs_to_esm/main.js",
output: "npm/translate_cjs_to_esm/main.out",

View file

@ -0,0 +1,4 @@
error: Uncaught Error: [ERR_REQUIRE_ESM]: require() of ES Module [WILDCARD]my_esm_module.js from [WILDCARD]index.js not supported. Instead change the require to a dynamic import() which is available in all CommonJS modules.
at Object.Module._extensions..js (node:module:[WILDCARD])
[WILDCARD]
at Module.load (node:module:[WILDCARD])

View file

@ -0,0 +1 @@
import "npm:@denotest/cjs-require-esm-error";

View file

@ -0,0 +1,4 @@
error: Uncaught Error: [ERR_REQUIRE_ESM]: require() of ES Module [WILDCARD]esm_mjs.mjs from [WILDCARD]require_mjs.js not supported. Instead change the require to a dynamic import() which is available in all CommonJS modules.
at Module.load (node:module:[WILDCARD])
[WILDCARD]
at Function.Module._load (node:module:[WILDCARD])

View file

@ -0,0 +1 @@
import "npm:@denotest/cjs-require-esm-error/require_mjs.js";

View file

@ -0,0 +1 @@
export class Test {}

View file

@ -0,0 +1,3 @@
{
"type": "module"
}

View file

@ -0,0 +1 @@
export class Test {}

View file

@ -0,0 +1 @@
module.exports.Test = require("./esm/my_esm_module.js");

View file

@ -0,0 +1,4 @@
{
"name": "@denotest/cjs-require-esm-error",
"version": "1.0.0"
}

View file

@ -0,0 +1 @@
module.exports.Test = require("./esm_mjs.mjs");

View file

@ -81,7 +81,7 @@ pub trait NpmResolver: std::fmt::Debug + MaybeSend + MaybeSync {
fn resolve_package_folder_from_path(
&self,
path: &Path,
) -> Result<PathBuf, AnyError>;
) -> Result<Option<PathBuf>, AnyError>;
/// Resolves an npm package folder path from a Deno module.
fn resolve_package_folder_from_deno_module(

View file

@ -371,7 +371,8 @@ where
&Url::from_file_path(parent_path.unwrap()).unwrap(),
permissions,
)
.ok();
.ok()
.flatten();
if pkg.is_none() {
return Ok(None);
}
@ -499,7 +500,7 @@ where
fn op_require_read_closest_package_json<P>(
state: &mut OpState,
filename: String,
) -> Result<PackageJson, AnyError>
) -> Result<Option<PackageJson>, AnyError>
where
P: NodePermissions + 'static,
{

View file

@ -879,7 +879,9 @@ Module.prototype.load = function (filename) {
StringPrototypeEndsWith(filename, ".mjs") && !Module._extensions[".mjs"]
) {
// TODO: use proper error class
throw new Error("require ESM", filename);
throw new Error(
requireEsmErrorText(filename, moduleParentCache.get(this)?.filename),
);
}
Module._extensions[extension](this, filename);
@ -990,23 +992,29 @@ Module._extensions[".js"] = function (module, filename) {
const content = ops.op_require_read_file(filename);
if (StringPrototypeEndsWith(filename, ".js")) {
const pkg = ops.op_require_read_package_scope(filename);
if (pkg && pkg.exists && pkg.typ == "module") {
let message = `Trying to import ESM module: ${filename}`;
if (module.parent) {
message += ` from ${module.parent.filename}`;
}
message += ` using require()`;
throw new Error(message);
const pkg = ops.op_require_read_closest_package_json(filename);
if (pkg && pkg.exists && pkg.typ === "module") {
throw new Error(
requireEsmErrorText(filename, moduleParentCache.get(module)?.filename),
);
}
}
module._compile(content, filename);
};
function requireEsmErrorText(filename, parent) {
let message = `[ERR_REQUIRE_ESM]: require() of ES Module ${filename}`;
if (parent) {
message += ` from ${parent}`;
}
message +=
` not supported. Instead change the require to a dynamic import() which is available in all CommonJS modules.`;
return message;
}
function stripBOM(content) {
if (StringPrototypeCharCodeAt(content, 0) === 0xfeff) {
content = StringPrototypeSlice(content, 1);

View file

@ -426,12 +426,12 @@ impl NodeResolver {
if url_str.starts_with("http") {
Ok(NodeResolution::Esm(url))
} else if url_str.ends_with(".js") || url_str.ends_with(".d.ts") {
let package_config =
let maybe_package_config =
self.get_closest_package_json(&url, &AllowAllNodePermissions)?;
if package_config.typ == "module" {
Ok(NodeResolution::Esm(url))
} else {
Ok(NodeResolution::CommonJs(url))
match maybe_package_config {
Some(c) if c.typ == "module" => Ok(NodeResolution::Esm(url)),
Some(_) => Ok(NodeResolution::CommonJs(url)),
None => Ok(NodeResolution::Esm(url)),
}
} else if url_str.ends_with(".mjs") || url_str.ends_with(".d.mts") {
Ok(NodeResolution::Esm(url))
@ -555,63 +555,22 @@ impl NodeResolver {
));
}
let package_config =
self.get_package_scope_config(referrer, permissions)?;
let mut package_json_path = None;
if package_config.exists {
package_json_path = Some(package_config.path.clone());
if let Some(imports) = &package_config.imports {
if imports.contains_key(name) && !name.contains('*') {
let maybe_resolved = self.resolve_package_target(
package_json_path.as_ref().unwrap(),
imports.get(name).unwrap().to_owned(),
"".to_string(),
name.to_string(),
referrer,
referrer_kind,
false,
true,
conditions,
mode,
permissions,
)?;
if let Some(resolved) = maybe_resolved {
return Ok(resolved);
}
} else {
let mut best_match = "";
let mut best_match_subpath = None;
for key in imports.keys() {
let pattern_index = key.find('*');
if let Some(pattern_index) = pattern_index {
let key_sub = &key[0..=pattern_index];
if name.starts_with(key_sub) {
let pattern_trailer = &key[pattern_index + 1..];
if name.len() > key.len()
&& name.ends_with(&pattern_trailer)
&& pattern_key_compare(best_match, key) == 1
&& key.rfind('*') == Some(pattern_index)
{
best_match = key;
best_match_subpath = Some(
name[pattern_index..=(name.len() - pattern_trailer.len())]
.to_string(),
);
}
}
}
}
if !best_match.is_empty() {
let target = imports.get(best_match).unwrap().to_owned();
if let Some(package_config) =
self.get_package_scope_config(referrer, permissions)?
{
if package_config.exists {
package_json_path = Some(package_config.path.clone());
if let Some(imports) = &package_config.imports {
if imports.contains_key(name) && !name.contains('*') {
let maybe_resolved = self.resolve_package_target(
package_json_path.as_ref().unwrap(),
target,
best_match_subpath.unwrap(),
best_match.to_string(),
imports.get(name).unwrap().to_owned(),
"".to_string(),
name.to_string(),
referrer,
referrer_kind,
true,
false,
true,
conditions,
mode,
@ -620,6 +579,50 @@ impl NodeResolver {
if let Some(resolved) = maybe_resolved {
return Ok(resolved);
}
} else {
let mut best_match = "";
let mut best_match_subpath = None;
for key in imports.keys() {
let pattern_index = key.find('*');
if let Some(pattern_index) = pattern_index {
let key_sub = &key[0..=pattern_index];
if name.starts_with(key_sub) {
let pattern_trailer = &key[pattern_index + 1..];
if name.len() > key.len()
&& name.ends_with(&pattern_trailer)
&& pattern_key_compare(best_match, key) == 1
&& key.rfind('*') == Some(pattern_index)
{
best_match = key;
best_match_subpath = Some(
name
[pattern_index..=(name.len() - pattern_trailer.len())]
.to_string(),
);
}
}
}
}
if !best_match.is_empty() {
let target = imports.get(best_match).unwrap().to_owned();
let maybe_resolved = self.resolve_package_target(
package_json_path.as_ref().unwrap(),
target,
best_match_subpath.unwrap(),
best_match.to_string(),
referrer,
referrer_kind,
true,
true,
conditions,
mode,
permissions,
)?;
if let Some(resolved) = maybe_resolved {
return Ok(resolved);
}
}
}
}
}
@ -988,8 +991,10 @@ impl NodeResolver {
parse_package_name(specifier, referrer)?;
// ResolveSelf
let package_config =
self.get_package_scope_config(referrer, permissions)?;
let Some(package_config) =
self.get_package_scope_config(referrer, permissions)? else {
return Ok(None);
};
if package_config.exists
&& package_config.name.as_ref() == Some(&package_name)
{
@ -1063,27 +1068,35 @@ impl NodeResolver {
&self,
referrer: &ModuleSpecifier,
permissions: &dyn NodePermissions,
) -> Result<PackageJson, AnyError> {
let root_folder = self
) -> Result<Option<PackageJson>, AnyError> {
let Some(root_folder) = self
.npm_resolver
.resolve_package_folder_from_path(&referrer.to_file_path().unwrap())?;
.resolve_package_folder_from_path(&referrer.to_file_path().unwrap())? else {
return Ok(None);
};
let package_json_path = root_folder.join("package.json");
self.load_package_json(permissions, package_json_path)
self
.load_package_json(permissions, package_json_path)
.map(Some)
}
pub(super) fn get_closest_package_json(
&self,
url: &ModuleSpecifier,
permissions: &dyn NodePermissions,
) -> Result<PackageJson, AnyError> {
let package_json_path = self.get_closest_package_json_path(url)?;
self.load_package_json(permissions, package_json_path)
) -> Result<Option<PackageJson>, AnyError> {
let Some(package_json_path) = self.get_closest_package_json_path(url)? else {
return Ok(None);
};
self
.load_package_json(permissions, package_json_path)
.map(Some)
}
fn get_closest_package_json_path(
&self,
url: &ModuleSpecifier,
) -> Result<PathBuf, AnyError> {
) -> Result<Option<PathBuf>, AnyError> {
let file_path = url.to_file_path().unwrap();
let current_dir = deno_core::strip_unc_prefix(
self.fs.realpath_sync(file_path.parent().unwrap())?,
@ -1091,20 +1104,22 @@ impl NodeResolver {
let mut current_dir = current_dir.as_path();
let package_json_path = current_dir.join("package.json");
if self.fs.exists(&package_json_path) {
return Ok(package_json_path);
return Ok(Some(package_json_path));
}
let root_pkg_folder = self
let Some(root_pkg_folder) = self
.npm_resolver
.resolve_package_folder_from_path(current_dir)?;
.resolve_package_folder_from_path(current_dir)? else {
return Ok(None);
};
while current_dir.starts_with(&root_pkg_folder) {
current_dir = current_dir.parent().unwrap();
let package_json_path = current_dir.join("package.json");
if self.fs.exists(&package_json_path) {
return Ok(package_json_path);
return Ok(Some(package_json_path));
}
}
bail!("did not find package.json in {}", root_pkg_folder.display())
Ok(None)
}
pub(super) fn load_package_json(