From 826e42a5b5880c974ae019a7a21aade6a718062c Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 1 Nov 2024 12:27:00 -0400 Subject: fix: improved support for cjs and cts modules (#26558) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * cts support * better cjs/cts type checking * deno compile cjs/cts support * More efficient detect cjs (going towards stabilization) * Determination of whether .js, .ts, .jsx, or .tsx is cjs or esm is only done after loading * Support `import x = require(...);` Co-authored-by: Bartek Iwańczuk --- resolvers/node/resolution.rs | 196 +++++++++---------------------------------- 1 file changed, 40 insertions(+), 156 deletions(-) (limited to 'resolvers/node/resolution.rs') diff --git a/resolvers/node/resolution.rs b/resolvers/node/resolution.rs index 811583a5e..d44539e97 100644 --- a/resolvers/node/resolution.rs +++ b/resolvers/node/resolution.rs @@ -6,9 +6,6 @@ use std::path::PathBuf; use anyhow::bail; use anyhow::Error as AnyError; -use deno_media_type::MediaType; -use deno_package_json::PackageJsonRc; -use deno_path_util::strip_unc_prefix; use deno_path_util::url_from_file_path; use serde_json::Map; use serde_json::Value; @@ -16,8 +13,6 @@ use url::Url; use crate::env::NodeResolverEnv; use crate::errors; -use crate::errors::CanonicalizingPkgJsonDirError; -use crate::errors::ClosestPkgJsonError; use crate::errors::DataUrlReferrerError; use crate::errors::FinalizeResolutionError; use crate::errors::InvalidModuleSpecifierError; @@ -32,7 +27,6 @@ use crate::errors::PackageExportsResolveError; use crate::errors::PackageImportNotDefinedError; use crate::errors::PackageImportsResolveError; use crate::errors::PackageImportsResolveErrorKind; -use crate::errors::PackageJsonLoadError; use crate::errors::PackagePathNotExportedError; use crate::errors::PackageResolveError; use crate::errors::PackageSubpathResolveError; @@ -42,14 +36,13 @@ use crate::errors::PackageTargetResolveError; use crate::errors::PackageTargetResolveErrorKind; use crate::errors::ResolveBinaryCommandsError; use crate::errors::ResolvePkgJsonBinExportError; -use crate::errors::ResolvePkgSubpathFromDenoModuleError; -use crate::errors::TypeScriptNotSupportedInNpmError; use crate::errors::TypesNotFoundError; use crate::errors::TypesNotFoundErrorData; use crate::errors::UnsupportedDirImportError; use crate::errors::UnsupportedEsmUrlSchemeError; -use crate::errors::UrlToNodeResolutionError; +use crate::npm::InNpmPackageCheckerRc; use crate::NpmResolverRc; +use crate::PackageJsonResolverRc; use crate::PathClean; use deno_package_json::PackageJson; @@ -73,16 +66,14 @@ impl NodeResolutionMode { #[derive(Debug)] pub enum NodeResolution { - Esm(Url), - CommonJs(Url), + Module(Url), BuiltIn(String), } impl NodeResolution { pub fn into_url(self) -> Url { match self { - Self::Esm(u) => u, - Self::CommonJs(u) => u, + Self::Module(u) => u, Self::BuiltIn(specifier) => { if specifier.starts_with("node:") { Url::parse(&specifier).unwrap() @@ -92,42 +83,6 @@ impl NodeResolution { } } } - - pub fn into_specifier_and_media_type( - resolution: Option, - ) -> (Url, MediaType) { - match resolution { - Some(NodeResolution::CommonJs(specifier)) => { - let media_type = MediaType::from_specifier(&specifier); - ( - specifier, - match media_type { - MediaType::JavaScript | MediaType::Jsx => MediaType::Cjs, - MediaType::TypeScript | MediaType::Tsx => MediaType::Cts, - MediaType::Dts => MediaType::Dcts, - _ => media_type, - }, - ) - } - Some(NodeResolution::Esm(specifier)) => { - let media_type = MediaType::from_specifier(&specifier); - ( - specifier, - match media_type { - MediaType::JavaScript | MediaType::Jsx => MediaType::Mjs, - MediaType::TypeScript | MediaType::Tsx => MediaType::Mts, - MediaType::Dts => MediaType::Dmts, - _ => media_type, - }, - ) - } - Some(resolution) => (resolution.into_url(), MediaType::Dts), - None => ( - Url::parse("internal:///missing_dependency.d.ts").unwrap(), - MediaType::Dts, - ), - } - } } #[allow(clippy::disallowed_types)] @@ -136,16 +91,28 @@ pub type NodeResolverRc = crate::sync::MaybeArc>; #[derive(Debug)] pub struct NodeResolver { env: TEnv, + in_npm_pkg_checker: InNpmPackageCheckerRc, npm_resolver: NpmResolverRc, + pkg_json_resolver: PackageJsonResolverRc, } impl NodeResolver { - pub fn new(env: TEnv, npm_resolver: NpmResolverRc) -> Self { - Self { env, npm_resolver } + pub fn new( + env: TEnv, + in_npm_pkg_checker: InNpmPackageCheckerRc, + npm_resolver: NpmResolverRc, + pkg_json_resolver: PackageJsonResolverRc, + ) -> Self { + Self { + env, + in_npm_pkg_checker, + npm_resolver, + pkg_json_resolver, + } } pub fn in_npm_package(&self, specifier: &Url) -> bool { - self.npm_resolver.in_npm_package(specifier) + self.in_npm_pkg_checker.in_npm_package(specifier) } /// This function is an implementation of `defaultResolve` in @@ -166,7 +133,7 @@ impl NodeResolver { if let Ok(url) = Url::parse(specifier) { if url.scheme() == "data" { - return Ok(NodeResolution::Esm(url)); + return Ok(NodeResolution::Module(url)); } if let Some(module_name) = @@ -191,7 +158,7 @@ impl NodeResolver { let url = referrer .join(specifier) .map_err(|source| DataUrlReferrerError { source })?; - return Ok(NodeResolution::Esm(url)); + return Ok(NodeResolution::Module(url)); } } @@ -212,7 +179,7 @@ impl NodeResolver { }; let url = self.finalize_resolution(url, Some(referrer))?; - let resolve_response = self.url_to_node_resolution(url)?; + let resolve_response = NodeResolution::Module(url); // TODO(bartlomieju): skipped checking errors for commonJS resolution and // "preserveSymlinksMain"/"preserveSymlinks" options. Ok(resolve_response) @@ -236,6 +203,7 @@ impl NodeResolver { })?) } else if specifier.starts_with('#') { let pkg_config = self + .pkg_json_resolver .get_closest_package_json(referrer) .map_err(PackageImportsResolveErrorKind::ClosestPkgJson) .map_err(|err| PackageImportsResolveError(Box::new(err)))?; @@ -332,7 +300,7 @@ impl NodeResolver { package_subpath: Option<&str>, maybe_referrer: Option<&Url>, mode: NodeResolutionMode, - ) -> Result { + ) -> Result { let node_module_kind = NodeModuleKind::Esm; let package_subpath = package_subpath .map(|s| format!("./{s}")) @@ -345,10 +313,9 @@ impl NodeResolver { DEFAULT_CONDITIONS, mode, )?; - let resolve_response = self.url_to_node_resolution(resolved_url)?; // TODO(bartlomieju): skipped checking errors for commonJS resolution and // "preserveSymlinksMain"/"preserveSymlinks" options. - Ok(resolve_response) + Ok(resolved_url) } pub fn resolve_binary_commands( @@ -356,7 +323,9 @@ impl NodeResolver { package_folder: &Path, ) -> Result, ResolveBinaryCommandsError> { let pkg_json_path = package_folder.join("package.json"); - let Some(package_json) = self.load_package_json(&pkg_json_path)? else { + let Some(package_json) = + self.pkg_json_resolver.load_package_json(&pkg_json_path)? + else { return Ok(Vec::new()); }; @@ -381,9 +350,11 @@ impl NodeResolver { &self, package_folder: &Path, sub_path: Option<&str>, - ) -> Result { + ) -> Result { let pkg_json_path = package_folder.join("package.json"); - let Some(package_json) = self.load_package_json(&pkg_json_path)? else { + let Some(package_json) = + self.pkg_json_resolver.load_package_json(&pkg_json_path)? + else { return Err(ResolvePkgJsonBinExportError::MissingPkgJson { pkg_json_path, }); @@ -396,37 +367,9 @@ impl NodeResolver { })?; let url = url_from_file_path(&package_folder.join(bin_entry)).unwrap(); - let resolve_response = self.url_to_node_resolution(url)?; // TODO(bartlomieju): skipped checking errors for commonJS resolution and // "preserveSymlinksMain"/"preserveSymlinks" options. - Ok(resolve_response) - } - - pub fn url_to_node_resolution( - &self, - url: Url, - ) -> Result { - let url_str = url.as_str().to_lowercase(); - if url_str.starts_with("http") || url_str.ends_with(".json") { - Ok(NodeResolution::Esm(url)) - } else if url_str.ends_with(".js") || url_str.ends_with(".d.ts") { - let maybe_package_config = self.get_closest_package_json(&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)) - } else if url_str.ends_with(".ts") || url_str.ends_with(".mts") { - if self.in_npm_package(&url) { - Err(TypeScriptNotSupportedInNpmError { specifier: url }.into()) - } else { - Ok(NodeResolution::Esm(url)) - } - } else { - Ok(NodeResolution::CommonJs(url)) - } + Ok(url) } /// Checks if the resolved file has a corresponding declaration file. @@ -1101,7 +1044,9 @@ impl NodeResolver { let (package_name, package_subpath, _is_scoped) = parse_npm_pkg_name(specifier, referrer)?; - if let Some(package_config) = self.get_closest_package_json(referrer)? { + if let Some(package_config) = + self.pkg_json_resolver.get_closest_package_json(referrer)? + { // ResolveSelf if package_config.name.as_ref() == Some(&package_name) { if let Some(exports) = &package_config.exports { @@ -1216,7 +1161,10 @@ impl NodeResolver { mode: NodeResolutionMode, ) -> Result { let package_json_path = package_dir_path.join("package.json"); - match self.load_package_json(&package_json_path)? { + match self + .pkg_json_resolver + .load_package_json(&package_json_path)? + { Some(pkg_json) => self.resolve_package_subpath( &pkg_json, package_subpath, @@ -1337,70 +1285,6 @@ impl NodeResolver { } } - pub fn get_closest_package_json( - &self, - url: &Url, - ) -> Result, ClosestPkgJsonError> { - let Ok(file_path) = deno_path_util::url_to_file_path(url) else { - return Ok(None); - }; - self.get_closest_package_json_from_path(&file_path) - } - - pub fn get_closest_package_json_from_path( - &self, - file_path: &Path, - ) -> Result, ClosestPkgJsonError> { - // we use this for deno compile using byonm because the script paths - // won't be in virtual file system, but the package.json paths will be - fn canonicalize_first_ancestor_exists( - dir_path: &Path, - env: &dyn NodeResolverEnv, - ) -> Result, std::io::Error> { - for ancestor in dir_path.ancestors() { - match env.realpath_sync(ancestor) { - Ok(dir_path) => return Ok(Some(dir_path)), - Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - // keep searching - } - Err(err) => return Err(err), - } - } - Ok(None) - } - - let parent_dir = file_path.parent().unwrap(); - let Some(start_dir) = canonicalize_first_ancestor_exists( - parent_dir, &self.env, - ) - .map_err(|source| CanonicalizingPkgJsonDirError { - dir_path: parent_dir.to_path_buf(), - source, - })? - else { - return Ok(None); - }; - let start_dir = strip_unc_prefix(start_dir); - for current_dir in start_dir.ancestors() { - let package_json_path = current_dir.join("package.json"); - if let Some(pkg_json) = self.load_package_json(&package_json_path)? { - return Ok(Some(pkg_json)); - } - } - - Ok(None) - } - - pub fn load_package_json( - &self, - package_json_path: &Path, - ) -> Result, PackageJsonLoadError> { - crate::package_json::load_pkg_json( - self.env.pkg_json_fs(), - package_json_path, - ) - } - pub(super) fn legacy_main_resolve( &self, package_json: &PackageJson, -- cgit v1.2.3 From f091d1ad69b4e5217ae3272b641171781a372c4f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 13 Nov 2024 10:10:09 -0500 Subject: feat(node): stabilize detecting if CJS via `"type": "commonjs"` in a package.json (#26439) This will respect `"type": "commonjs"` in a package.json to determine if `.js`/`.jsx`/`.ts`/.tsx` files are CJS or ESM. If the file is found to be ESM it will be loaded as ESM though. --- resolvers/node/resolution.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'resolvers/node/resolution.rs') diff --git a/resolvers/node/resolution.rs b/resolvers/node/resolution.rs index d44539e97..fcff29242 100644 --- a/resolvers/node/resolution.rs +++ b/resolvers/node/resolution.rs @@ -50,6 +50,15 @@ pub static DEFAULT_CONDITIONS: &[&str] = &["deno", "node", "import"]; pub static REQUIRE_CONDITIONS: &[&str] = &["require", "node"]; static TYPES_ONLY_CONDITIONS: &[&str] = &["types"]; +fn conditions_from_module_kind( + kind: NodeModuleKind, +) -> &'static [&'static str] { + match kind { + NodeModuleKind::Esm => DEFAULT_CONDITIONS, + NodeModuleKind::Cjs => REQUIRE_CONDITIONS, + } +} + pub type NodeModuleKind = deno_package_json::NodeModuleKind; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -166,8 +175,7 @@ impl NodeResolver { specifier, referrer, referrer_kind, - // even though the referrer may be CJS, if we're here that means we're doing ESM resolution - DEFAULT_CONDITIONS, + conditions_from_module_kind(referrer_kind), mode, )?; @@ -299,9 +307,9 @@ impl NodeResolver { package_dir: &Path, package_subpath: Option<&str>, maybe_referrer: Option<&Url>, + referrer_kind: NodeModuleKind, mode: NodeResolutionMode, ) -> Result { - let node_module_kind = NodeModuleKind::Esm; let package_subpath = package_subpath .map(|s| format!("./{s}")) .unwrap_or_else(|| ".".to_string()); @@ -309,8 +317,8 @@ impl NodeResolver { package_dir, &package_subpath, maybe_referrer, - node_module_kind, - DEFAULT_CONDITIONS, + referrer_kind, + conditions_from_module_kind(referrer_kind), mode, )?; // TODO(bartlomieju): skipped checking errors for commonJS resolution and @@ -441,10 +449,7 @@ impl NodeResolver { /* sub path */ ".", maybe_referrer, referrer_kind, - match referrer_kind { - NodeModuleKind::Esm => DEFAULT_CONDITIONS, - NodeModuleKind::Cjs => REQUIRE_CONDITIONS, - }, + conditions_from_module_kind(referrer_kind), NodeResolutionMode::Types, ); if let Ok(resolution) = resolution_result { -- cgit v1.2.3 From 617350e79c58b6e01984e3d7c7436d243d0e5cff Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 14 Nov 2024 15:24:25 -0500 Subject: refactor(resolver): move more resolution code into deno_resolver (#26873) Follow-up to cjs refactor. This moves most of the resolution code into the deno_resolver crate. Still pending is the npm resolution code. --- resolvers/node/resolution.rs | 51 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 5 deletions(-) (limited to 'resolvers/node/resolution.rs') diff --git a/resolvers/node/resolution.rs b/resolvers/node/resolution.rs index fcff29242..673a61abe 100644 --- a/resolvers/node/resolution.rs +++ b/resolvers/node/resolution.rs @@ -41,7 +41,7 @@ use crate::errors::TypesNotFoundErrorData; use crate::errors::UnsupportedDirImportError; use crate::errors::UnsupportedEsmUrlSchemeError; use crate::npm::InNpmPackageCheckerRc; -use crate::NpmResolverRc; +use crate::NpmPackageFolderResolverRc; use crate::PackageJsonResolverRc; use crate::PathClean; use deno_package_json::PackageJson; @@ -101,7 +101,7 @@ pub type NodeResolverRc = crate::sync::MaybeArc>; pub struct NodeResolver { env: TEnv, in_npm_pkg_checker: InNpmPackageCheckerRc, - npm_resolver: NpmResolverRc, + npm_pkg_folder_resolver: NpmPackageFolderResolverRc, pkg_json_resolver: PackageJsonResolverRc, } @@ -109,13 +109,13 @@ impl NodeResolver { pub fn new( env: TEnv, in_npm_pkg_checker: InNpmPackageCheckerRc, - npm_resolver: NpmResolverRc, + npm_pkg_folder_resolver: NpmPackageFolderResolverRc, pkg_json_resolver: PackageJsonResolverRc, ) -> Self { Self { env, in_npm_pkg_checker, - npm_resolver, + npm_pkg_folder_resolver, pkg_json_resolver, } } @@ -1126,7 +1126,7 @@ impl NodeResolver { mode: NodeResolutionMode, ) -> Result { let package_dir_path = self - .npm_resolver + .npm_pkg_folder_resolver .resolve_package_folder_from_package(package_name, referrer)?; // todo: error with this instead when can't find package @@ -1412,6 +1412,25 @@ impl NodeResolver { ) } } + + /// Resolves a specifier that is pointing into a node_modules folder by canonicalizing it. + /// + /// Returns `None` when the specifier is not in a node_modules folder. + pub fn handle_if_in_node_modules(&self, specifier: &Url) -> Option { + // skip canonicalizing if we definitely know it's unnecessary + if specifier.scheme() == "file" + && specifier.path().contains("/node_modules/") + { + // Specifiers in the node_modules directory are canonicalized + // so canoncalize then check if it's in the node_modules directory. + let specifier = resolve_specifier_into_node_modules(specifier, &|path| { + self.env.realpath_sync(path) + }); + return Some(specifier); + } + + None + } } fn resolve_bin_entry_value<'a>( @@ -1660,6 +1679,28 @@ pub fn parse_npm_pkg_name( Ok((package_name, package_subpath, is_scoped)) } +/// Resolves a specifier that is pointing into a node_modules folder. +/// +/// Note: This should be called whenever getting the specifier from +/// a Module::External(module) reference because that module might +/// not be fully resolved at the time deno_graph is analyzing it +/// because the node_modules folder might not exist at that time. +pub fn resolve_specifier_into_node_modules( + specifier: &Url, + canonicalize: &impl Fn(&Path) -> std::io::Result, +) -> Url { + deno_path_util::url_to_file_path(specifier) + .ok() + // this path might not exist at the time the graph is being created + // because the node_modules folder might not yet exist + .and_then(|path| { + deno_path_util::canonicalize_path_maybe_not_exists(&path, canonicalize) + .ok() + }) + .and_then(|path| deno_path_util::url_from_file_path(&path).ok()) + .unwrap_or_else(|| specifier.clone()) +} + fn pattern_key_compare(a: &str, b: &str) -> i32 { let a_pattern_index = a.find('*'); let b_pattern_index = b.find('*'); -- cgit v1.2.3 From 186b52731c6bb326c4d32905c5e732d082e83465 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 19 Nov 2024 09:57:12 -0500 Subject: fix(node): handle resolving ".//" in npm packages (#26920) The issue was this package had an import like: `".//index.js"` and we resolved that as specified, but node normalizes it to `"./index.js"` so we have to copy node. --- resolvers/node/resolution.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'resolvers/node/resolution.rs') diff --git a/resolvers/node/resolution.rs b/resolvers/node/resolution.rs index 673a61abe..c2ec25aca 100644 --- a/resolvers/node/resolution.rs +++ b/resolvers/node/resolution.rs @@ -202,7 +202,7 @@ impl NodeResolver { mode: NodeResolutionMode, ) -> Result { if should_be_treated_as_relative_or_absolute_path(specifier) { - Ok(referrer.join(specifier).map_err(|err| { + Ok(node_join_url(referrer, specifier).map_err(|err| { NodeResolveRelativeJoinError { path: specifier.to_string(), base: referrer.clone(), @@ -1763,6 +1763,17 @@ fn get_module_name_from_builtin_node_module_specifier( Some(specifier) } +/// Node is more lenient joining paths than the url crate is, +/// so this function handles that. +fn node_join_url(url: &Url, path: &str) -> Result { + if let Some(suffix) = path.strip_prefix(".//") { + // specifier had two leading slashes + url.join(&format!("./{}", suffix)) + } else { + url.join(path) + } +} + #[cfg(test)] mod tests { use serde_json::json; -- cgit v1.2.3