fix(lsp): ignore code errors when type passes for non-`@deno-types` reolution (#22682)

5 files changed, 63 insertions, 11 deletions

diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs
index 25cfd94e2..6deafde4c 100644
--- a/cli/lsp/diagnostics.rs
+++ b/cli/lsp/diagnostics.rs
@@ -1456,12 +1456,28 @@ fn diagnose_dependency(
     .iter()
     .map(|i| documents::to_lsp_range(&i.range))
     .collect();
+  // TODO(nayeemrmn): This is a crude way of detecting `@deno-types` which has
+  // a different specifier and therefore needs a separate call to
+  // `diagnose_resolution()`. It would be much cleaner if that were modelled as
+  // a separate dependency: https://github.com/denoland/deno_graph/issues/247.
+  let is_types_deno_types = !dependency.maybe_type.is_none()
+    && !dependency
+      .imports
+      .iter()
+      .any(|i| dependency.maybe_type.includes(&i.range.start).is_some());
 
   diagnostics.extend(
     diagnose_resolution(
       snapshot,
       dependency_key,
-      if dependency.maybe_code.is_none() {
+      if dependency.maybe_code.is_none()
+        // If not @deno-types, diagnose the types if the code errored because
+        // it's likely resolving into the node_modules folder, which might be
+        // erroring correctly due to resolution only being for bundlers. Let this
+        // fail at runtime if necesarry, but don't bother erroring in the editor
+        || !is_types_deno_types && matches!(dependency.maybe_type, Resolution::Ok(_))
+          && matches!(dependency.maybe_code, Resolution::Err(_))
+      {
         &dependency.maybe_type
       } else {
         &dependency.maybe_code
@@ -1476,16 +1492,8 @@ fn diagnose_dependency(
         .map(|range| diag.to_lsp_diagnostic(range))
     }),
   );
-  // TODO(nayeemrmn): This is a crude way of detecting `@deno-types` which has
-  // a different specifier and therefore needs a separate call to
-  // `diagnose_resolution()`. It would be much cleaner if that were modelled as
-  // a separate dependency: https://github.com/denoland/deno_graph/issues/247.
-  if !dependency.maybe_type.is_none()
-    && !dependency
-      .imports
-      .iter()
-      .any(|i| dependency.maybe_type.includes(&i.range.start).is_some())
-  {
+
+  if is_types_deno_types {
     let range = match &dependency.maybe_type {
       Resolution::Ok(resolved) => documents::to_lsp_range(&resolved.range),
       Resolution::Err(error) => documents::to_lsp_range(error.range()),
diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs
index 4ea4336a5..0c4c1544f 100644
--- a/tests/integration/lsp_tests.rs
+++ b/tests/integration/lsp_tests.rs
@@ -8660,6 +8660,39 @@ fn lsp_ts_diagnostics_refresh_on_lsp_version_reset() {
 }
 
 #[test]
+fn lsp_diagnostics_none_for_resolving_types() {
+  let context = TestContextBuilder::for_npm().use_temp_cwd().build();
+  context
+    .temp_dir()
+    .write("deno.json", r#"{ "unstable": ["byonm"] }"#);
+  context.temp_dir().write(
+    "package.json",
+    r#"{ "dependencies": { "@denotest/monaco-editor": "*" } }"#,
+  );
+  context.run_npm("install");
+
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  // The types for this package will succeed, but the code will fail
+  // because the package is only made for bundling and not meant to
+  // run in Deno or Node.
+  let diagnostics = client.did_open(json!({
+    "textDocument": {
+      "uri": context.temp_dir().path().join("file.ts").uri_file(),
+      "languageId": "typescript",
+      "version": 1,
+      "text": concat!(
+        "import * as a from \"@denotest/monaco-editor\";\n",
+        "console.log(new a.Editor())\n",
+      )
+    },
+  }));
+  let diagnostics = diagnostics.all();
+  assert!(diagnostics.is_empty(), "{:?}", diagnostics);
+  client.shutdown();
+}
+
+#[test]
 fn lsp_jupyter_diagnostics() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
   let mut client = context.new_lsp_command().build();
diff --git a/tests/testdata/npm/registry/@denotest/monaco-editor/1.0.0/main.js b/tests/testdata/npm/registry/@denotest/monaco-editor/1.0.0/main.js
new file mode 100644
index 000000000..403806c6b
--- /dev/null
+++ b/tests/testdata/npm/registry/@denotest/monaco-editor/1.0.0/main.js
@@ -0,0 +1,4 @@
+// The monaco-editor package uses an entry in the package.json
+// where it has no "type": "module" and then only specifies a
+// "module": "./main.js"-like entry that points at an ESM file.
+export class Editor {}
\ No newline at end of file
diff --git a/tests/testdata/npm/registry/@denotest/monaco-editor/1.0.0/main.types.d.ts b/tests/testdata/npm/registry/@denotest/monaco-editor/1.0.0/main.types.d.ts
new file mode 100644
index 000000000..d978fa159
--- /dev/null
+++ b/tests/testdata/npm/registry/@denotest/monaco-editor/1.0.0/main.types.d.ts
@@ -0,0 +1 @@
+export class Editor {}
diff --git a/tests/testdata/npm/registry/@denotest/monaco-editor/1.0.0/package.json b/tests/testdata/npm/registry/@denotest/monaco-editor/1.0.0/package.json
new file mode 100644
index 000000000..eb0428b49
--- /dev/null
+++ b/tests/testdata/npm/registry/@denotest/monaco-editor/1.0.0/package.json
@@ -0,0 +1,6 @@
+{
+  "name": "@denotest/monaco-editor",
+  "version": "1.0.0",
+  "module": "./main.js",
+  "types": "./main.types.d.ts"
+}