From 9be6709d754bf76301ab488002a474dd22206d4f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 16 Apr 2024 16:46:31 -0400 Subject: [PATCH] feat(check): allow using side effect imports with unknown module kinds (ex. css modules) (#23392) This allows people to use imports like: ```ts import "./app.css"; ``` ...with `deno check` in systems where there's a bundle step (ex. Vite). This will still error when using it with `deno run` or if the referenced file does not exist. See test cases for behaviour. --- cli/factory.rs | 1 + cli/graph_util.rs | 12 +++++++- cli/tsc/mod.rs | 30 +++++++++++++++++-- tests/specs/check/css_import/__test__.jsonc | 24 +++++++++++++++ tests/specs/check/css_import/app.css | 0 tests/specs/check/css_import/exists.out | 1 + tests/specs/check/css_import/exists.ts | 2 ++ .../check/css_import/exists_and_try_uses.out | 5 ++++ .../check/css_import/exists_and_try_uses.ts | 3 ++ .../check/css_import/exists_dynamic_import.ts | 3 ++ .../css_import/exists_run_with_check.out | 3 ++ tests/specs/check/css_import/not_exists.out | 2 ++ tests/specs/check/css_import/not_exists.ts | 1 + 13 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 tests/specs/check/css_import/__test__.jsonc create mode 100644 tests/specs/check/css_import/app.css create mode 100644 tests/specs/check/css_import/exists.out create mode 100644 tests/specs/check/css_import/exists.ts create mode 100644 tests/specs/check/css_import/exists_and_try_uses.out create mode 100644 tests/specs/check/css_import/exists_and_try_uses.ts create mode 100644 tests/specs/check/css_import/exists_dynamic_import.ts create mode 100644 tests/specs/check/css_import/exists_run_with_check.out create mode 100644 tests/specs/check/css_import/not_exists.out create mode 100644 tests/specs/check/css_import/not_exists.ts diff --git a/cli/factory.rs b/cli/factory.rs index 56837110a9a3da..fd33d295c88e56 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -666,6 +666,7 @@ impl CliFactory { // todo(dsherret): ideally the graph container would not be used // for deno cache because it doesn't dynamically load modules DenoSubcommand::Cache(_) => GraphKind::All, + DenoSubcommand::Check(_) => GraphKind::TypesOnly, _ => self.options.type_check_mode().as_graph_kind(), }; Arc::new(ModuleGraphContainer::new(graph_kind)) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 6214a1628d8258..f3b69b243493dd 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -110,13 +110,23 @@ pub fn graph_valid( } } + if graph.graph_kind() == GraphKind::TypesOnly + && matches!( + error, + ModuleGraphError::ModuleError(ModuleError::UnsupportedMediaType(..)) + ) + { + log::debug!("Ignoring: {}", message); + return None; + } + if options.is_vendoring { // warn about failing dynamic imports when vendoring, but don't fail completely if matches!( error, ModuleGraphError::ModuleError(ModuleError::MissingDynamic(_, _)) ) { - log::warn!("Ignoring: {:#}", message); + log::warn!("Ignoring: {}", message); return None; } diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 2d06e0a956b8d9..e2a7da3edc7452 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -508,7 +508,20 @@ fn op_load_inner( } else { &specifier }; - let maybe_source = if let Some(module) = graph.get(specifier) { + let maybe_module = match graph.try_get(specifier) { + Ok(maybe_module) => maybe_module, + Err(err) => match err { + deno_graph::ModuleError::UnsupportedMediaType(_, media_type, _) => { + return Ok(Some(LoadResponse { + data: "".to_string(), + version: Some("1".to_string()), + script_kind: as_ts_script_kind(*media_type), + })) + } + _ => None, + }, + }; + let maybe_source = if let Some(module) = maybe_module { match module { Module::Js(module) => { media_type = module.media_type; @@ -674,7 +687,20 @@ fn resolve_graph_specifier_types( state: &State, ) -> Result, AnyError> { let graph = &state.graph; - let maybe_module = graph.get(specifier); + let maybe_module = match graph.try_get(specifier) { + Ok(Some(module)) => Some(module), + Ok(None) => None, + Err(err) => match err { + deno_graph::ModuleError::UnsupportedMediaType( + specifier, + media_type, + _, + ) => { + return Ok(Some((specifier.clone(), *media_type))); + } + _ => None, + }, + }; // follow the types reference directive, which may be pointing at an npm package let maybe_module = match maybe_module { Some(Module::Js(module)) => { diff --git a/tests/specs/check/css_import/__test__.jsonc b/tests/specs/check/css_import/__test__.jsonc new file mode 100644 index 00000000000000..629dcd3833d07c --- /dev/null +++ b/tests/specs/check/css_import/__test__.jsonc @@ -0,0 +1,24 @@ +{ + "steps": [{ + "args": "check exists.ts", + "output": "exists.out" + }, { + "args": "run --check exists.ts", + "output": "exists_run_with_check.out", + "exitCode": 1 + }, { + "args": "check not_exists.ts", + "output": "not_exists.out", + "exitCode": 1 + }, { + "args": "check exists_and_try_uses.ts", + "output": "exists_and_try_uses.out", + "exitCode": 1 + }, { + "args": "check exists_dynamic_import.ts", + "output": "Check file:///[WILDCARD]exists_dynamic_import.ts\n" + }, { + "args": "run --check --reload exists_dynamic_import.ts", + "output": "Check file:///[WILDCARD]exists_dynamic_import.ts\n" + }] +} diff --git a/tests/specs/check/css_import/app.css b/tests/specs/check/css_import/app.css new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/specs/check/css_import/exists.out b/tests/specs/check/css_import/exists.out new file mode 100644 index 00000000000000..1e9af360769642 --- /dev/null +++ b/tests/specs/check/css_import/exists.out @@ -0,0 +1 @@ +Check [WILDLINE]exists.ts diff --git a/tests/specs/check/css_import/exists.ts b/tests/specs/check/css_import/exists.ts new file mode 100644 index 00000000000000..d5e05ae672a56f --- /dev/null +++ b/tests/specs/check/css_import/exists.ts @@ -0,0 +1,2 @@ +// should not error for deno check +import "./app.css"; diff --git a/tests/specs/check/css_import/exists_and_try_uses.out b/tests/specs/check/css_import/exists_and_try_uses.out new file mode 100644 index 00000000000000..b51ea2391a1d9b --- /dev/null +++ b/tests/specs/check/css_import/exists_and_try_uses.out @@ -0,0 +1,5 @@ +Check file:///[WILDLINE]/exists_and_try_uses.ts +error: TS1192 [ERROR]: Module '"file:///[WILDLINE]/app.css"' has no default export. +import test from "./app.css"; + ~~~~ + at file:///[WILDLINE]/exists_and_try_uses.ts:1:8 diff --git a/tests/specs/check/css_import/exists_and_try_uses.ts b/tests/specs/check/css_import/exists_and_try_uses.ts new file mode 100644 index 00000000000000..afaa6e4e012085 --- /dev/null +++ b/tests/specs/check/css_import/exists_and_try_uses.ts @@ -0,0 +1,3 @@ +import test from "./app.css"; + +test(123); diff --git a/tests/specs/check/css_import/exists_dynamic_import.ts b/tests/specs/check/css_import/exists_dynamic_import.ts new file mode 100644 index 00000000000000..14f63fd2315e78 --- /dev/null +++ b/tests/specs/check/css_import/exists_dynamic_import.ts @@ -0,0 +1,3 @@ +if (false) { + await import("./app.css"); +} diff --git a/tests/specs/check/css_import/exists_run_with_check.out b/tests/specs/check/css_import/exists_run_with_check.out new file mode 100644 index 00000000000000..1a1dafeb74509c --- /dev/null +++ b/tests/specs/check/css_import/exists_run_with_check.out @@ -0,0 +1,3 @@ +error: Expected a JavaScript or TypeScript module, but identified a Unknown module. Importing these types of modules is currently not supported. + Specifier: file:///[WILDLINE]/app.css + at file:///[WILDLINE]/exists.ts:2:8 diff --git a/tests/specs/check/css_import/not_exists.out b/tests/specs/check/css_import/not_exists.out new file mode 100644 index 00000000000000..95fd14668ee4d5 --- /dev/null +++ b/tests/specs/check/css_import/not_exists.out @@ -0,0 +1,2 @@ +error: Module not found "file:///[WILDLINE]/not_exists.css". + at file:///[WILDLINE]/not_exists.ts:1:8 diff --git a/tests/specs/check/css_import/not_exists.ts b/tests/specs/check/css_import/not_exists.ts new file mode 100644 index 00000000000000..762c9acd77eb09 --- /dev/null +++ b/tests/specs/check/css_import/not_exists.ts @@ -0,0 +1 @@ +import "./not_exists.css";