fix(npm): allow resolving from package.json when an import map exists (#17905)

cli/lsp/language_server.rs

@@ -811,7 +811,6 @@ impl Inner {
 
   fn update_config_file(&mut self) -> Result<(), AnyError> {
     self.maybe_config_file = None;
-    self.maybe_package_json = None;
     self.fmt_options = Default::default();
     self.lint_options = Default::default();
 
@@ -1205,7 +1204,7 @@ impl Inner {
       .map(|f| self.url_map.normalize_url(&f.uri))
       .collect();
 
-    // if the current tsconfig has changed, we need to reload it
+    // if the current deno.json has changed, we need to reload it
     if let Some(config_file) = &self.maybe_config_file {
       if changes.contains(&config_file.specifier) {
         if let Err(err) = self.update_config_file() {
@@ -1218,17 +1217,18 @@ impl Inner {
       }
     }
     if let Some(package_json) = &self.maybe_package_json {
-      if changes.contains(&package_json.specifier()) {
+      // always update the package json if the deno config changes
+      if touched || changes.contains(&package_json.specifier()) {
         if let Err(err) = self.update_package_json() {
           self.client.show_message(MessageType::WARNING, err).await;
         }
         touched = true;
       }
     }
-    // if the current import map, or config file has changed, we need to reload
+    // if the current import map, or config file has changed, we need to
     // reload the import map
     if let Some(import_map_uri) = &self.maybe_import_map_uri {
-      if changes.contains(import_map_uri) || touched {
+      if touched || changes.contains(import_map_uri) {
         if let Err(err) = self.update_import_map().await {
           self.client.show_message(MessageType::WARNING, err).await;
         }

cli/resolver.rs

@@ -113,24 +113,32 @@ impl Resolver for CliGraphResolver {
     specifier: &str,
     referrer: &ModuleSpecifier,
   ) -> Result<ModuleSpecifier, AnyError> {
-    if let Some(import_map) = &self.maybe_import_map {
+    // attempt to resolve with the import map first
-      return import_map
+    let maybe_import_map_err = match self
-        .resolve(specifier, referrer)
+      .maybe_import_map
-        .map_err(|err| err.into());
+      .as_ref()
-    }
+      .map(|import_map| import_map.resolve(specifier, referrer))
+    {
+      Some(Ok(value)) => return Ok(value),
+      Some(Err(err)) => Some(err),
+      None => None,
+    };
 
+    // then with package.json
     if let Some(deps) = self.maybe_package_json_deps.as_ref() {
       if let Some(specifier) = resolve_package_json_dep(specifier, deps)? {
         return Ok(specifier);
       }
-      if let Some(req) = deps.get(specifier) {
-        return Ok(ModuleSpecifier::parse(&format!("npm:{req}")).unwrap());
-      }
     }
 
+    // otherwise, surface the import map error or try resolving when has no import map
+    if let Some(err) = maybe_import_map_err {
+      Err(err.into())
+    } else {
       deno_graph::resolve_import(specifier, referrer).map_err(|err| err.into())
     }
   }
+}
 
 fn resolve_package_json_dep(
   specifier: &str,

cli/tests/integration/check_tests.rs

@@ -251,3 +251,13 @@ itest!(package_json_fail_check {
   copy_temp_dir: Some("package_json/basic"),
   exit_code: 1,
 });
+
+itest!(package_json_with_deno_json {
+  args: "check --quiet main.ts",
+  output: "package_json/deno_json/main.check.out",
+  cwd: Some("package_json/deno_json/"),
+  copy_temp_dir: Some("package_json/deno_json/"),
+  envs: env_vars_for_npm_tests_no_sync_download(),
+  http_server: true,
+  exit_code: 1,
+});

cli/tests/integration/run_tests.rs

@@ -2793,6 +2793,15 @@ itest!(package_json_auto_discovered_for_npm_binary {
   http_server: true,
 });
 
+itest!(package_json_with_deno_json {
+  args: "run --quiet -A main.ts",
+  output: "package_json/deno_json/main.out",
+  cwd: Some("package_json/deno_json/"),
+  copy_temp_dir: Some("package_json/deno_json/"),
+  envs: env_vars_for_npm_tests_no_sync_download(),
+  http_server: true,
+});
+
 itest!(wasm_streaming_panic_test {
   args: "run run/wasm_streaming_panic_test.js",
   output: "run/wasm_streaming_panic_test.js.out",

cli/tests/testdata/package_json/deno_json/deno.json

@@ -0,0 +1,5 @@
+{
+  "imports": {
+    "other": "./other.ts"
+  }
+}

cli/tests/testdata/package_json/deno_json/main.check.out

@@ -0,0 +1,11 @@
+error: TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
+const _strValue1: string = NUMBER_VALUE;
+      ~~~~~~~~~~
+    at file:///[WILDCARD]/main.ts:8:7
+
+TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
+const _strValue2: string = test.getValue();
+      ~~~~~~~~~~
+    at file:///[WILDCARD]/main.ts:9:7
+
+Found 2 errors.

cli/tests/testdata/package_json/deno_json/main.out

@@ -0,0 +1,2 @@
+1
+2

cli/tests/testdata/package_json/deno_json/main.ts

@@ -0,0 +1,9 @@
+import { NUMBER_VALUE } from "other";
+import * as test from "@denotest/esm-basic";
+
+test.setValue(2);
+console.log(test.getValue());
+
+// these should cause type errors
+const _strValue1: string = NUMBER_VALUE;
+const _strValue2: string = test.getValue();

cli/tests/testdata/package_json/deno_json/other.ts

@@ -0,0 +1,3 @@
+console.log(1);
+
+export const NUMBER_VALUE = 1;

cli/tests/testdata/package_json/deno_json/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "@denotest/esm-basic": "*"
+  }
+}