From a5111fbc4d7daaa9056d0e4b21b3d54a009a7f99 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 21 May 2024 17:54:15 +0100 Subject: [PATCH] fix(cli): use CliNodeResolver::resolve() for managed node_modules (#23902) --- cli/lsp/documents.rs | 7 +-- cli/lsp/resolver.rs | 3 +- cli/resolver.rs | 12 +++++ tests/integration/lsp_tests.rs | 49 +++++++++++++++++++ .../types-nested-js-dts/1.0.0/import.d.mts | 1 + .../types-nested-js-dts/1.0.0/import.mjs | 0 .../types-nested-js-dts/1.0.0/index.d.mts | 1 + .../types-nested-js-dts/1.0.0/package.json | 5 ++ 8 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/import.d.mts create mode 100644 tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/import.mjs create mode 100644 tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/index.d.mts create mode 100644 tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/package.json diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 8a6e8b65b5..df15bff2ae 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -29,7 +29,6 @@ use deno_core::ModuleSpecifier; use deno_graph::source::ResolutionMode; use deno_graph::Resolution; use deno_runtime::deno_node; -use deno_runtime::deno_node::NodeResolutionMode; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; @@ -1295,11 +1294,7 @@ impl Documents { } if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(specifier) { - return self.resolver.npm_to_file_url( - &npm_ref, - referrer, - NodeResolutionMode::Types, - ); + return self.resolver.npm_to_file_url(&npm_ref, referrer); } let Some(doc) = self.get(specifier) else { return Some((specifier.clone(), MediaType::from_specifier(specifier))); diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 0f515060a0..27c4f6acf7 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -229,7 +229,6 @@ impl LspResolver { &self, req_ref: &NpmPackageReqReference, referrer: &ModuleSpecifier, - mode: NodeResolutionMode, ) -> Option<(ModuleSpecifier, MediaType)> { let node_resolver = self.node_resolver.as_ref()?; Some(NodeResolution::into_specifier_and_media_type( @@ -238,7 +237,7 @@ impl LspResolver { req_ref, &PermissionsContainer::allow_all(), referrer, - mode, + NodeResolutionMode::Types, ) .ok(), )) diff --git a/cli/resolver.rs b/cli/resolver.rs index 7e68a62e9b..8fbacd8f17 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -677,6 +677,18 @@ impl Resolver for CliGraphResolver { } } } + } else if referrer.scheme() == "file" { + if let Some(node_resolver) = &self.node_resolver { + let node_result = node_resolver.resolve_if_in_npm_package( + specifier, + referrer, + to_node_mode(mode), + &PermissionsContainer::allow_all(), + ); + if let Some(Ok(Some(res))) = node_result { + return Ok(res.into_url()); + } + } } let specifier = result?; diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 1f7758a9da..3473248996 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -7188,6 +7188,55 @@ fn lsp_npm_completions_auto_import_and_quick_fix_no_import_map() { client.shutdown(); } +// Regression test for https://github.com/denoland/deno/issues/23895. +#[test] +fn lsp_npm_types_nested_js_dts() { + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); + let temp_dir = context.temp_dir(); + let file = source_file( + temp_dir.path().join("file.ts"), + r#" + import { someString } from "npm:@denotest/types-nested-js-dts"; + const someNumber: number = someString; + console.log(someNumber); + "#, + ); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + client.write_request( + "workspace/executeCommand", + json!({ + "command": "deno.cache", + "arguments": [[], file.uri()], + }), + ); + let diagnostics = client.did_open_file(&file); + assert_eq!( + json!(diagnostics.all()), + json!([ + { + "range": { + "start": { + "line": 2, + "character": 12, + }, + "end": { + "line": 2, + "character": 22, + }, + }, + "severity": 1, + "code": 2322, + "source": "deno-ts", + "message": "Type 'string' is not assignable to type 'number'.", + }, + ]) + ); +} + #[test] fn lsp_completions_using_decl() { let context = TestContextBuilder::new().use_temp_cwd().build(); diff --git a/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/import.d.mts b/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/import.d.mts new file mode 100644 index 0000000000..435448424e --- /dev/null +++ b/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/import.d.mts @@ -0,0 +1 @@ +export const someString: string; diff --git a/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/import.mjs b/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/import.mjs new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/index.d.mts b/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/index.d.mts new file mode 100644 index 0000000000..c5ec00e5b7 --- /dev/null +++ b/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/index.d.mts @@ -0,0 +1 @@ +export { someString } from "./import.mjs"; diff --git a/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/package.json b/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/package.json new file mode 100644 index 0000000000..a37f8556cc --- /dev/null +++ b/tests/registry/npm/@denotest/types-nested-js-dts/1.0.0/package.json @@ -0,0 +1,5 @@ +{ + "name": "@denotest/types-nested-js-dts", + "version": "1.0.0", + "types": "./index.d.mts" +}