diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 65ce330dfc..853708221f 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -353,7 +353,12 @@ impl<'a> TsResponseImportMapper<'a> { let pkg_reqs = npm_resolver.resolve_pkg_reqs_from_pkg_id(&pkg_id); // check if any pkg reqs match what is found in an import map if !pkg_reqs.is_empty() { - let sub_path = self.resolve_package_path(specifier); + let sub_path = npm_resolver + .resolve_pkg_folder_from_pkg_id(&pkg_id) + .ok() + .and_then(|pkg_folder| { + self.resolve_package_path(specifier, &pkg_folder) + }); if let Some(import_map) = self.maybe_import_map { let pkg_reqs = pkg_reqs.iter().collect::>(); let mut matches = Vec::new(); @@ -368,8 +373,13 @@ impl<'a> TsResponseImportMapper<'a> { if let Some(key_sub_path) = sub_path.strip_prefix(value_sub_path) { - matches - .push(format!("{}{}", entry.raw_key, key_sub_path)); + // keys that don't end in a slash can't be mapped to a subpath + if entry.raw_key.ends_with('/') + || key_sub_path.is_empty() + { + matches + .push(format!("{}{}", entry.raw_key, key_sub_path)); + } } } } @@ -413,10 +423,16 @@ impl<'a> TsResponseImportMapper<'a> { fn resolve_package_path( &self, specifier: &ModuleSpecifier, + package_root_folder: &Path, ) -> Option { let package_json = self .resolver - .get_closest_package_json(specifier) + .pkg_json_resolver(specifier) + // the specifier might have a closer package.json, but we + // want the root of the package's package.json + .get_closest_package_json_from_file_path( + &package_root_folder.join("package.json"), + ) .ok() .flatten()?; let root_folder = package_json.path.parent()?; diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index c705511f30..2434501a1b 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -20,14 +20,12 @@ use deno_resolver::DenoResolverOptions; use deno_resolver::NodeAndNpmReqResolver; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolver; -use deno_runtime::deno_node::PackageJson; use deno_runtime::deno_node::PackageJsonResolver; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; use indexmap::IndexMap; -use node_resolver::errors::ClosestPkgJsonError; use node_resolver::InNpmPackageChecker; use node_resolver::NodeResolutionKind; use node_resolver::ResolutionMode; @@ -380,6 +378,14 @@ impl LspResolver { resolver.npm_resolver.as_ref().and_then(|r| r.as_managed()) } + pub fn pkg_json_resolver( + &self, + referrer: &ModuleSpecifier, + ) -> &Arc { + let resolver = self.get_scope_resolver(Some(referrer)); + &resolver.pkg_json_resolver + } + pub fn graph_imports_by_referrer( &self, file_referrer: &ModuleSpecifier, @@ -522,16 +528,6 @@ impl LspResolver { .is_some() } - pub fn get_closest_package_json( - &self, - referrer: &ModuleSpecifier, - ) -> Result>, ClosestPkgJsonError> { - let resolver = self.get_scope_resolver(Some(referrer)); - resolver - .pkg_json_resolver - .get_closest_package_json(referrer) - } - pub fn resolve_redirects( &self, specifier: &ModuleSpecifier, diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs index 0c279d9e12..d383a5413f 100644 --- a/cli/npm/managed/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -236,8 +236,21 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver { else { return Ok(None); }; - let folder_name = folder_path.parent().unwrap().to_string_lossy(); - Ok(get_package_folder_id_from_folder_name(&folder_name)) + // ex. project/node_modules/.deno/preact@10.24.3/node_modules/preact/ + let Some(node_modules_ancestor) = folder_path + .ancestors() + .find(|ancestor| ancestor.ends_with("node_modules")) + else { + return Ok(None); + }; + let Some(folder_name) = + node_modules_ancestor.parent().and_then(|p| p.file_name()) + else { + return Ok(None); + }; + Ok(get_package_folder_id_from_folder_name( + &folder_name.to_string_lossy(), + )) } async fn cache_packages(&self) -> Result<(), AnyError> { diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs index 1c204f54e8..ddcdec0bbd 100644 --- a/ext/node/ops/require.rs +++ b/ext/node/ops/require.rs @@ -429,7 +429,9 @@ where let pkg_json_resolver = state.borrow::(); let pkg = pkg_json_resolver - .get_closest_package_json_from_path(&PathBuf::from(parent_path.unwrap())) + .get_closest_package_json_from_file_path(&PathBuf::from( + parent_path.unwrap(), + )) .ok() .flatten(); if pkg.is_none() { @@ -620,8 +622,8 @@ where let referrer_path = ensure_read_permission::

(state, &referrer_path) .map_err(RequireErrorKind::Permission)?; let pkg_json_resolver = state.borrow::(); - let Some(pkg) = - pkg_json_resolver.get_closest_package_json_from_path(&referrer_path)? + let Some(pkg) = pkg_json_resolver + .get_closest_package_json_from_file_path(&referrer_path)? else { return Ok(None); }; diff --git a/resolvers/node/package_json.rs b/resolvers/node/package_json.rs index ae016ebe3e..e3793af84a 100644 --- a/resolvers/node/package_json.rs +++ b/resolvers/node/package_json.rs @@ -60,10 +60,10 @@ impl PackageJsonResolver { let Ok(file_path) = deno_path_util::url_to_file_path(url) else { return Ok(None); }; - self.get_closest_package_json_from_path(&file_path) + self.get_closest_package_json_from_file_path(&file_path) } - pub fn get_closest_package_json_from_path( + pub fn get_closest_package_json_from_file_path( &self, file_path: &Path, ) -> Result, ClosestPkgJsonError> { diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 296da75315..decc635ffa 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -9610,6 +9610,69 @@ fn lsp_completions_npm() { client.shutdown(); } +#[test] +fn lsp_auto_imports_npm_auto() { + let context = TestContextBuilder::for_npm().use_temp_cwd().build(); + let temp_dir_path = context.temp_dir().path(); + temp_dir_path.join("deno.json").write_json(&json!({ + "nodeModulesDir": "auto", + "imports": { + "preact": "npm:preact@^10.19.6", + }, + })); + // add a file that uses the import so that typescript knows about it + temp_dir_path + .join("mod.ts") + .write("import { useEffect } from 'preact/hooks'; console.log(useEffect);"); + context.run_deno("cache mod.ts"); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + let file_uri = temp_dir_path.join("file.ts").url_file(); + let mut diagnostics = client + .did_open(json!({ + "textDocument": { + "uri": file_uri, + "languageId": "typescript", + "version": 1, + "text": "useEffect", + } + })) + .all(); + assert_eq!(diagnostics.len(), 1); + let diagnostic = diagnostics.remove(0); + let res = client.write_request( + "textDocument/codeAction", + json!({ + "textDocument": { + "uri": file_uri, + }, + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 0, "character": 9 } + }, + "context": { + "diagnostics": [diagnostic], + "only": ["quickfix"] + } + }), + ); + assert_eq!( + res + .as_array() + .unwrap() + .first() + .unwrap() + .as_object() + .unwrap() + .get("title") + .unwrap() + .as_str() + .unwrap(), + "Add import from \"preact/hooks\"" + ); + client.shutdown(); +} + #[test] fn lsp_npm_specifier_unopened_file() { let context = TestContextBuilder::new() diff --git a/tests/util/server/src/builders.rs b/tests/util/server/src/builders.rs index 4a1510ce4c..1cc1af2812 100644 --- a/tests/util/server/src/builders.rs +++ b/tests/util/server/src/builders.rs @@ -325,6 +325,15 @@ impl TestContext { builder } + pub fn run_deno(&self, args: impl AsRef) { + self + .new_command() + .name("deno") + .args(args) + .run() + .skip_output_check(); + } + pub fn run_npm(&self, args: impl AsRef) { self .new_command()