summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Sherret <dsherret@users.noreply.github.com>2023-02-23 17:20:23 -0500
committerGitHub <noreply@github.com>2023-02-23 23:20:23 +0100
commite57b38f8b216bc8c0c1c22c10ab5c9ce2560c58a (patch)
treea83e2dfd1c2b0af6bfbd68e3e54b0078a1915366
parentda781280b8422b4116473b366fb7d207909a31da (diff)
fix(npm): allow resolving from package.json when an import map exists (#17905)
-rw-r--r--cli/lsp/language_server.rs10
-rw-r--r--cli/resolver.rs26
-rw-r--r--cli/tests/integration/check_tests.rs10
-rw-r--r--cli/tests/integration/run_tests.rs9
-rw-r--r--cli/tests/testdata/package_json/deno_json/deno.json5
-rw-r--r--cli/tests/testdata/package_json/deno_json/main.check.out11
-rw-r--r--cli/tests/testdata/package_json/deno_json/main.out2
-rw-r--r--cli/tests/testdata/package_json/deno_json/main.ts9
-rw-r--r--cli/tests/testdata/package_json/deno_json/other.ts3
-rw-r--r--cli/tests/testdata/package_json/deno_json/package.json5
10 files changed, 76 insertions, 14 deletions
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index dfbb9c2e1..ee91e045b 100644
--- a/cli/lsp/language_server.rs
+++ b/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;
}
diff --git a/cli/resolver.rs b/cli/resolver.rs
index db6ef0c8e..bcc1c8da8 100644
--- a/cli/resolver.rs
+++ b/cli/resolver.rs
@@ -113,22 +113,30 @@ impl Resolver for CliGraphResolver {
specifier: &str,
referrer: &ModuleSpecifier,
) -> Result<ModuleSpecifier, AnyError> {
- if let Some(import_map) = &self.maybe_import_map {
- return import_map
- .resolve(specifier, referrer)
- .map_err(|err| err.into());
- }
+ // attempt to resolve with the import map first
+ let maybe_import_map_err = match self
+ .maybe_import_map
+ .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());
- }
}
- deno_graph::resolve_import(specifier, referrer).map_err(|err| err.into())
+ // 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())
+ }
}
}
diff --git a/cli/tests/integration/check_tests.rs b/cli/tests/integration/check_tests.rs
index 021a536c4..1273fbdce 100644
--- a/cli/tests/integration/check_tests.rs
+++ b/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,
+});
diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs
index d89142c21..4b7869ef6 100644
--- a/cli/tests/integration/run_tests.rs
+++ b/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",
diff --git a/cli/tests/testdata/package_json/deno_json/deno.json b/cli/tests/testdata/package_json/deno_json/deno.json
new file mode 100644
index 000000000..8a89da280
--- /dev/null
+++ b/cli/tests/testdata/package_json/deno_json/deno.json
@@ -0,0 +1,5 @@
+{
+ "imports": {
+ "other": "./other.ts"
+ }
+}
diff --git a/cli/tests/testdata/package_json/deno_json/main.check.out b/cli/tests/testdata/package_json/deno_json/main.check.out
new file mode 100644
index 000000000..53b6869c0
--- /dev/null
+++ b/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.
diff --git a/cli/tests/testdata/package_json/deno_json/main.out b/cli/tests/testdata/package_json/deno_json/main.out
new file mode 100644
index 000000000..1191247b6
--- /dev/null
+++ b/cli/tests/testdata/package_json/deno_json/main.out
@@ -0,0 +1,2 @@
+1
+2
diff --git a/cli/tests/testdata/package_json/deno_json/main.ts b/cli/tests/testdata/package_json/deno_json/main.ts
new file mode 100644
index 000000000..7768ff3fc
--- /dev/null
+++ b/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();
diff --git a/cli/tests/testdata/package_json/deno_json/other.ts b/cli/tests/testdata/package_json/deno_json/other.ts
new file mode 100644
index 000000000..997d84adf
--- /dev/null
+++ b/cli/tests/testdata/package_json/deno_json/other.ts
@@ -0,0 +1,3 @@
+console.log(1);
+
+export const NUMBER_VALUE = 1;
diff --git a/cli/tests/testdata/package_json/deno_json/package.json b/cli/tests/testdata/package_json/deno_json/package.json
new file mode 100644
index 000000000..54ca824d6
--- /dev/null
+++ b/cli/tests/testdata/package_json/deno_json/package.json
@@ -0,0 +1,5 @@
+{
+ "dependencies": {
+ "@denotest/esm-basic": "*"
+ }
+}