Skip to content

Commit

Permalink
fix(cli/emit): Check JS roots with // @ts-check (denoland#14090)
Browse files Browse the repository at this point in the history
  • Loading branch information
nayeemrmn committed Apr 18, 2022
1 parent f52031e commit 2f29673
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 13 deletions.
43 changes: 30 additions & 13 deletions cli/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,21 @@ fn get_tsc_roots(
.entries()
.into_iter()
.filter_map(|(specifier, module_entry)| match module_entry {
ModuleEntry::Module { media_type, .. } => match &media_type {
ModuleEntry::Module {
media_type,
ts_check,
..
} => match &media_type {
MediaType::TypeScript
| MediaType::Tsx
| MediaType::Mts
| MediaType::Cts
| MediaType::Jsx => Some((specifier.clone(), *media_type)),
MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs
if check_js || *ts_check =>
{
Some((specifier.clone(), *media_type))
}
_ => None,
},
_ => None,
Expand Down Expand Up @@ -461,20 +470,25 @@ pub fn check_and_maybe_emit(
// resolve it via the graph.
let graph_data = graph_data.read();
let specifier = graph_data.follow_redirect(&specifiers[0]);
let (source_bytes, media_type) = match graph_data.get(&specifier) {
Some(ModuleEntry::Module {
code, media_type, ..
}) => (code.as_bytes(), *media_type),
_ => {
log::debug!("skipping emit for {}", specifier);
continue;
}
};
let (source_bytes, media_type, ts_check) =
match graph_data.get(&specifier) {
Some(ModuleEntry::Module {
code,
media_type,
ts_check,
..
}) => (code.as_bytes(), *media_type, *ts_check),
_ => {
log::debug!("skipping emit for {}", specifier);
continue;
}
};
// Sometimes if `tsc` sees a CommonJS file or a JSON module, it will
// _helpfully_ output it, which we don't really want to do unless
// someone has enabled check_js.
if matches!(media_type, MediaType::Json)
|| (!check_js
&& !ts_check
&& matches!(
media_type,
MediaType::JavaScript | MediaType::Cjs | MediaType::Mjs
Expand Down Expand Up @@ -862,10 +876,13 @@ fn valid_emit(
reload_exclusions: &HashSet<ModuleSpecifier>,
) -> bool {
let config_bytes = ts_config.as_bytes();
let emit_js = ts_config.get_check_js();
let check_js = ts_config.get_check_js();
for (specifier, module_entry) in graph_data.entries() {
if let ModuleEntry::Module {
code, media_type, ..
code,
media_type,
ts_check,
..
} = module_entry
{
match media_type {
Expand All @@ -875,7 +892,7 @@ fn valid_emit(
| MediaType::Tsx
| MediaType::Jsx => {}
MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs => {
if !emit_js {
if !check_js && !ts_check {
continue;
}
}
Expand Down
22 changes: 22 additions & 0 deletions cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,18 @@ use deno_graph::ModuleGraphError;
use deno_graph::ModuleKind;
use deno_graph::Range;
use deno_graph::Resolved;
use once_cell::sync::Lazy;
use regex::Regex;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::collections::HashSet;
use std::collections::VecDeque;
use std::sync::Arc;

/// Matches the `@ts-check` pragma.
static TS_CHECK_RE: Lazy<Regex> =
Lazy::new(|| Regex::new(r#"(?i)^\s*@ts-check(?:\s+|$)"#).unwrap());

pub fn contains_specifier(
v: &[(ModuleSpecifier, ModuleKind)],
specifier: &ModuleSpecifier,
Expand All @@ -33,6 +39,8 @@ pub enum ModuleEntry {
code: Arc<String>,
dependencies: BTreeMap<String, Dependency>,
media_type: MediaType,
/// Whether or not this is a JS/JSX module with a `@ts-check` directive.
ts_check: bool,
/// A set of type libs that the module has passed a type check with this
/// session. This would consist of window, worker or both.
checked_libs: HashSet<TypeLib>,
Expand Down Expand Up @@ -122,9 +130,23 @@ impl GraphData {
}
}
}
let ts_check = match &media_type {
MediaType::JavaScript
| MediaType::Mjs
| MediaType::Cjs
| MediaType::Jsx => {
let parsed_source = module.maybe_parsed_source.as_ref().unwrap();
parsed_source
.get_leading_comments()
.iter()
.any(|c| TS_CHECK_RE.is_match(&c.text))
}
_ => false,
};
let module_entry = ModuleEntry::Module {
code,
dependencies: module.dependencies.clone(),
ts_check,
media_type,
checked_libs: Default::default(),
maybe_types,
Expand Down
7 changes: 7 additions & 0 deletions cli/tests/integration/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2726,3 +2726,10 @@ itest!(complex_error {
output: "complex_error.ts.out",
exit_code: 1,
});

// Regression test for https://github.com/denoland/deno/issues/12143.
itest!(js_root_with_ts_check {
args: "run --quiet js_root_with_ts_check.js",
output: "js_root_with_ts_check.js.out",
exit_code: 1,
});
5 changes: 5 additions & 0 deletions cli/tests/testdata/js_root_with_ts_check.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// @ts-check

/** @type {number} */
const a = "";
console.log(a);
4 changes: 4 additions & 0 deletions cli/tests/testdata/js_root_with_ts_check.js.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: TS2322 [ERROR]: Type 'string' is not assignable to type 'number'.
const a = "";
^
at [WILDCARD]

0 comments on commit 2f29673

Please sign in to comment.