fix(npm): using types for packages with subpath (#16656)

For CommonJS packages we were not trying different extensions for files
specified as subpath of the package ([package_name]/[subpath]). 
This commit fixes that.
This commit is contained in:
Bartek Iwańczuk 2022-11-16 20:41:27 +01:00 committed by GitHub
parent bd159b8bad
commit 1416713cb3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 137 additions and 31 deletions

View File

@ -25,6 +25,7 @@ use deno_runtime::deno_node::legacy_main_resolve;
use deno_runtime::deno_node::package_exports_resolve;
use deno_runtime::deno_node::package_imports_resolve;
use deno_runtime::deno_node::package_resolve;
use deno_runtime::deno_node::path_to_declaration_path;
use deno_runtime::deno_node::NodeModuleKind;
use deno_runtime::deno_node::PackageJson;
use deno_runtime::deno_node::PathClean;
@ -548,34 +549,6 @@ fn mode_conditions(mode: NodeResolutionMode) -> &'static [&'static str] {
}
}
/// Checks if the resolved file has a corresponding declaration file.
fn path_to_declaration_path(
path: PathBuf,
referrer_kind: NodeModuleKind,
) -> PathBuf {
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.ts")
{
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
}
}
}
pub fn node_resolve_binary_export(
pkg_req: &NpmPackageReq,
bin_name: Option<&str>,

View File

@ -266,6 +266,14 @@ itest!(check_local {
exit_code: 1,
});
itest!(types {
args: "check --quiet npm/types/main.ts",
output: "npm/types/main.out",
envs: env_vars(),
http_server: true,
exit_code: 1,
});
itest!(types_ambient_module {
args: "check --quiet npm/types_ambient_module/main.ts",
output: "npm/types_ambient_module/main.out",

View File

@ -0,0 +1,4 @@
export interface Fizzbuzz {
fizz: string;
buzz: string;
}

View File

@ -0,0 +1,5 @@
{
"name": "@denotest/types-ambient",
"version": "1.0.0",
"types": "./index.d.ts"
}

View File

@ -0,0 +1,4 @@
export interface SomeInterface {
prop: string;
prop2: number;
}

View File

@ -0,0 +1,5 @@
{
"name": "@denotest/types-ambient",
"version": "1.0.0",
"types": "./index.d.ts"
}

View File

@ -0,0 +1,4 @@
export interface Foobar {
foo: string;
bar: string;
}

51
cli/tests/testdata/npm/types/main.out vendored Normal file
View File

@ -0,0 +1,51 @@
error: TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
bar: 1,
~~~
at [WILDCARD]/npm/types/main.ts:7:3
The expected type comes from property 'bar' which is declared here on type 'Foobar'
bar: string;
~~~
at [WILDCARD]/@denotest/types_imported/1.0.0/subpath.d.ts:3:5
TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
prop: 1,
~~~~
at [WILDCARD]/npm/types/main.ts:11:3
The expected type comes from property 'prop' which is declared here on type 'SomeInterface'
prop: string;
~~~~
at [WILDCARD]/@denotest/types_imported/1.0.0/index.d.ts:2:5
TS2322 [ERROR]: Type 'string' is not assignable to type 'number'.
prop2: "asdf",
~~~~~
at [WILDCARD]/npm/types/main.ts:12:3
The expected type comes from property 'prop2' which is declared here on type 'SomeInterface'
prop2: number;
~~~~~
at [WILDCARD]/@denotest/types_imported/1.0.0/index.d.ts:3:5
TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
fizz: 1,
~~~~
at [WILDCARD]/npm/types/main.ts:16:3
The expected type comes from property 'fizz' which is declared here on type 'Fizzbuzz'
fizz: string;
~~~~
at [WILDCARD]/@denotest/types/1.0.0/index.d.ts:2:3
TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
buzz: 2,
~~~~
at [WILDCARD]/npm/types/main.ts:17:3
The expected type comes from property 'buzz' which is declared here on type 'Fizzbuzz'
buzz: string;
~~~~
at [WILDCARD]/@denotest/types/1.0.0/index.d.ts:3:3
Found 5 errors.

18
cli/tests/testdata/npm/types/main.ts vendored Normal file
View File

@ -0,0 +1,18 @@
import type { Fizzbuzz } from "npm:@denotest/types";
import type { SomeInterface } from "npm:@denotest/types_imported";
import type { Foobar as FooInterface } from "npm:@denotest/types_imported/subpath";
const foobar: FooInterface = {
foo: "foo",
bar: 1,
};
const i: SomeInterface = {
prop: 1,
prop2: "asdf",
};
const fizzbuzz: Fizzbuzz = {
fizz: 1,
buzz: 2,
};

View File

@ -27,6 +27,7 @@ pub use resolution::legacy_main_resolve;
pub use resolution::package_exports_resolve;
pub use resolution::package_imports_resolve;
pub use resolution::package_resolve;
pub use resolution::path_to_declaration_path;
pub use resolution::NodeModuleKind;
pub use resolution::DEFAULT_CONDITIONS;
pub use resolution::TYPES_CONDITIONS;

View File

@ -27,6 +27,34 @@ pub enum NodeModuleKind {
Cjs,
}
/// Checks if the resolved file has a corresponding declaration file.
pub fn path_to_declaration_path(
path: PathBuf,
referrer_kind: NodeModuleKind,
) -> PathBuf {
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.ts")
{
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
}
}
}
fn to_specifier_display_string(url: &ModuleSpecifier) -> String {
if let Ok(path) = url.to_file_path() {
path.display().to_string()
@ -659,9 +687,14 @@ pub fn package_resolve(
return legacy_main_resolve(&package_json, referrer_kind, conditions);
}
Ok(Some(
package_json.path.parent().unwrap().join(&package_subpath),
))
let file_path = package_json.path.parent().unwrap().join(&package_subpath);
if conditions == TYPES_CONDITIONS {
let declaration_path = path_to_declaration_path(file_path, referrer_kind);
Ok(Some(declaration_path))
} else {
Ok(Some(file_path))
}
}
pub fn get_package_scope_config(