diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs index bfb9bed7f7..ec3fb00e94 100644 --- a/cli/npm/byonm.rs +++ b/cli/npm/byonm.rs @@ -12,7 +12,6 @@ use deno_core::error::AnyError; use deno_core::serde_json; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::NodePermissions; -use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::deno_node::NpmResolver; use deno_runtime::deno_node::PackageJson; use deno_semver::package::PackageReq; @@ -23,7 +22,6 @@ use crate::args::NpmProcessStateKind; use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs; use deno_runtime::fs_util::specifier_to_file_path; -use super::common::types_package_name; use super::CliNpmResolver; use super::InnerCliNpmResolverRef; @@ -97,20 +95,13 @@ impl NpmResolver for ByonmCliNpmResolver { &self, name: &str, referrer: &ModuleSpecifier, - mode: NodeResolutionMode, ) -> Result { fn inner( fs: &dyn FileSystem, name: &str, referrer: &ModuleSpecifier, - mode: NodeResolutionMode, ) -> Result { let referrer_file = specifier_to_file_path(referrer)?; - let types_pkg_name = if mode.is_types() && !name.starts_with("@types/") { - Some(types_package_name(name)) - } else { - None - }; let mut current_folder = referrer_file.parent().unwrap(); loop { let node_modules_folder = if current_folder.ends_with("node_modules") { @@ -119,14 +110,6 @@ impl NpmResolver for ByonmCliNpmResolver { Cow::Owned(current_folder.join("node_modules")) }; - // attempt to resolve the types package first, then fallback to the regular package - if let Some(types_pkg_name) = &types_pkg_name { - let sub_dir = join_package_name(&node_modules_folder, types_pkg_name); - if fs.is_dir_sync(&sub_dir) { - return Ok(sub_dir); - } - } - let sub_dir = join_package_name(&node_modules_folder, name); if fs.is_dir_sync(&sub_dir) { return Ok(sub_dir); @@ -146,7 +129,7 @@ impl NpmResolver for ByonmCliNpmResolver { ); } - let path = inner(&*self.fs, name, referrer, mode)?; + let path = inner(&*self.fs, name, referrer)?; Ok(self.fs.realpath_sync(&path)?) } diff --git a/cli/npm/common.rs b/cli/npm/common.rs index bf45aa5b81..c55f73cd50 100644 --- a/cli/npm/common.rs +++ b/cli/npm/common.rs @@ -3,14 +3,6 @@ use deno_npm::npm_rc::RegistryConfig; use reqwest::header; -/// Gets the corresponding @types package for the provided package name. -pub fn types_package_name(package_name: &str) -> String { - debug_assert!(!package_name.starts_with("@types/")); - // Scoped packages will get two underscores for each slash - // https://github.com/DefinitelyTyped/DefinitelyTyped/tree/15f1ece08f7b498f4b9a2147c2a46e94416ca777#what-about-scoped-packages - format!("@types/{}", package_name.replace('/', "__")) -} - // TODO(bartlomieju): support more auth methods besides token and basic auth pub fn maybe_auth_header_for_npm_registry( registry_config: &RegistryConfig, @@ -31,17 +23,3 @@ pub fn maybe_auth_header_for_npm_registry( None } - -#[cfg(test)] -mod test { - use super::types_package_name; - - #[test] - fn test_types_package_name() { - assert_eq!(types_package_name("name"), "@types/name"); - assert_eq!( - types_package_name("@scoped/package"), - "@types/@scoped__package" - ); - } -} diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs index 2298ea9bd7..d3045a1c9e 100644 --- a/cli/npm/managed/mod.rs +++ b/cli/npm/managed/mod.rs @@ -23,7 +23,6 @@ use deno_npm::NpmResolutionPackage; use deno_npm::NpmSystemInfo; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::NodePermissions; -use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::deno_node::NpmResolver; use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; @@ -531,11 +530,10 @@ impl NpmResolver for ManagedCliNpmResolver { &self, name: &str, referrer: &ModuleSpecifier, - mode: NodeResolutionMode, ) -> Result { let path = self .fs_resolver - .resolve_package_folder_from_package(name, referrer, mode)?; + .resolve_package_folder_from_package(name, referrer)?; let path = canonicalize_path_maybe_not_exists_with_fs(&path, self.fs.as_ref())?; log::debug!("Resolved {} from {} to {}", name, referrer, path.display()); diff --git a/cli/npm/managed/resolvers/common.rs b/cli/npm/managed/resolvers/common.rs index 767b3f9c6d..35f368cb54 100644 --- a/cli/npm/managed/resolvers/common.rs +++ b/cli/npm/managed/resolvers/common.rs @@ -19,7 +19,6 @@ use deno_npm::NpmPackageId; use deno_npm::NpmResolutionPackage; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::NodePermissions; -use deno_runtime::deno_node::NodeResolutionMode; use crate::npm::managed::cache::TarballCache; @@ -41,7 +40,6 @@ pub trait NpmPackageFsResolver: Send + Sync { &self, name: &str, referrer: &ModuleSpecifier, - mode: NodeResolutionMode, ) -> Result; fn resolve_package_cache_folder_id_from_specifier( diff --git a/cli/npm/managed/resolvers/global.rs b/cli/npm/managed/resolvers/global.rs index bfd2f4de36..d10b33e7df 100644 --- a/cli/npm/managed/resolvers/global.rs +++ b/cli/npm/managed/resolvers/global.rs @@ -11,16 +11,12 @@ use deno_ast::ModuleSpecifier; use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::url::Url; -use deno_npm::resolution::PackageNotFoundFromReferrerError; use deno_npm::NpmPackageCacheFolderId; use deno_npm::NpmPackageId; -use deno_npm::NpmResolutionPackage; use deno_npm::NpmSystemInfo; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::NodePermissions; -use deno_runtime::deno_node::NodeResolutionMode; -use super::super::super::common::types_package_name; use super::super::cache::NpmCache; use super::super::cache::TarballCache; use super::super::resolution::NpmResolution; @@ -57,17 +53,6 @@ impl GlobalNpmPackageResolver { system_info, } } - - fn resolve_types_package( - &self, - package_name: &str, - referrer_pkg_id: &NpmPackageCacheFolderId, - ) -> Result> { - let types_name = types_package_name(package_name); - self - .resolution - .resolve_package_from_package(&types_name, referrer_pkg_id) - } } #[async_trait(?Send)] @@ -92,7 +77,6 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver { &self, name: &str, referrer: &ModuleSpecifier, - mode: NodeResolutionMode, ) -> Result { let Some(referrer_pkg_id) = self .cache @@ -100,19 +84,9 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver { 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) { - Ok(pkg) => pkg, - Err(_) => self - .resolution - .resolve_package_from_package(name, &referrer_pkg_id)?, - } - } else { - self - .resolution - .resolve_package_from_package(name, &referrer_pkg_id)? - }; + let pkg = self + .resolution + .resolve_package_from_package(name, &referrer_pkg_id)?; self.package_folder(&pkg.id) } diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs index e93d400e16..d338720b67 100644 --- a/cli/npm/managed/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -39,14 +39,12 @@ use deno_npm::NpmResolutionPackage; use deno_npm::NpmSystemInfo; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodePermissions; -use deno_runtime::deno_node::NodeResolutionMode; use deno_semver::package::PackageNv; use serde::Deserialize; use serde::Serialize; use crate::npm::cache_dir::mixed_case_package_name_encode; -use super::super::super::common::types_package_name; use super::super::cache::NpmCache; use super::super::cache::TarballCache; use super::super::resolution::NpmResolution; @@ -175,7 +173,6 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver { &self, name: &str, referrer: &ModuleSpecifier, - mode: NodeResolutionMode, ) -> Result { let Some(local_path) = self.resolve_folder_for_specifier(referrer)? else { bail!("could not find npm package for '{}'", referrer); @@ -190,15 +187,6 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver { Cow::Owned(current_folder.join("node_modules")) }; - // attempt to resolve the types package first, then fallback to the regular package - if mode.is_types() && !name.starts_with("@types/") { - let sub_dir = - join_package_name(&node_modules_folder, &types_package_name(name)); - if self.fs.is_dir_sync(&sub_dir) { - return Ok(sub_dir); - } - } - let sub_dir = join_package_name(&node_modules_folder, name); if self.fs.is_dir_sync(&sub_dir) { return Ok(sub_dir); diff --git a/cli/resolver.rs b/cli/resolver.rs index 3edc6f4292..994f03f36d 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -514,14 +514,11 @@ impl CliGraphResolver { &self, specifier: &str, referrer: &ModuleSpecifier, - mode: NodeResolutionMode, original_err: AnyError, resolver: &ByonmCliNpmResolver, ) -> Result<(), AnyError> { if let Ok((pkg_name, _, _)) = parse_npm_pkg_name(specifier, referrer) { - match resolver - .resolve_package_folder_from_package(&pkg_name, referrer, mode) - { + match resolver.resolve_package_folder_from_package(&pkg_name, referrer) { Ok(_) => { return Err(original_err); } @@ -650,7 +647,6 @@ impl Resolver for CliGraphResolver { .check_surface_byonm_node_error( specifier, referrer, - to_node_mode(mode), anyhow!("Cannot find \"{}\"", specifier), resolver, ) @@ -659,11 +655,7 @@ impl Resolver for CliGraphResolver { Err(err) => { self .check_surface_byonm_node_error( - specifier, - referrer, - to_node_mode(mode), - err, - resolver, + specifier, referrer, err, resolver, ) .map_err(ResolveError::Other)?; } diff --git a/ext/node/analyze.rs b/ext/node/analyze.rs index df68cb0fc4..3b06a90e08 100644 --- a/ext/node/analyze.rs +++ b/ext/node/analyze.rs @@ -307,7 +307,6 @@ impl NodeCodeTranslator { let module_dir = self.npm_resolver.resolve_package_folder_from_package( package_specifier.as_str(), referrer, - mode, )?; let package_json_path = module_dir.join("package.json"); diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 36c13f8a56..094bea3aee 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -162,7 +162,6 @@ pub trait NpmResolver: std::fmt::Debug + MaybeSend + MaybeSync { &self, specifier: &str, referrer: &ModuleSpecifier, - mode: NodeResolutionMode, ) -> Result; fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool; diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs index 3e1f1c6ae1..3fde6c31ad 100644 --- a/ext/node/ops/require.rs +++ b/ext/node/ops/require.rs @@ -198,7 +198,6 @@ pub fn op_require_resolve_deno_dir( &ModuleSpecifier::from_file_path(&parent_filename).unwrap_or_else(|_| { panic!("Url::from_file_path: [{:?}]", parent_filename) }), - NodeResolutionMode::Execution, ) .ok() .map(|p| p.to_string_lossy().to_string()) diff --git a/ext/node/resolution.rs b/ext/node/resolution.rs index 834b465cde..8047ac4ecb 100644 --- a/ext/node/resolution.rs +++ b/ext/node/resolution.rs @@ -1037,9 +1037,45 @@ impl NodeResolver { } } + let result = self.resolve_package_subpath_for_package( + &package_name, + &package_subpath, + referrer, + referrer_kind, + conditions, + mode, + ); + if mode.is_types() && !matches!(result, Ok(Some(_))) { + // try to resolve with the @types/node package + let package_name = types_package_name(&package_name); + if let Ok(Some(result)) = self.resolve_package_subpath_for_package( + &package_name, + &package_subpath, + referrer, + referrer_kind, + conditions, + mode, + ) { + return Ok(Some(result)); + } + } + + result + } + + #[allow(clippy::too_many_arguments)] + fn resolve_package_subpath_for_package( + &self, + package_name: &str, + package_subpath: &str, + referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, + conditions: &[&str], + mode: NodeResolutionMode, + ) -> Result, AnyError> { let package_dir_path = self .npm_resolver - .resolve_package_folder_from_package(&package_name, referrer, mode)?; + .resolve_package_folder_from_package(package_name, referrer)?; let package_json_path = package_dir_path.join("package.json"); // todo: error with this instead when can't find package @@ -1060,7 +1096,7 @@ impl NodeResolver { .load_package_json(&mut AllowAllNodePermissions, package_json_path)?; self.resolve_package_subpath( &package_json, - &package_subpath, + package_subpath, referrer, referrer_kind, conditions, @@ -1600,6 +1636,14 @@ fn pattern_key_compare(a: &str, b: &str) -> i32 { 0 } +/// Gets the corresponding @types package for the provided package name. +fn types_package_name(package_name: &str) -> String { + debug_assert!(!package_name.starts_with("@types/")); + // Scoped packages will get two underscores for each slash + // https://github.com/DefinitelyTyped/DefinitelyTyped/tree/15f1ece08f7b498f4b9a2147c2a46e94416ca777#what-about-scoped-packages + format!("@types/{}", package_name.replace('/', "__")) +} + #[cfg(test)] mod tests { use deno_core::serde_json::json; @@ -1780,4 +1824,13 @@ mod tests { assert_eq!(actual.to_string_lossy(), *expected); } } + + #[test] + fn test_types_package_name() { + assert_eq!(types_package_name("name"), "@types/name"); + assert_eq!( + types_package_name("@scoped/package"), + "@types/@scoped__package" + ); + } } diff --git a/tests/specs/npm/check_prefers_non_types_node_pkg/__test__.jsonc b/tests/specs/npm/check_prefers_non_types_node_pkg/__test__.jsonc new file mode 100644 index 0000000000..ed3827ef64 --- /dev/null +++ b/tests/specs/npm/check_prefers_non_types_node_pkg/__test__.jsonc @@ -0,0 +1,7 @@ +{ + "envs": { + "DENO_FUTURE": "1" + }, + "args": "check --quiet main.ts", + "output": "" +} diff --git a/tests/specs/npm/check_prefers_non_types_node_pkg/main.ts b/tests/specs/npm/check_prefers_non_types_node_pkg/main.ts new file mode 100644 index 0000000000..8774bdbfc2 --- /dev/null +++ b/tests/specs/npm/check_prefers_non_types_node_pkg/main.ts @@ -0,0 +1,3 @@ +import { compressToEncodedURIComponent } from "lz-string"; + +console.log(compressToEncodedURIComponent("Hello, World!")); diff --git a/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/@types/lz-string/package.json b/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/@types/lz-string/package.json new file mode 100644 index 0000000000..afe623e003 --- /dev/null +++ b/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/@types/lz-string/package.json @@ -0,0 +1,12 @@ +{ + "name": "@types/lz-string", + "version": "1.5.0", + "description": "Stub TypeScript definitions entry for lz-string, which provides its own types definitions", + "main": "", + "scripts": {}, + "license": "MIT", + "dependencies": { + "lz-string": "*" + }, + "deprecated": "This is a stub types definition. lz-string provides its own type definitions, so you do not need this installed." +} diff --git a/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/index.d.ts b/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/index.d.ts new file mode 100644 index 0000000000..b6abfd8ba5 --- /dev/null +++ b/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/index.d.ts @@ -0,0 +1 @@ +export function compressToEncodedURIComponent(input: string): string; diff --git a/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/index.js b/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/index.js new file mode 100644 index 0000000000..603b710ba3 --- /dev/null +++ b/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/index.js @@ -0,0 +1 @@ +module.exports.compressToEncodedURIComponent = (a) => a; diff --git a/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/package.json b/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/package.json new file mode 100644 index 0000000000..f8bfd5d988 --- /dev/null +++ b/tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/package.json @@ -0,0 +1,4 @@ +{ + "name": "lz-string", + "version": "1.5.0" +} \ No newline at end of file diff --git a/tests/specs/npm/check_prefers_non_types_node_pkg/package.json b/tests/specs/npm/check_prefers_non_types_node_pkg/package.json new file mode 100644 index 0000000000..ea3b2d26f4 --- /dev/null +++ b/tests/specs/npm/check_prefers_non_types_node_pkg/package.json @@ -0,0 +1,6 @@ +{ + "dependencies": { + "lz-string": "*", + "@types/lz-string": "*" + } +}