diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index e922662ad7..18464b178c 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -943,7 +943,7 @@ impl Documents { /// tsc when type checking. pub fn resolve( &self, - specifiers: Vec, + specifiers: &[String], referrer: &ModuleSpecifier, maybe_npm_resolver: Option<&NpmPackageResolver>, ) -> Option>> { @@ -955,7 +955,7 @@ impl Documents { // we're in an npm package, so use node resolution results.push(Some(NodeResolution::into_specifier_and_media_type( node::node_resolve( - &specifier, + specifier, referrer, NodeResolutionMode::Types, npm_resolver, @@ -968,13 +968,13 @@ impl Documents { } // handle npm: urls if specifier.starts_with("asset:") { - if let Ok(specifier) = ModuleSpecifier::parse(&specifier) { + if let Ok(specifier) = ModuleSpecifier::parse(specifier) { let media_type = MediaType::from(&specifier); results.push(Some((specifier, media_type))); } else { results.push(None); } - } else if let Some(dep) = dependencies.deps.get(&specifier) { + } else if let Some(dep) = dependencies.deps.get(specifier) { if let Resolved::Ok { specifier, .. } = &dep.maybe_type { results.push(self.resolve_dependency(specifier, maybe_npm_resolver)); } else if let Resolved::Ok { specifier, .. } = &dep.maybe_code { @@ -983,12 +983,12 @@ impl Documents { results.push(None); } } else if let Some(Resolved::Ok { specifier, .. }) = - self.resolve_imports_dependency(&specifier) + self.resolve_imports_dependency(specifier) { // clone here to avoid double borrow of self let specifier = specifier.clone(); results.push(self.resolve_dependency(&specifier, maybe_npm_resolver)); - } else if let Ok(npm_ref) = NpmPackageReference::from_str(&specifier) { + } else if let Ok(npm_ref) = NpmPackageReference::from_str(specifier) { results.push(maybe_npm_resolver.map(|npm_resolver| { NodeResolution::into_specifier_and_media_type( node_resolve_npm_reference( diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 88de60131c..f703598892 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -2726,7 +2726,7 @@ fn op_resolve( let referrer = state.normalize_specifier(&args.base)?; let result = if let Some(resolved) = state.state_snapshot.documents.resolve( - args.specifiers, + &args.specifiers, &referrer, state.state_snapshot.maybe_npm_resolver.as_ref(), ) { diff --git a/cli/node/mod.rs b/cli/node/mod.rs index 15e9a3702c..d0c3ed03f3 100644 --- a/cli/node/mod.rs +++ b/cli/node/mod.rs @@ -95,15 +95,11 @@ impl NodeResolution { }, ) } - maybe_response => { - let specifier = match maybe_response { - Some(response) => response.into_url(), - None => { - ModuleSpecifier::parse("deno:///missing_dependency.d.ts").unwrap() - } - }; - (specifier, MediaType::Dts) - } + Some(resolution) => (resolution.into_url(), MediaType::Dts), + None => ( + ModuleSpecifier::parse("deno:///missing_dependency.d.ts").unwrap(), + MediaType::Dts, + ), } } } diff --git a/cli/npm/resolvers/global.rs b/cli/npm/resolvers/global.rs index 07df80d307..093e15deda 100644 --- a/cli/npm/resolvers/global.rs +++ b/cli/npm/resolvers/global.rs @@ -13,15 +13,16 @@ use deno_core::futures::future::BoxFuture; use deno_core::futures::FutureExt; use deno_core::url::Url; use deno_runtime::deno_node::NodeResolutionMode; -use deno_runtime::deno_node::PackageJson; use crate::args::Lockfile; +use crate::npm::cache::NpmPackageCacheFolderId; use crate::npm::resolution::NpmResolution; use crate::npm::resolution::NpmResolutionSnapshot; use crate::npm::resolvers::common::cache_packages; use crate::npm::NpmCache; use crate::npm::NpmPackageId; use crate::npm::NpmPackageReq; +use crate::npm::NpmResolutionPackage; use crate::npm::RealNpmRegistryApi; use super::common::ensure_registry_read_permission; @@ -61,6 +62,17 @@ impl GlobalNpmPackageResolver { .cache .package_folder_for_id(&folder_id, &self.registry_url) } + + 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) + } } impl InnerNpmPackageResolver for GlobalNpmPackageResolver { @@ -81,30 +93,20 @@ impl InnerNpmPackageResolver for GlobalNpmPackageResolver { let referrer_pkg_id = self .cache .resolve_package_folder_id_from_specifier(referrer, &self.registry_url)?; - let pkg_result = self - .resolution - .resolve_package_from_package(name, &referrer_pkg_id); - if mode.is_types() && !name.starts_with("@types/") { - // When doing types resolution, the package must contain a "types" - // entry, or else it will then search for a @types package - if let Ok(pkg) = pkg_result { - let package_folder = self.package_folder(&pkg.id); - let package_json = PackageJson::load_skip_read_permission( - package_folder.join("package.json"), - )?; - if package_json.types.is_some() { - return Ok(package_folder); - } + 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)?, } - - let name = types_package_name(name); - let pkg = self - .resolution - .resolve_package_from_package(&name, &referrer_pkg_id)?; - Ok(self.package_folder(&pkg.id)) } else { - Ok(self.package_folder(&pkg_result?.id)) - } + self + .resolution + .resolve_package_from_package(name, &referrer_pkg_id)? + }; + Ok(self.package_folder(&pkg.id)) } fn resolve_package_folder_from_specifier( diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index d30775dd42..a9c459d122 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -189,7 +189,11 @@ impl NpmPackageResolver { .inner .resolve_package_folder_from_deno_module(pkg_req)?; let path = canonicalize_path_maybe_not_exists(&path)?; - log::debug!("Resolved {} to {}", pkg_req, path.display()); + log::debug!( + "Resolved package folder of {} to {}", + pkg_req, + path.display() + ); Ok(path) } @@ -217,7 +221,11 @@ impl NpmPackageResolver { let path = self .inner .resolve_package_folder_from_specifier(specifier)?; - log::debug!("Resolved {} to {}", specifier, path.display()); + log::debug!( + "Resolved package folder of {} to {}", + specifier, + path.display() + ); Ok(path) } diff --git a/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/dist/main.d.ts b/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/dist/main.d.ts index 2341a14f08..8942f08dda 100644 --- a/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/dist/main.d.ts +++ b/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/dist/main.d.ts @@ -1 +1 @@ -export function getValue(): 5; +export { getValue } from "@denotest/types-entry-value-not-exists"; diff --git a/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/dist/main.js b/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/dist/main.js index d0c5dbc70c..63c7ef0cee 100644 --- a/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/dist/main.js +++ b/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/dist/main.js @@ -1 +1 @@ -module.exports.getValue = () => 5; +module.exports.getValue = require("@denotest/types-entry-value-not-exists").getValue; diff --git a/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/package.json b/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/package.json index 377af2bae6..6abccec985 100644 --- a/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/package.json +++ b/cli/tests/testdata/npm/registry/@denotest/types-no-types-entry/1.0.0/package.json @@ -1,5 +1,8 @@ { "name": "@denotest/types-no-types-entry", "version": "1.0.0", - "main": "./dist/main.js" + "main": "./dist/main.js", + "dependencies": { + "@denotest/types-entry-value-not-exists": "^1.0" + } } diff --git a/cli/tests/testdata/npm/types_no_types_entry/main.out b/cli/tests/testdata/npm/types_no_types_entry/main.out index 2aa78dd25c..429940fffa 100644 --- a/cli/tests/testdata/npm/types_no_types_entry/main.out +++ b/cli/tests/testdata/npm/types_no_types_entry/main.out @@ -1,4 +1,6 @@ Download http://localhost:4545/npm/registry/@denotest/types-no-types-entry +Download http://localhost:4545/npm/registry/@denotest/types-entry-value-not-exists +Download http://localhost:4545/npm/registry/@denotest/types-entry-value-not-exists/1.0.0.tgz Download http://localhost:4545/npm/registry/@denotest/types-no-types-entry/1.0.0.tgz Check file://[WILDCARD]/types_no_types_entry/main.ts error: TS2322 [ERROR]: Type 'number' is not assignable to type 'string'. diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 1fb4d32d30..f7cc6d6e8a 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -542,7 +542,8 @@ fn op_resolve( args: ResolveArgs, ) -> Result, AnyError> { let state = state.borrow_mut::(); - let mut resolved: Vec<(String, String)> = Vec::new(); + let mut resolved: Vec<(String, String)> = + Vec::with_capacity(args.specifiers.len()); let referrer = if let Some(remapped_specifier) = state.remapped_specifiers.get(&args.base) { @@ -661,6 +662,7 @@ fn op_resolve( ".d.ts".to_string(), ), }; + log::debug!("Resolved {} to {:?}", specifier, result); resolved.push(result); } }