From 7528c7909ca18b5e5e99b4f72665b15da42fa1f5 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 28 Jan 2025 15:32:37 -0500 Subject: [PATCH] fix(check): better handling of TypeScript in npm packages for type checking (#27853) 1. Allows resolving to `.ts` files for type checking. 2. Probes for `.ts` files to use for type checking. To emphasize, this is only for type checking. --- Cargo.lock | 1 + resolvers/node/Cargo.toml | 1 + resolvers/node/resolution.rs | 26 ++++++++++--------- .../specs/check/ts_in_npm_pkg/__test__.jsonc | 5 ++++ tests/specs/check/ts_in_npm_pkg/check.out | 7 +++++ tests/specs/check/ts_in_npm_pkg/main.ts | 3 +++ .../node_modules/package/main.ts | 1 + .../node_modules/package/other.ts | 3 +++ .../node_modules/package/package.json | 7 +++++ tests/specs/check/ts_in_npm_pkg/package.json | 2 ++ 10 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 tests/specs/check/ts_in_npm_pkg/__test__.jsonc create mode 100644 tests/specs/check/ts_in_npm_pkg/check.out create mode 100644 tests/specs/check/ts_in_npm_pkg/main.ts create mode 100644 tests/specs/check/ts_in_npm_pkg/node_modules/package/main.ts create mode 100644 tests/specs/check/ts_in_npm_pkg/node_modules/package/other.ts create mode 100644 tests/specs/check/ts_in_npm_pkg/node_modules/package/package.json create mode 100644 tests/specs/check/ts_in_npm_pkg/package.json diff --git a/Cargo.lock b/Cargo.lock index 1a7ba0221af788..e29f791feca13c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5279,6 +5279,7 @@ dependencies = [ "boxed_error", "dashmap", "deno_error", + "deno_media_type", "deno_package_json", "deno_path_util", "futures", diff --git a/resolvers/node/Cargo.toml b/resolvers/node/Cargo.toml index fd0d8a8f0b81d4..0fef5205336789 100644 --- a/resolvers/node/Cargo.toml +++ b/resolvers/node/Cargo.toml @@ -22,6 +22,7 @@ async-trait.workspace = true boxed_error.workspace = true dashmap.workspace = true deno_error.workspace = true +deno_media_type.workspace = true deno_package_json.workspace = true deno_path_util.workspace = true futures.workspace = true diff --git a/resolvers/node/resolution.rs b/resolvers/node/resolution.rs index 273c630b27dd9a..0764698998dd7d 100644 --- a/resolvers/node/resolution.rs +++ b/resolvers/node/resolution.rs @@ -7,6 +7,7 @@ use std::path::PathBuf; use anyhow::bail; use anyhow::Error as AnyError; +use deno_media_type::MediaType; use deno_package_json::PackageJson; use serde_json::Map; use serde_json::Value; @@ -496,18 +497,18 @@ impl< fn probe_extensions( sys: &TSys, path: &Path, - lowercase_path: &str, + media_type: MediaType, resolution_mode: ResolutionMode, ) -> Option { let mut searched_for_d_mts = false; let mut searched_for_d_cts = false; - if lowercase_path.ends_with(".mjs") { + if media_type == MediaType::Mjs { let d_mts_path = with_known_extension(path, "d.mts"); if sys.fs_is_file_no_err(&d_mts_path) { return Some(d_mts_path); } searched_for_d_mts = true; - } else if lowercase_path.ends_with(".cjs") { + } else if media_type == MediaType::Cjs { let d_cts_path = with_known_extension(path, "d.cts"); if sys.fs_is_file_no_err(&d_cts_path) { return Some(d_cts_path); @@ -534,18 +535,19 @@ impl< return Some(specific_dts_path); } } + let ts_path = with_known_extension(path, "ts"); + if sys.fs_is_file_no_err(&ts_path) { + return Some(ts_path); + } None } - let lowercase_path = path.to_string_lossy().to_lowercase(); - if lowercase_path.ends_with(".d.ts") - || lowercase_path.ends_with(".d.cts") - || lowercase_path.ends_with(".d.mts") - { + let media_type = MediaType::from_path(&path); + if media_type.is_declaration() { return Ok(UrlOrPath::Path(path)); } if let Some(path) = - probe_extensions(&self.sys, &path, &lowercase_path, resolution_mode) + probe_extensions(&self.sys, &path, media_type, resolution_mode) { return Ok(UrlOrPath::Path(path)); } @@ -565,14 +567,14 @@ impl< if let Some(path) = probe_extensions( &self.sys, &index_path, - &index_path.to_string_lossy().to_lowercase(), + MediaType::from_path(&index_path), resolution_mode, ) { return Ok(UrlOrPath::Path(path)); } } - // allow resolving .css files for types resolution - if lowercase_path.ends_with(".css") { + // allow resolving .ts-like or .css files for types resolution + if media_type.is_typed() || media_type == MediaType::Css { return Ok(UrlOrPath::Path(path)); } Err(TypesNotFoundError(Box::new(TypesNotFoundErrorData { diff --git a/tests/specs/check/ts_in_npm_pkg/__test__.jsonc b/tests/specs/check/ts_in_npm_pkg/__test__.jsonc new file mode 100644 index 00000000000000..7a5a4fb74a3dc1 --- /dev/null +++ b/tests/specs/check/ts_in_npm_pkg/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "args": "check --all main.ts", + "output": "check.out", + "exitCode": 1 +} diff --git a/tests/specs/check/ts_in_npm_pkg/check.out b/tests/specs/check/ts_in_npm_pkg/check.out new file mode 100644 index 00000000000000..90c15dfe330fbc --- /dev/null +++ b/tests/specs/check/ts_in_npm_pkg/check.out @@ -0,0 +1,7 @@ +Check file:///[WILDLINE]/main.ts +TS2322 [ERROR]: Type 'string' is not assignable to type 'Test'. +const test: Test = ""; + ~~~~ + at file:///[WILDLINE]/main.ts:3:7 + +error: Type checking failed. diff --git a/tests/specs/check/ts_in_npm_pkg/main.ts b/tests/specs/check/ts_in_npm_pkg/main.ts new file mode 100644 index 00000000000000..8c47064125ad5d --- /dev/null +++ b/tests/specs/check/ts_in_npm_pkg/main.ts @@ -0,0 +1,3 @@ +import type { Test } from "package"; + +const test: Test = ""; diff --git a/tests/specs/check/ts_in_npm_pkg/node_modules/package/main.ts b/tests/specs/check/ts_in_npm_pkg/node_modules/package/main.ts new file mode 100644 index 00000000000000..c587c7fa92c601 --- /dev/null +++ b/tests/specs/check/ts_in_npm_pkg/node_modules/package/main.ts @@ -0,0 +1 @@ +export * from "./other" diff --git a/tests/specs/check/ts_in_npm_pkg/node_modules/package/other.ts b/tests/specs/check/ts_in_npm_pkg/node_modules/package/other.ts new file mode 100644 index 00000000000000..2916d0bad7205f --- /dev/null +++ b/tests/specs/check/ts_in_npm_pkg/node_modules/package/other.ts @@ -0,0 +1,3 @@ +export class Test { + prop = "" +} diff --git a/tests/specs/check/ts_in_npm_pkg/node_modules/package/package.json b/tests/specs/check/ts_in_npm_pkg/node_modules/package/package.json new file mode 100644 index 00000000000000..4511be0d3177c7 --- /dev/null +++ b/tests/specs/check/ts_in_npm_pkg/node_modules/package/package.json @@ -0,0 +1,7 @@ +{ + "exports": { + "default": { + "types": "./main.ts" + } + } +} diff --git a/tests/specs/check/ts_in_npm_pkg/package.json b/tests/specs/check/ts_in_npm_pkg/package.json new file mode 100644 index 00000000000000..2c63c0851048d8 --- /dev/null +++ b/tests/specs/check/ts_in_npm_pkg/package.json @@ -0,0 +1,2 @@ +{ +}