mirror of
https://github.com/denoland/deno.git
synced 2024-12-24 08:09:08 -05:00
fix(check): attempt to resolve types from pkg before @types
pkg (#24152)
I've been meaning to fix this for ages, but I finally ran into it here: https://github.com/dsherret/ts-ast-viewer/actions/runs/9432038675/job/25981325408 We need to resolve the `@types` package as a fallback instead of eagerly resolving it.
This commit is contained in:
parent
2ca9466dcd
commit
4cd6a81619
18 changed files with 96 additions and 101 deletions
|
@ -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<PathBuf, AnyError> {
|
||||
fn inner(
|
||||
fs: &dyn FileSystem,
|
||||
name: &str,
|
||||
referrer: &ModuleSpecifier,
|
||||
mode: NodeResolutionMode,
|
||||
) -> Result<PathBuf, AnyError> {
|
||||
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)?)
|
||||
}
|
||||
|
||||
|
|
|
@ -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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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<PathBuf, AnyError> {
|
||||
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());
|
||||
|
|
|
@ -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<PathBuf, AnyError>;
|
||||
|
||||
fn resolve_package_cache_folder_id_from_specifier(
|
||||
|
|
|
@ -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<NpmResolutionPackage, Box<PackageNotFoundFromReferrerError>> {
|
||||
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<PathBuf, AnyError> {
|
||||
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)
|
||||
}
|
||||
|
||||
|
|
|
@ -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<PathBuf, AnyError> {
|
||||
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);
|
||||
|
|
|
@ -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)?;
|
||||
}
|
||||
|
|
|
@ -307,7 +307,6 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer> NodeCodeTranslator<TCjsCodeAnalyzer> {
|
|||
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");
|
||||
|
|
|
@ -162,7 +162,6 @@ pub trait NpmResolver: std::fmt::Debug + MaybeSend + MaybeSync {
|
|||
&self,
|
||||
specifier: &str,
|
||||
referrer: &ModuleSpecifier,
|
||||
mode: NodeResolutionMode,
|
||||
) -> Result<PathBuf, AnyError>;
|
||||
|
||||
fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool;
|
||||
|
|
|
@ -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())
|
||||
|
|
|
@ -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<Option<ModuleSpecifier>, 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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,7 @@
|
|||
{
|
||||
"envs": {
|
||||
"DENO_FUTURE": "1"
|
||||
},
|
||||
"args": "check --quiet main.ts",
|
||||
"output": ""
|
||||
}
|
3
tests/specs/npm/check_prefers_non_types_node_pkg/main.ts
Normal file
3
tests/specs/npm/check_prefers_non_types_node_pkg/main.ts
Normal file
|
@ -0,0 +1,3 @@
|
|||
import { compressToEncodedURIComponent } from "lz-string";
|
||||
|
||||
console.log(compressToEncodedURIComponent("Hello, World!"));
|
12
tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/@types/lz-string/package.json
generated
vendored
Normal file
12
tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/@types/lz-string/package.json
generated
vendored
Normal file
|
@ -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."
|
||||
}
|
1
tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/index.d.ts
generated
vendored
Normal file
1
tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/index.d.ts
generated
vendored
Normal file
|
@ -0,0 +1 @@
|
|||
export function compressToEncodedURIComponent(input: string): string;
|
1
tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/index.js
generated
vendored
Normal file
1
tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/index.js
generated
vendored
Normal file
|
@ -0,0 +1 @@
|
|||
module.exports.compressToEncodedURIComponent = (a) => a;
|
4
tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/package.json
generated
vendored
Normal file
4
tests/specs/npm/check_prefers_non_types_node_pkg/node_modules/lz-string/package.json
generated
vendored
Normal file
|
@ -0,0 +1,4 @@
|
|||
{
|
||||
"name": "lz-string",
|
||||
"version": "1.5.0"
|
||||
}
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"dependencies": {
|
||||
"lz-string": "*",
|
||||
"@types/lz-string": "*"
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue