1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-28 16:20:57 -05:00

fix(npm): don't resolve JS files when resolving types (#16854)

Closes #16851
This commit is contained in:
David Sherret 2022-11-28 17:48:56 -05:00 committed by Yoshiya Hinosawa
parent 998e5cf171
commit 77c1608213
No known key found for this signature in database
GPG key ID: 0E8BFAA8A5B4E92B
6 changed files with 83 additions and 49 deletions

View file

@ -492,7 +492,10 @@ pub fn node_resolve(
let path = url.to_file_path().unwrap(); let path = url.to_file_path().unwrap();
// todo(16370): the module kind is not correct here. I think we need // todo(16370): the module kind is not correct here. I think we need
// typescript to tell us if the referrer is esm or cjs // typescript to tell us if the referrer is esm or cjs
let path = path_to_declaration_path(path, NodeModuleKind::Esm); let path = match path_to_declaration_path(path, NodeModuleKind::Esm) {
Some(path) => path,
None => return Ok(None),
};
ModuleSpecifier::from_file_path(path).unwrap() ModuleSpecifier::from_file_path(path).unwrap()
} }
}; };
@ -532,7 +535,10 @@ pub fn node_resolve_npm_reference(
let resolved_path = match mode { let resolved_path = match mode {
NodeResolutionMode::Execution => resolved_path, NodeResolutionMode::Execution => resolved_path,
NodeResolutionMode::Types => { NodeResolutionMode::Types => {
path_to_declaration_path(resolved_path, node_module_kind) match path_to_declaration_path(resolved_path, node_module_kind) {
Some(path) => path,
None => return Ok(None),
}
} }
}; };
let url = ModuleSpecifier::from_file_path(resolved_path).unwrap(); let url = ModuleSpecifier::from_file_path(resolved_path).unwrap();
@ -794,7 +800,9 @@ fn module_resolve(
// should use the value provided by typescript instead // should use the value provided by typescript instead
let declaration_path = let declaration_path =
path_to_declaration_path(file_path, NodeModuleKind::Esm); path_to_declaration_path(file_path, NodeModuleKind::Esm);
Some(ModuleSpecifier::from_file_path(declaration_path).unwrap()) declaration_path.map(|declaration_path| {
ModuleSpecifier::from_file_path(declaration_path).unwrap()
})
} else { } else {
Some(resolved_specifier) Some(resolved_specifier)
} }

View file

