Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: default to TS for file extension and support ext flag in more scenarios #25472

Merged
merged 17 commits into from
Sep 18, 2024
Merged
4 changes: 4 additions & 0 deletions .dprint.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,14 @@
"tests/node_compat/runner/TODO.md",
"tests/node_compat/test",
"tests/registry/",
"tests/specs/bench/default_ts",
"tests/specs/fmt",
"tests/specs/lint/bom",
"tests/specs/lint/default_ts",
"tests/specs/lint/syntax_error_reporting",
"tests/specs/publish/no_check_surfaces_syntax_error",
"tests/specs/run/default_ts",
"tests/specs/test/default_ts",
"tests/testdata/byte_order_mark.ts",
"tests/testdata/encoding",
"tests/testdata/file_extensions/ts_with_js_extension.js",
Expand Down
34 changes: 7 additions & 27 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ pub struct LintFlags {
pub json: bool,
pub compact: bool,
pub watch: Option<WatchFlags>,
pub ext: Option<String>,
}

impl LintFlags {
Expand Down Expand Up @@ -1622,6 +1621,7 @@ If you specify a directory instead of a file, the path is expanded to all contai
.arg(no_clear_screen_arg())
.arg(script_arg().last(true))
.arg(env_file_arg())
.arg(executable_ext_arg())
})
}

