Skip to content

Commit fd9f0d1

Browse files
authored
Turbopack: validate CSS without computing all paths (#83810)
Don't compute all paths of all modules beforehand (and store them in a vector) for every single endpoint Instead, compute the list of all potentially problematic modules on demand. Closes PACK-5455
1 parent 1300acf commit fd9f0d1

File tree

1 file changed

+54
-70
lines changed

1 file changed

+54
-70
lines changed

crates/next-api/src/module_graph.rs

Lines changed: 54 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ use rustc_hash::FxHashMap;
1616
use tracing::Instrument;
1717
use turbo_rcstr::{RcStr, rcstr};
1818
use turbo_tasks::{
19-
CollectiblesSource, FxIndexMap, FxIndexSet, ResolvedVc, TryFlatJoinIterExt, TryJoinIterExt,
20-
ValueToString, Vc,
19+
CollectiblesSource, FxIndexMap, FxIndexSet, ResolvedVc, TryFlatJoinIterExt, TryJoinIterExt, Vc,
2120
};
2221
use turbo_tasks_fs::FileSystemPath;
2322
use turbopack::css::{CssModuleAsset, ModuleCssAsset};
@@ -429,8 +428,8 @@ impl ClientReferencesGraph {
429428

430429
#[turbo_tasks::value(shared)]
431430
struct CssGlobalImportIssue {
432-
parent_module: ResolvedVc<Box<dyn Module>>,
433-
module: ResolvedVc<Box<dyn Module>>,
431+
pub parent_module: ResolvedVc<Box<dyn Module>>,
432+
pub module: ResolvedVc<Box<dyn Module>>,
434433
}
435434

436435
impl CssGlobalImportIssue {
@@ -512,17 +511,16 @@ type FxModuleNameMap = FxIndexMap<ResolvedVc<Box<dyn Module>>, RcStr>;
512511
#[turbo_tasks::value(transparent)]
513512
struct ModuleNameMap(pub FxModuleNameMap);
514513

514+
#[tracing::instrument(level = tracing::Level::INFO, name = "validate pages css imports", skip_all)]
515515
#[turbo_tasks::function]
516516
async fn validate_pages_css_imports(
517517
graph: Vc<SingleModuleGraph>,
518518
is_single_page: bool,
519519
entry: Vc<Box<dyn Module>>,
520520
app_module: ResolvedVc<Box<dyn Module>>,
521-
module_name_map: ResolvedVc<ModuleNameMap>,
522521
) -> Result<()> {
523522
let graph = &*graph.await?;
524523
let entry = entry.to_resolved().await?;
525-
let module_name_map = module_name_map.await?;
526524

527525
let entries = if !is_single_page {
528526
if !graph.has_entry_module(entry) {
@@ -534,33 +532,31 @@ async fn validate_pages_css_imports(
534532
Either::Right(graph.entry_modules())
535533
};
536534

535+
let mut candidates = vec![];
536+
537537
graph.traverse_edges_from_entries(entries, |parent_info, node| {
538538
let module = node.module;
539539

540-
// If the module being imported isn't a global css module, there is nothing to validate.
541-
let module_is_global_css =
542-
ResolvedVc::try_downcast_type::<CssModuleAsset>(module).is_some();
540+
// If we're at a root node, there is nothing importing this module and we can skip
541+
// any further validations.
542+
let Some((parent_node, _)) = parent_info else {
543+
return GraphTraversalAction::Continue;
544+
};
545+
let parent_module = parent_node.module;
543546

544-
if !module_is_global_css {
547+
// Importing CSS from _app.js is always allowed.
548+
if parent_module == app_module {
545549
return GraphTraversalAction::Continue;
546550
}
547551

548-
// We allow imports of global CSS files which are inside of `node_modules`.
549-
let module_name_contains_node_modules = module_name_map
550-
.get(&module)
551-
.is_some_and(|s| s.contains("node_modules"));
552+
// If the module being imported isn't a global css module, there is nothing to validate.
553+
let module_is_global_css =
554+
ResolvedVc::try_downcast_type::<CssModuleAsset>(module).is_some();
552555

553-
if module_name_contains_node_modules {
556+
if !module_is_global_css {
554557
return GraphTraversalAction::Continue;
555558
}
556559

557-
// If we're at a root node, there is nothing importing this module and we can skip
558-
// any further validations.
559-
let Some((parent_node, _)) = parent_info else {
560-
return GraphTraversalAction::Continue;
561-
};
562-
563-
let parent_module = parent_node.module;
564560
let parent_is_css_module = ResolvedVc::try_downcast_type::<ModuleCssAsset>(parent_module)
565561
.is_some()
566562
|| ResolvedVc::try_downcast_type::<CssModuleAsset>(parent_module).is_some();
@@ -574,14 +570,38 @@ async fn validate_pages_css_imports(
574570
// the same as the app module. If it isn't we know it isn't a valid place to import global
575571
// css.
576572
if parent_module != app_module {
577-
CssGlobalImportIssue::new(parent_module, module)
578-
.resolved_cell()
579-
.emit();
573+
candidates.push(CssGlobalImportIssue::new(parent_module, module))
580574
}
581575

582576
GraphTraversalAction::Continue
583577
})?;
584578

579+
candidates
580+
.into_iter()
581+
.map(async |issue| {
582+
// We allow imports of global CSS files which are inside of `node_modules`.
583+
Ok(
584+
if !issue
585+
.module
586+
.ident()
587+
.path()
588+
.await?
589+
.path
590+
.contains("/node_modules/")
591+
{
592+
Some(issue)
593+
} else {
594+
None
595+
},
596+
)
597+
})
598+
.try_flat_join()
599+
.await?
600+
.into_iter()
601+
.for_each(|issue| {
602+
issue.resolved_cell().emit();
603+
});
604+
585605
Ok(())
586606
}
587607

@@ -785,54 +805,18 @@ impl GlobalBuildInformation {
785805
entry: Vc<Box<dyn Module>>,
786806
app_module: Vc<Box<dyn Module>>,
787807
) -> Result<()> {
788-
let span = tracing::info_span!("validate pages css imports");
789-
async move {
790-
let graphs = &self.bare_graphs.await?.graphs;
791-
792-
// We need to collect the module names here to pass into the
793-
// `validate_pages_css_imports` function. This is because the function is
794-
// called for each graph, and we need to know the module names of the parent
795-
// modules to determine if the import is valid. We can't do this in the
796-
// called function because it's within a closure that can't resolve turbo tasks.
797-
let graph_to_module_ident_tuples = async |graph: &ResolvedVc<SingleModuleGraph>| {
798-
graph
799-
.await?
800-
.graph
801-
.node_weights()
802-
.map(async |n| Ok((n.module(), n.module().ident().to_string().owned().await?)))
803-
.try_join()
804-
.await
805-
};
806-
807-
let identifier_map = graphs
808-
.iter()
809-
.map(graph_to_module_ident_tuples)
810-
.try_join()
811-
.await?
812-
.into_iter()
813-
.flatten()
814-
.collect::<FxIndexMap<_, _>>();
815-
let identifier_map = ModuleNameMap(identifier_map).cell();
808+
let graphs = &self.bare_graphs.await?.graphs;
816809

817-
graphs
818-
.iter()
819-
.map(|graph| {
820-
validate_pages_css_imports(
821-
**graph,
822-
self.is_single_page,
823-
entry,
824-
app_module,
825-
identifier_map,
826-
)
810+
graphs
811+
.iter()
812+
.map(|graph| {
813+
validate_pages_css_imports(**graph, self.is_single_page, entry, app_module)
827814
.as_side_effect()
828-
})
829-
.try_join()
830-
.await?;
815+
})
816+
.try_join()
817+
.await?;
831818

832-
Ok(())
833-
}
834-
.instrument(span)
835-
.await
819+
Ok(())
836820
}
837821
}
838822

0 commit comments

Comments
 (0)