From f0c9e276c82b0be58348142440b4dfd49a891e72 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 19 Nov 2022 10:40:01 -0500 Subject: [PATCH] fix(npm): handle directory resolution when resolving declaration files (#16706) Also fixes resolving specifiers like `./something.generated` in declaration files. Closes #16695 --- .../@denotest/check-error/1.0.0/index.d.ts | 4 + .../check-error/1.0.0/other_dir.d.ts | 1 + .../check-error/1.0.0/other_dir/index.js | 1 + .../check-error/1.0.0/sub_dir/index.d.ts | 1 + .../check-error/1.0.0/sub_dir/index.js | 1 + .../check-error/1.0.0/sub_dir/lib.d.ts | 1 + ext/node/resolution.rs | 91 ++++++++++++++++--- 7 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/other_dir.d.ts create mode 100644 cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/other_dir/index.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/index.d.ts create mode 100644 cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/index.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/lib.d.ts diff --git a/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/index.d.ts b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/index.d.ts index 673c0035ef..bfb0483c28 100644 --- a/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/index.d.ts +++ b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/index.d.ts @@ -4,3 +4,7 @@ export class Class1 extends Class2 { export class Class2 extends Class1 { } + +// these should be fine though +export { subDir } from "./sub_dir"; +export { otherDir } from "./other_dir"; diff --git a/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/other_dir.d.ts b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/other_dir.d.ts new file mode 100644 index 0000000000..e7254c16c2 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/other_dir.d.ts @@ -0,0 +1 @@ +export const otherDir: 2; diff --git a/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/other_dir/index.js b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/other_dir/index.js new file mode 100644 index 0000000000..56259f22d2 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/other_dir/index.js @@ -0,0 +1 @@ +module.exports.otherDir = 2; diff --git a/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/index.d.ts b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/index.d.ts new file mode 100644 index 0000000000..f41a696fd2 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/index.d.ts @@ -0,0 +1 @@ +export * from './lib'; diff --git a/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/index.js b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/index.js new file mode 100644 index 0000000000..3dfac4c235 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/index.js @@ -0,0 +1 @@ +module.exports.subDir = 1; diff --git a/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/lib.d.ts b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/lib.d.ts new file mode 100644 index 0000000000..e5834b52bb --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/check-error/1.0.0/sub_dir/lib.d.ts @@ -0,0 +1 @@ +export const subDir: 1; diff --git a/ext/node/resolution.rs b/ext/node/resolution.rs index 9a6bc3aebe..409f9fcb5b 100644 --- a/ext/node/resolution.rs +++ b/ext/node/resolution.rs @@ -32,6 +32,25 @@ pub fn path_to_declaration_path( path: PathBuf, referrer_kind: NodeModuleKind, ) -> PathBuf { + fn probe_extensions( + path: &Path, + referrer_kind: NodeModuleKind, + ) -> Option { + let specific_dts_path = match referrer_kind { + NodeModuleKind::Cjs => with_known_extension(path, "d.cts"), + NodeModuleKind::Esm => with_known_extension(path, "d.mts"), + }; + if specific_dts_path.exists() { + return Some(specific_dts_path); + } + let dts_path = with_known_extension(path, "d.ts"); + if dts_path.exists() { + Some(dts_path) + } else { + None + } + } + let lowercase_path = path.to_string_lossy().to_lowercase(); if lowercase_path.ends_with(".d.ts") || lowercase_path.ends_with(".d.cts") @@ -39,20 +58,54 @@ pub fn path_to_declaration_path( { return path; } - let specific_dts_path = match referrer_kind { - NodeModuleKind::Cjs => path.with_extension("d.cts"), - NodeModuleKind::Esm => path.with_extension("d.mts"), - }; - if specific_dts_path.exists() { - specific_dts_path - } else { - let dts_path = path.with_extension("d.ts"); - if dts_path.exists() { - dts_path - } else { - path + if let Some(path) = probe_extensions(&path, referrer_kind) { + return path; + } + if path.is_dir() { + if let Some(path) = probe_extensions(&path.join("index"), referrer_kind) { + return path; } } + path +} + +/// Alternate `PathBuf::with_extension` that will handle known extensions +/// more intelligently. +pub fn with_known_extension(path: &Path, ext: &str) -> PathBuf { + const NON_DECL_EXTS: &[&str] = &["cjs", "js", "json", "jsx", "mjs", "tsx"]; + const DECL_EXTS: &[&str] = &["cts", "mts", "ts"]; + + let file_name = match path.file_name() { + Some(value) => value.to_string_lossy(), + None => return path.to_path_buf(), + }; + let lowercase_file_name = file_name.to_lowercase(); + let period_index = lowercase_file_name.rfind('.').and_then(|period_index| { + let ext = &lowercase_file_name[period_index + 1..]; + if DECL_EXTS.contains(&ext) { + if let Some(next_period_index) = + lowercase_file_name[..period_index].rfind('.') + { + if &lowercase_file_name[next_period_index + 1..period_index] == "d" { + Some(next_period_index) + } else { + Some(period_index) + } + } else { + Some(period_index) + } + } else if NON_DECL_EXTS.contains(&ext) { + Some(period_index) + } else { + None + } + }); + + let file_name = match period_index { + Some(period_index) => &file_name[..period_index], + None => &file_name, + }; + path.with_file_name(format!("{}.{}", file_name, ext)) } fn to_specifier_display_string(url: &ModuleSpecifier) -> String { @@ -854,4 +907,18 @@ mod tests { ) ); } + + #[test] + fn test_with_known_extension() { + let cases = &[ + ("test", "d.ts", "test.d.ts"), + ("test.d.ts", "ts", "test.ts"), + ("test.worker", "d.ts", "test.worker.d.ts"), + ("test.d.mts", "js", "test.js"), + ]; + for (path, ext, expected) in cases { + let actual = with_known_extension(&PathBuf::from(path), ext); + assert_eq!(actual.to_string_lossy(), *expected); + } + } }