Expand Down Expand Up @@ -2118,9 +2118,6 @@ Ignore formatting a file by adding an ignore comment at the top of the file:
Arg::new("ext")
.long("ext")
.help("Set content type of the supplied file")
// prefer using ts for formatting instead of js because ts works in more scenarios
.default_value("ts")
.hide_default_value(true)
.value_parser([
"ts", "tsx", "js", "jsx", "md", "json", "jsonc", "css", "scss",
"sass", "less", "html", "svelte", "vue", "astro", "yml", "yaml",
Expand Down Expand Up @@ -2903,6 +2900,7 @@ or <c>**/__tests__/**</>:
.action(ArgAction::SetTrue)
)
.arg(env_file_arg())
.arg(executable_ext_arg())
)
}

Expand Down Expand Up @@ -4067,6 +4065,7 @@ fn bench_parse(
flags.type_check_mode = TypeCheckMode::Local;

runtime_args_parse(flags, matches, true, false)?;
ext_arg_parse(flags, matches);

// NOTE: `deno bench` always uses `--no-prompt`, tests shouldn't ever do
// interactive prompts, unless done by user code
Expand Down Expand Up @@ -4590,8 +4589,9 @@ fn lint_parse(
matches: &mut ArgMatches,
) -> clap::error::Result<()> {
unstable_args_parse(flags, matches, UnstableArgsConfig::ResolutionOnly);

ext_arg_parse(flags, matches);
config_args_parse(flags, matches);

let files = match matches.remove_many::<String>("files") {
Some(f) => f.collect(),
None => vec![],
Expand All @@ -4618,7 +4618,6 @@ fn lint_parse(

let json = matches.get_flag("json");
let compact = matches.get_flag("compact");
let ext = matches.remove_one::<String>("ext");

flags.subcommand = DenoSubcommand::Lint(LintFlags {
files: FileFlags {
Expand All @@ -4633,7 +4632,6 @@ fn lint_parse(
json,
compact,
watch: watch_arg_parse(matches)?,
ext,
});
Ok(())
}
Expand Down Expand Up @@ -4809,6 +4807,8 @@ fn test_parse(
) -> clap::error::Result<()> {
flags.type_check_mode = TypeCheckMode::Local;
runtime_args_parse(flags, matches, true, true)?;
ext_arg_parse(flags, matches);

// NOTE: `deno test` always uses `--no-prompt`, tests shouldn't ever do
// interactive prompts, unless done by user code
flags.permissions.no_prompt = true;
Expand Down Expand Up @@ -6268,7 +6268,6 @@ mod tests {
unstable_yaml: false,
watch: Default::default(),
}),
ext: Some("ts".to_string()),
..Flags::default()
}
);
Expand All @@ -6295,7 +6294,6 @@ mod tests {
unstable_yaml: false,
watch: Default::default(),
}),
ext: Some("ts".to_string()),
..Flags::default()
}
);
Expand All @@ -6322,7 +6320,6 @@ mod tests {
unstable_yaml: false,
watch: Default::default(),
}),
ext: Some("ts".to_string()),
..Flags::default()
}
);
Expand All @@ -6349,7 +6346,6 @@ mod tests {
unstable_yaml: false,
watch: Some(Default::default()),
}),
ext: Some("ts".to_string()),
..Flags::default()
}
);
Expand Down Expand Up @@ -6389,7 +6385,6 @@ mod tests {
exclude: vec![],
})
}),
ext: Some("ts".to_string()),
..Flags::default()
}
);
Expand Down Expand Up @@ -6423,7 +6418,6 @@ mod tests {
unstable_yaml: false,
watch: Some(Default::default()),
}),
ext: Some("ts".to_string()),
..Flags::default()
}
);
Expand All @@ -6450,7 +6444,6 @@ mod tests {
unstable_yaml: false,
watch: Default::default(),
}),
ext: Some("ts".to_string()),
config_flag: ConfigFlag::Path("deno.jsonc".to_string()),
..Flags::default()
}
Expand Down Expand Up @@ -6486,7 +6479,6 @@ mod tests {
watch: Some(Default::default()),
}),
config_flag: ConfigFlag::Path("deno.jsonc".to_string()),
ext: Some("ts".to_string()),
..Flags::default()
}
);
Expand Down Expand Up @@ -6525,7 +6517,6 @@ mod tests {
unstable_yaml: false,
watch: Default::default(),
}),
ext: Some("ts".to_string()),
..Flags::default()
}
);
Expand Down Expand Up @@ -6559,7 +6550,6 @@ mod tests {
unstable_yaml: false,
watch: Default::default(),
}),
ext: Some("ts".to_string()),
..Flags::default()
}
);
Expand All @@ -6584,7 +6574,6 @@ mod tests {
json: false,
compact: false,
watch: Default::default(),
ext: None,
}),
..Flags::default()
}
Expand Down Expand Up @@ -6613,7 +6602,6 @@ mod tests {
json: false,
compact: false,
watch: Some(Default::default()),
ext: None,
}),
..Flags::default()
}
Expand Down Expand Up @@ -6647,7 +6635,6 @@ mod tests {
no_clear_screen: true,
exclude: vec![],
}),
ext: None,
}),
..Flags::default()
}
Expand Down Expand Up @@ -6675,7 +6662,6 @@ mod tests {
json: false,
compact: false,
watch: Default::default(),
ext: None,
}),
..Flags::default()
}
Expand All @@ -6698,7 +6684,6 @@ mod tests {
json: false,
compact: false,
watch: Default::default(),
ext: None,
}),
..Flags::default()
}
Expand Down Expand Up @@ -6726,7 +6711,6 @@ mod tests {
json: false,
compact: false,
watch: Default::default(),
ext: None,
}),
..Flags::default()
}
Expand Down Expand Up @@ -6755,7 +6739,6 @@ mod tests {
json: false,
compact: false,
watch: Default::default(),
ext: None,
}),
..Flags::default()
}
Expand All @@ -6778,7 +6761,6 @@ mod tests {
json: true,
compact: false,
watch: Default::default(),
ext: None,
}),
..Flags::default()
}
Expand Down Expand Up @@ -6808,7 +6790,6 @@ mod tests {
json: true,
compact: false,
watch: Default::default(),
ext: None,
}),
config_flag: ConfigFlag::Path("Deno.jsonc".to_string()),
..Flags::default()
Expand Down Expand Up @@ -6839,7 +6820,6 @@ mod tests {
json: false,
compact: true,
watch: Default::default(),
ext: None,
}),
config_flag: ConfigFlag::Path("Deno.jsonc".to_string()),
..Flags::default()
Expand Down
2 changes: 1 addition & 1 deletion cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1131,7 +1131,7 @@ impl CliOptions {
resolve_url_or_path(&compile_flags.source_file, self.initial_cwd())?
}
DenoSubcommand::Eval(_) => {
resolve_url_or_path("./$deno$eval", self.initial_cwd())?
resolve_url_or_path("./$deno$eval.ts", self.initial_cwd())?
}
DenoSubcommand::Repl(_) => {
resolve_url_or_path("./$deno$repl.ts", self.initial_cwd())?
Expand Down
2 changes: 1 addition & 1 deletion cli/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub use deno_cache_dir::HttpCache;
/// a concise interface to the DENO_DIR when building module graphs.
pub struct FetchCacher {
file_fetcher: Arc<FileFetcher>,
file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
pub file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
global_http_cache: Arc<GlobalHttpCache>,
npm_resolver: Arc<dyn CliNpmResolver>,
module_info_cache: Arc<ModuleInfoCache>,
Expand Down
11 changes: 10 additions & 1 deletion cli/file_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,20 @@ fn fetch_local(specifier: &ModuleSpecifier) -> Result<File, AnyError> {
let local = specifier.to_file_path().map_err(|_| {
uri_error(format!("Invalid file path.\n Specifier: {specifier}"))
})?;
// If it doesnt have a extension, we want to treat it as typescript by default
let headers = if local.extension().is_none() {
Some(HashMap::from([(
"content-type".to_string(),
"application/typescript".to_string(),
)]))
} else {
None
};
Comment on lines +142 to +149
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you want is MediaType::from_specifier(specifier) != MediaType::Unknown

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, as per what you have said in #11220 (comment)

extensionless files default to TS everywhere (entrypoint and imported)

let bytes = fs::read(local)?;

Ok(File {
specifier: specifier.clone(),
maybe_headers: None,
maybe_headers: headers,
source: bytes.into(),
})
}
Expand Down
4 changes: 3 additions & 1 deletion cli/graph_container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ impl MainModuleGraphContainer {
pub async fn check_specifiers(
&self,
specifiers: &[ModuleSpecifier],
ext_overwrite: Option<&String>,
) -> Result<(), AnyError> {
let mut graph_permit = self.acquire_update_permit().await;
let graph = graph_permit.graph_mut();
Expand All @@ -76,6 +77,7 @@ impl MainModuleGraphContainer {
false,
self.cli_options.ts_type_lib_window(),
FetchPermissionsOption::AllowAll,
ext_overwrite,
)
.await?;
graph_permit.commit();
Expand All @@ -94,7 +96,7 @@ impl MainModuleGraphContainer {
log::warn!("{} No matching files found.", colors::yellow("Warning"));
}

self.check_specifiers(&specifiers).await
self.check_specifiers(&specifiers, None).await
}

pub fn collect_specifiers(
Expand Down
1 change: 1 addition & 0 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1420,6 +1420,7 @@ impl Inner {
document.content(),
&fmt_options,
&unstable_options,
None,
)
}
};
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/testing/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ impl TestRun {
)?;
let main_graph_container = factory.main_module_graph_container().await?;
main_graph_container
.check_specifiers(&self.queue.iter().cloned().collect::<Vec<_>>())
.check_specifiers(&self.queue.iter().cloned().collect::<Vec<_>>(), None)
.await?;

let (concurrent_jobs, fail_fast) =
Expand Down
22 changes: 22 additions & 0 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,32 @@ impl ModuleLoadPreparer {
is_dynamic: bool,
lib: TsTypeLib,
permissions: crate::file_fetcher::FetchPermissionsOption,
ext_overwrite: Option<&String>,
) -> Result<(), AnyError> {
log::debug!("Preparing module load.");
let _pb_clear_guard = self.progress_bar.clear_guard();

let mut cache = self.module_graph_builder.create_fetch_cacher(permissions);
if let Some(ext) = ext_overwrite {
let maybe_content_type = match ext.as_str() {
"ts" => Some("text/typescript"),
"tsx" => Some("text/tsx"),
"js" => Some("text/javascript"),
"jsx" => Some("text/jsx"),
_ => None,
};
if let Some(content_type) = maybe_content_type {
for root in roots {
cache.file_header_overrides.insert(
root.clone(),
std::collections::HashMap::from([(
"content-type".to_string(),
content_type.to_string(),
)]),
);
}
}
}
log::debug!("Building module graph.");
let has_type_checked = !graph.roots.is_empty();

Expand Down Expand Up @@ -763,6 +784,7 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
is_dynamic,
lib,
root_permissions.into(),
None,
)
.await?;
update_permit.commit();
Expand Down
6 changes: 4 additions & 2 deletions cli/tools/bench/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,9 @@ pub async fn run_benchmarks(
}

let main_graph_container = factory.main_module_graph_container().await?;
main_graph_container.check_specifiers(&specifiers).await?;
main_graph_container
.check_specifiers(&specifiers, cli_options.ext_flag().as_ref())
.await?;

if workspace_bench_options.no_run {
return Ok(());
Expand Down Expand Up @@ -569,7 +571,7 @@ pub async fn run_benchmarks_with_watch(
factory
.main_module_graph_container()
.await?
.check_specifiers(&specifiers)
.check_specifiers(&specifiers, cli_options.ext_flag().as_ref())
.await?;

if workspace_bench_options.no_run {
Expand Down
Loading