Skip to content

Commit

Permalink
feat(check): allow using side effect imports with unknown module kind…
Browse files Browse the repository at this point in the history
…s (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.
  • Loading branch information
dsherret committed Apr 16, 2024
1 parent 422cff1 commit 43c8c1c
Show file tree
Hide file tree
Showing 13 changed files with 84 additions and 3 deletions.
1 change: 1 addition & 0 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
12 changes: 11 additions & 1 deletion cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
30 changes: 28 additions & 2 deletions cli/tsc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -674,7 +687,20 @@ fn resolve_graph_specifier_types(
state: &State,
) -> Result<Option<(ModuleSpecifier, MediaType)>, 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)) => {
Expand Down
24 changes: 24 additions & 0 deletions tests/specs/check/css_import/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -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"
}]
}
Empty file.
1 change: 1 addition & 0 deletions tests/specs/check/css_import/exists.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Check [WILDLINE]exists.ts
2 changes: 2 additions & 0 deletions tests/specs/check/css_import/exists.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// should not error for deno check
import "./app.css";
5 changes: 5 additions & 0 deletions tests/specs/check/css_import/exists_and_try_uses.out
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions tests/specs/check/css_import/exists_and_try_uses.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import test from "./app.css";

test(123);
3 changes: 3 additions & 0 deletions tests/specs/check/css_import/exists_dynamic_import.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
if (false) {
await import("./app.css");
}
3 changes: 3 additions & 0 deletions tests/specs/check/css_import/exists_run_with_check.out
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions tests/specs/check/css_import/not_exists.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
error: Module not found "file:///[WILDLINE]/not_exists.css".
at file:///[WILDLINE]/not_exists.ts:1:8
1 change: 1 addition & 0 deletions tests/specs/check/css_import/not_exists.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "./not_exists.css";

0 comments on commit 43c8c1c

Please sign in to comment.