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

View file

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

View file

@ -280,6 +280,7 @@ impl LanguageServer {
kind: GraphKind::All, kind: GraphKind::All,
check_js: CheckJsOption::False, check_js: CheckJsOption::False,
exit_integrity_errors: 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?; let main_graph_container = factory.main_module_graph_container().await?;
main_graph_container main_graph_container
.check_specifiers(&self.queue.iter().cloned().collect::<Vec<_>>(), None) .check_specifiers(
&self.queue.iter().cloned().collect::<Vec<_>>(),
Default::default(),
)
.await?; .await?;
let (concurrent_jobs, fail_fast) = let (concurrent_jobs, fail_fast) =

View file

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

View file

@ -38,6 +38,7 @@ use crate::args::Flags;
use crate::colors; use crate::colors;
use crate::display::write_json_to_stdout; use crate::display::write_json_to_stdout;
use crate::factory::CliFactory; use crate::factory::CliFactory;
use crate::graph_container::CheckSpecifiersOptions;
use crate::graph_util::has_graph_root_local_dependent_changed; use crate::graph_util::has_graph_root_local_dependent_changed;
use crate::ops; use crate::ops;
use crate::sys::CliSys; use crate::sys::CliSys;
@ -462,7 +463,13 @@ pub async fn run_benchmarks(
let main_graph_container = factory.main_module_graph_container().await?; let main_graph_container = factory.main_module_graph_container().await?;
main_graph_container 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?; .await?;
if workspace_bench_options.no_run { if workspace_bench_options.no_run {
@ -595,7 +602,13 @@ pub async fn run_benchmarks_with_watch(
factory factory
.main_module_graph_container() .main_module_graph_container()
.await? .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?; .await?;
if workspace_bench_options.no_run { if workspace_bench_options.no_run {

View file

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

View file

@ -151,6 +151,7 @@ pub async fn doc(
GraphWalkErrorsOptions { GraphWalkErrorsOptions {
check_js: CheckJsOption::False, check_js: CheckJsOption::False,
kind: GraphKind::TypesOnly, kind: GraphKind::TypesOnly,
allow_unknown_media_types: false,
}, },
); );
for error in errors { 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 factory = CliFactory::from_flags(flags.clone());
let emitter = factory.emitter()?; let emitter = factory.emitter()?;
let main_graph_container = factory.main_module_graph_container().await?; let main_graph_container = factory.main_module_graph_container().await?;
let specifiers = main_graph_container.collect_specifiers(entrypoints)?;
main_graph_container 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?; .await?;
emitter emitter
.cache_module_emits(&main_graph_container.graph()) .cache_module_emits(&main_graph_container.graph())

View file

@ -136,7 +136,7 @@ pub async fn cache_top_level_deps(
}, },
) )
.await?; .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?; npm_installer.cache_packages(PackageCaching::All).await?;

View file

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

View file

@ -77,6 +77,7 @@ use crate::colors;
use crate::display; use crate::display;
use crate::factory::CliFactory; use crate::factory::CliFactory;
use crate::file_fetcher::CliFileFetcher; use crate::file_fetcher::CliFileFetcher;
use crate::graph_container::CheckSpecifiersOptions;
use crate::graph_util::has_graph_root_local_dependent_changed; use crate::graph_util::has_graph_root_local_dependent_changed;
use crate::ops; use crate::ops;
use crate::sys::CliSys; use crate::sys::CliSys;
@ -1604,7 +1605,10 @@ pub async fn run_tests(
main_graph_container main_graph_container
.check_specifiers( .check_specifiers(
&specifiers_for_typecheck_and_test, &specifiers_for_typecheck_and_test,
cli_options.ext_flag().as_ref(), CheckSpecifiersOptions {
ext_overwrite: cli_options.ext_flag().as_ref(),
..Default::default()
},
) )
.await?; .await?;
@ -1787,7 +1791,10 @@ pub async fn run_tests_with_watch(
main_graph_container main_graph_container
.check_specifiers( .check_specifiers(
&specifiers_for_typecheck_and_test, &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?; .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";