@ -190,19 +190,19 @@ mod npm {
}); });
itest!(import_map { itest!(import_map {
args: "run --allow-read --allow-env --import-map npm/import_map/import_map.json npm/import_map/main.js", args: "run --allow-read --allow-env --import-map npm/import_map/import_map.json npm/import_map/main.js",
output: "npm/import_map/main.out", output: "npm/import_map/main.out",
envs: env_vars_for_npm_tests(), envs: env_vars_for_npm_tests(),
http_server: true, http_server: true,
}); });
itest!(lock_file { itest!(lock_file {
args: "run --allow-read --allow-env --lock npm/lock_file/lock.json npm/lock_file/main.js", args: "run --allow-read --allow-env --lock npm/lock_file/lock.json npm/lock_file/main.js",
output: "npm/lock_file/main.out", output: "npm/lock_file/main.out",
envs: env_vars_for_npm_tests(), envs: env_vars_for_npm_tests(),
http_server: true, http_server: true,
exit_code: 10, exit_code: 10,
}); });
itest!(sub_paths { itest!(sub_paths {
args: "run -A --quiet npm/sub_paths/main.jsx", args: "run -A --quiet npm/sub_paths/main.jsx",
@ -296,12 +296,20 @@ mod npm {
}); });
itest!(types_ambient_module_import_map { itest!(types_ambient_module_import_map {
args: "check --quiet --import-map=npm/types_ambient_module/import_map.json npm/types_ambient_module/main_import_map.ts", args: "check --quiet --import-map=npm/types_ambient_module/import_map.json npm/types_ambient_module/main_import_map.ts",
output: "npm/types_ambient_module/main_import_map.out", output: "npm/types_ambient_module/main_import_map.out",
envs: env_vars_for_npm_tests(), envs: env_vars_for_npm_tests(),
http_server: true, http_server: true,
exit_code: 1, exit_code: 1,
}); });
itest!(no_types_cjs {
args: "check --quiet npm/no_types_cjs/main.ts",
output_str: Some(""),
exit_code: 0,
envs: env_vars_for_npm_tests(),
http_server: true,
});
itest!(no_types_in_conditional_exports { itest!(no_types_in_conditional_exports {
args: "run --check --unstable npm/no_types_in_conditional_exports/main.ts", args: "run --check --unstable npm/no_types_in_conditional_exports/main.ts",
@ -791,20 +799,20 @@ mod npm {
} }
itest!(compile_errors { itest!(compile_errors {
args: "compile -A --quiet npm/cached_only/main.ts", args: "compile -A --quiet npm/cached_only/main.ts",
output_str: Some("error: npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: npm:chalk@5\n"), output_str: Some("error: npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: npm:chalk@5\n"),
exit_code: 1, exit_code: 1,
envs: env_vars_for_npm_tests(), envs: env_vars_for_npm_tests(),
http_server: true, http_server: true,
}); });
itest!(bundle_errors { itest!(bundle_errors {
args: "bundle --quiet npm/esm/main.js", args: "bundle --quiet npm/esm/main.js",
output_str: Some("error: npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: npm:chalk@5\n"), output_str: Some("error: npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: npm:chalk@5\n"),
exit_code: 1, exit_code: 1,
envs: env_vars_for_npm_tests(), envs: env_vars_for_npm_tests(),
http_server: true, http_server: true,
}); });
itest!(info_chalk_display { itest!(info_chalk_display {
args: "info --quiet npm/cjs_with_deps/main.js", args: "info --quiet npm/cjs_with_deps/main.js",
@ -832,14 +840,14 @@ mod npm {
}); });
itest!(info_chalk_json_node_modules_dir { itest!(info_chalk_json_node_modules_dir {
args: args:
"info --quiet --node-modules-dir --json $TESTDATA/npm/cjs_with_deps/main.js", "info --quiet --node-modules-dir --json $TESTDATA/npm/cjs_with_deps/main.js",
output: "npm/cjs_with_deps/main_info_json.out", output: "npm/cjs_with_deps/main_info_json.out",
exit_code: 0, exit_code: 0,
envs: env_vars_for_npm_tests(), envs: env_vars_for_npm_tests(),
http_server: true, http_server: true,
temp_cwd: true, temp_cwd: true,
}); });
itest!(info_cli_chalk_display { itest!(info_cli_chalk_display {
args: "info --quiet npm:chalk@4", args: "info --quiet npm:chalk@4",

View file

@ -0,0 +1,7 @@
import mod from "npm:@denotest/no-types-cjs";
// it actually returns a `number` and has that in its
// jsdocs, but the jsdocs should not have been resolved so
// this should type check just fine
const value: string = mod();
console.log(value);

View file

@ -0,0 +1,6 @@
/**
* @return {number}
*/
module.exports = function () {
return 5;
};

View file

@ -0,0 +1,5 @@
{
"name": "@denotest/no-types-cjs",
"version": "1.0.0",
"main": "./main.js"
}

View file

@ -31,7 +31,7 @@ pub enum NodeModuleKind {
pub fn path_to_declaration_path( pub fn path_to_declaration_path(
path: PathBuf, path: PathBuf,
referrer_kind: NodeModuleKind, referrer_kind: NodeModuleKind,
) -> PathBuf { ) -> Option<PathBuf> {
fn probe_extensions( fn probe_extensions(
path: &Path, path: &Path,
referrer_kind: NodeModuleKind, referrer_kind: NodeModuleKind,
@ -56,17 +56,17 @@ pub fn path_to_declaration_path(
|| lowercase_path.ends_with(".d.cts") || lowercase_path.ends_with(".d.cts")
|| lowercase_path.ends_with(".d.ts") || lowercase_path.ends_with(".d.ts")
{ {
return path; return Some(path);
} }
if let Some(path) = probe_extensions(&path, referrer_kind) { if let Some(path) = probe_extensions(&path, referrer_kind) {
return path; return Some(path);
} }
if path.is_dir() { if path.is_dir() {
if let Some(path) = probe_extensions(&path.join("index"), referrer_kind) { if let Some(path) = probe_extensions(&path.join("index"), referrer_kind) {
return path; return Some(path);
} }
} }
path None
} }
/// Alternate `PathBuf::with_extension` that will handle known extensions /// Alternate `PathBuf::with_extension` that will handle known extensions
@ -743,8 +743,9 @@ pub fn package_resolve(
let file_path = package_json.path.parent().unwrap().join(&package_subpath); let file_path = package_json.path.parent().unwrap().join(&package_subpath);
if conditions == TYPES_CONDITIONS { if conditions == TYPES_CONDITIONS {
let declaration_path = path_to_declaration_path(file_path, referrer_kind); let maybe_declaration_path =
Ok(Some(declaration_path)) path_to_declaration_path(file_path, referrer_kind);
Ok(maybe_declaration_path)
} else { } else {
Ok(Some(file_path)) Ok(Some(file_path))
} }
@ -813,8 +814,7 @@ pub fn legacy_main_resolve(
// a corresponding declaration file // a corresponding declaration file
if let Some(main) = package_json.main(referrer_kind) { if let Some(main) = package_json.main(referrer_kind) {
let main = package_json.path.parent().unwrap().join(main).clean(); let main = package_json.path.parent().unwrap().join(main).clean();
let path = path_to_declaration_path(main, referrer_kind); if let Some(path) = path_to_declaration_path(main, referrer_kind) {
if path.exists() {
return Ok(Some(path)); return Ok(Some(path));
} }
} }