fix(install): don't error on unknown media types in install (#28234)

Fixes https://github.com/denoland/deno/issues/28223

This is kind of an ugly fix, but it works, and I think is the easiest
way to handle the fact that when caching the module graph we might
encounter imports that won't actually error at runtime (for instance in
files that will be bundled).
This commit is contained in:
Nathan Whitaker 2025-02-21 12:20:55 -08:00 committed by Ryan Dahl
parent 2a9d873dd8
commit f566aeccb7
16 changed files with 128 additions and 33 deletions

View file

@ -13,6 +13,7 @@ use deno_runtime::deno_permissions::PermissionsContainer;
use crate::args::CliOptions;
use crate::module_loader::ModuleLoadPreparer;
use crate::module_loader::PrepareModuleLoadOptions;
use crate::util::fs::collect_specifiers;
use crate::util::path::is_script_ext;
@ -48,6 +49,12 @@ pub struct MainModuleGraphContainer {
root_permissions: PermissionsContainer,
}
#[derive(Default, Debug)]
pub struct CheckSpecifiersOptions<'a> {
pub ext_overwrite: Option<&'a String>,
pub allow_unknown_media_types: bool,
}
impl MainModuleGraphContainer {
pub fn new(
cli_options: Arc<CliOptions>,
@ -68,7 +75,7 @@ impl MainModuleGraphContainer {
pub async fn check_specifiers(
&self,
specifiers: &[ModuleSpecifier],
ext_overwrite: Option<&String>,
options: CheckSpecifiersOptions<'_>,
) -> Result<(), AnyError> {
let mut graph_permit = self.acquire_update_permit().await;
let graph = graph_permit.graph_mut();
@ -77,10 +84,13 @@ impl MainModuleGraphContainer {
.prepare_module_load(
graph,
specifiers,
false,
self.cli_options.ts_type_lib_window(),
self.root_permissions.clone(),
ext_overwrite,
PrepareModuleLoadOptions {
is_dynamic: false,
lib: self.cli_options.ts_type_lib_window(),
permissions: self.root_permissions.clone(),
ext_overwrite: options.ext_overwrite,
allow_unknown_media_types: options.allow_unknown_media_types,
},
)
.await?;
graph_permit.commit();
@ -99,7 +109,7 @@ impl MainModuleGraphContainer {
log::warn!("{} No matching files found.", colors::yellow("Warning"));
}
self.check_specifiers(&specifiers, None).await
self.check_specifiers(&specifiers, Default::default()).await
}
pub fn collect_specifiers(

View file

@ -79,6 +79,7 @@ pub struct GraphValidOptions<'a> {
/// lockfile checksum mismatches and JSR integrity failures.
/// Otherwise, surfaces integrity errors as errors.
pub exit_integrity_errors: bool,
pub allow_unknown_media_types: bool,
}
/// Check if `roots` and their deps are available. Returns `Ok(())` if
@ -105,6 +106,7 @@ pub fn graph_valid(
GraphWalkErrorsOptions {
check_js: options.check_js,
kind: options.kind,
allow_unknown_media_types: options.allow_unknown_media_types,
},
);
if let Some(error) = errors.next() {
@ -144,6 +146,7 @@ pub fn fill_graph_from_lockfile(
pub struct GraphWalkErrorsOptions<'a> {
pub check_js: CheckJsOption<'a>,
pub kind: GraphKind,
pub allow_unknown_media_types: bool,
}
/// Walks the errors found in the module graph that should be surfaced to users
@ -157,9 +160,10 @@ pub fn graph_walk_errors<'a>(
fn should_ignore_error(
sys: &CliSys,
graph_kind: GraphKind,
allow_unknown_media_types: bool,
error: &ModuleGraphError,
) -> bool {
if graph_kind == GraphKind::TypesOnly
if (graph_kind == GraphKind::TypesOnly || allow_unknown_media_types)
&& matches!(
error,
ModuleGraphError::ModuleError(ModuleError::UnsupportedMediaType(..))
@ -184,8 +188,13 @@ pub fn graph_walk_errors<'a>(
},
)
.errors()
.flat_map(|error| {
if should_ignore_error(sys, graph.graph_kind(), &error) {
.flat_map(move |error| {
if should_ignore_error(
sys,
graph.graph_kind(),
options.allow_unknown_media_types,
&error,
) {
log::debug!("Ignoring: {}", error);
return None;
}
@ -934,6 +943,7 @@ impl ModuleGraphBuilder {
self.graph_roots_valid(
graph,
&graph.roots.iter().cloned().collect::<Vec<_>>(),
false,
)
}
@ -941,6 +951,7 @@ impl ModuleGraphBuilder {
&self,
graph: &ModuleGraph,
roots: &[ModuleSpecifier],
allow_unknown_media_types: bool,
) -> Result<(), JsErrorBox> {
graph_valid(
graph,
@ -954,6 +965,7 @@ impl ModuleGraphBuilder {
},
check_js: CheckJsOption::Custom(self.tsconfig_resolver.as_ref()),
exit_integrity_errors: true,
allow_unknown_media_types,
},
)
}

View file

@ -371,6 +371,7 @@ impl LanguageServer {
kind: GraphKind::All,
check_js: CheckJsOption::False,
exit_integrity_errors: false,
allow_unknown_media_types: true,
},
)?;

View file

@ -235,7 +235,10 @@ impl TestRun {
)?;
let main_graph_container = factory.main_module_graph_container().await?;
main_graph_container
.check_specifiers(&self.queue.iter().cloned().collect::<Vec<_>>(), None)
.check_specifiers(
&self.queue.iter().cloned().collect::<Vec<_>>(),
Default::default(),
)
.await?;
let (concurrent_jobs, fail_fast) =

View file

@ -124,6 +124,14 @@ pub struct ModuleLoadPreparer {
type_checker: Arc<TypeChecker>,
}
pub struct PrepareModuleLoadOptions<'a> {
pub is_dynamic: bool,
pub lib: TsTypeLib,
pub permissions: PermissionsContainer,
pub ext_overwrite: Option<&'a String>,
pub allow_unknown_media_types: bool,
}
impl ModuleLoadPreparer {
#[allow(clippy::too_many_arguments)]
pub fn new(
@ -146,17 +154,20 @@ impl ModuleLoadPreparer {
/// module before attempting to `load()` it from a `JsRuntime`. It will
/// populate the graph data in memory with the necessary source code, write
/// emits where necessary or report any module graph / type checking errors.
#[allow(clippy::too_many_arguments)]
pub async fn prepare_module_load(
&self,
graph: &mut ModuleGraph,
roots: &[ModuleSpecifier],
is_dynamic: bool,
lib: TsTypeLib,
permissions: PermissionsContainer,
ext_overwrite: Option<&String>,
options: PrepareModuleLoadOptions<'_>,
) -> Result<(), PrepareModuleLoadError> {
log::debug!("Preparing module load.");
let PrepareModuleLoadOptions {
is_dynamic,
lib,
permissions,
ext_overwrite,
allow_unknown_media_types,
} = options;
let _pb_clear_guard = self.progress_bar.clear_guard();
let mut cache = self.module_graph_builder.create_fetch_cacher(permissions);
@ -197,7 +208,7 @@ impl ModuleLoadPreparer {
)
.await?;
self.graph_roots_valid(graph, roots)?;
self.graph_roots_valid(graph, roots, allow_unknown_media_types)?;
// write the lockfile if there is one
if let Some(lockfile) = &self.lockfile {
@ -235,8 +246,13 @@ impl ModuleLoadPreparer {
&self,
graph: &ModuleGraph,
roots: &[ModuleSpecifier],
allow_unknown_media_types: bool,
) -> Result<(), JsErrorBox> {
self.module_graph_builder.graph_roots_valid(graph, roots)
self.module_graph_builder.graph_roots_valid(
graph,
roots,
allow_unknown_media_types,
)
}
}
@ -1067,7 +1083,11 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
log::debug!("Skipping prepare module load.");
// roots are already validated so we can skip those
if !graph.roots.contains(&specifier) {
module_load_preparer.graph_roots_valid(&graph, &[specifier])?;
module_load_preparer.graph_roots_valid(
&graph,
&[specifier],
false,
)?;
}
return Ok(());
}
@ -1086,10 +1106,13 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
.prepare_module_load(
graph,
&[specifier],
is_dynamic,
lib,
permissions,
None,
PrepareModuleLoadOptions {
is_dynamic,
lib,
permissions,
ext_overwrite: None,
allow_unknown_media_types: false,
},
)
.await
.map_err(JsErrorBox::from_err)?;

View file

@ -38,6 +38,7 @@ use crate::args::Flags;
use crate::colors;
use crate::display::write_json_to_stdout;
use crate::factory::CliFactory;
use crate::graph_container::CheckSpecifiersOptions;
use crate::graph_util::has_graph_root_local_dependent_changed;
use crate::ops;
use crate::sys::CliSys;
@ -462,7 +463,13 @@ pub async fn run_benchmarks(
let main_graph_container = factory.main_module_graph_container().await?;
main_graph_container
.check_specifiers(&specifiers, cli_options.ext_flag().as_ref())
.check_specifiers(
&specifiers,
CheckSpecifiersOptions {
ext_overwrite: cli_options.ext_flag().as_ref(),
..Default::default()
},
)
.await?;
if workspace_bench_options.no_run {
@ -595,7 +602,13 @@ pub async fn run_benchmarks_with_watch(
factory
.main_module_graph_container()
.await?
.check_specifiers(&specifiers, cli_options.ext_flag().as_ref())
.check_specifiers(
&specifiers,
CheckSpecifiersOptions {
ext_overwrite: cli_options.ext_flag().as_ref(),
allow_unknown_media_types: false,
},
)
.await?;
if workspace_bench_options.no_run {

View file

@ -89,7 +89,7 @@ pub async fn check(
};
main_graph_container
.check_specifiers(&specifiers_for_typecheck, None)
.check_specifiers(&specifiers_for_typecheck, Default::default())
.await
}

View file

@ -151,6 +151,7 @@ pub async fn doc(
GraphWalkErrorsOptions {
check_js: CheckJsOption::False,
kind: GraphKind::TypesOnly,
allow_unknown_media_types: false,
},
);
for error in errors {

View file

@ -279,8 +279,15 @@ pub(crate) async fn install_from_entrypoints(
let factory = CliFactory::from_flags(flags.clone());
let emitter = factory.emitter()?;
let main_graph_container = factory.main_module_graph_container().await?;
let specifiers = main_graph_container.collect_specifiers(entrypoints)?;
main_graph_container
.load_and_type_check_files(entrypoints)
.check_specifiers(
&specifiers,
crate::graph_container::CheckSpecifiersOptions {
ext_overwrite: None,
allow_unknown_media_types: true,
},
)
.await?;
emitter
.cache_module_emits(&main_graph_container.graph())

View file

@ -136,7 +136,7 @@ pub async fn cache_top_level_deps(
},
)
.await?;
maybe_graph_error = graph_builder.graph_roots_valid(graph, &roots);
maybe_graph_error = graph_builder.graph_roots_valid(graph, &roots, true);
}
npm_installer.cache_packages(PackageCaching::All).await?;

View file

@ -608,10 +608,13 @@ impl DepManager {
.prepare_module_load(
graph,
&roots,
false,
deno_config::deno_json::TsTypeLib::DenoWindow,
self.permissions_container.clone(),
None,
crate::module_loader::PrepareModuleLoadOptions {
is_dynamic: false,
lib: deno_config::deno_json::TsTypeLib::DenoWindow,
permissions: self.permissions_container.clone(),
ext_overwrite: None,
allow_unknown_media_types: true,
},
)
.await?;

View file

@ -76,6 +76,7 @@ use crate::colors;
use crate::display;
use crate::factory::CliFactory;
use crate::file_fetcher::CliFileFetcher;
use crate::graph_container::CheckSpecifiersOptions;
use crate::graph_util::has_graph_root_local_dependent_changed;
use crate::ops;
use crate::sys::CliSys;
@ -1599,7 +1600,10 @@ pub async fn run_tests(
main_graph_container
.check_specifiers(
&specifiers_for_typecheck_and_test,
cli_options.ext_flag().as_ref(),
CheckSpecifiersOptions {
ext_overwrite: cli_options.ext_flag().as_ref(),
..Default::default()
},
)
.await?;
@ -1782,7 +1786,10 @@ pub async fn run_tests_with_watch(
main_graph_container
.check_specifiers(
&specifiers_for_typecheck_and_test,
cli_options.ext_flag().as_ref(),
crate::graph_container::CheckSpecifiersOptions {
ext_overwrite: cli_options.ext_flag().as_ref(),
..Default::default()
},
)
.await?;

View file

@ -0,0 +1,9 @@
{
"tempDir": true,
"steps": [
{
"args": "install",
"output": ""
}
]
}

View file

@ -0,0 +1,5 @@
{
"imports": {
"foo": "./foo.ts"
}
}

View file

@ -0,0 +1 @@
import styles from "./styles.css";