Repository: vercel/next.js
Pull request: Turbopack: validate CSS without computing all paths

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

Changed files:
filename: crates/next-api/src/module_graph.rs
patch: @@ -16,8 +16,7 @@ use rustc_hash::FxHashMap;
 use tracing::Instrument;
 use turbo_rcstr::{RcStr, rcstr};
 use turbo_tasks::{
-    CollectiblesSource, FxIndexMap, FxIndexSet, ResolvedVc, TryFlatJoinIterExt, TryJoinIterExt,
-    ValueToString, Vc,
+    CollectiblesSource, FxIndexMap, FxIndexSet, ResolvedVc, TryFlatJoinIterExt, TryJoinIterExt, Vc,
 };
 use turbo_tasks_fs::FileSystemPath;
 use turbopack::css::{CssModuleAsset, ModuleCssAsset};
@@ -429,8 +428,8 @@ impl ClientReferencesGraph {
 
 #[turbo_tasks::value(shared)]
 struct CssGlobalImportIssue {
-    parent_module: ResolvedVc<Box<dyn Module>>,
-    module: ResolvedVc<Box<dyn Module>>,
+    pub parent_module: ResolvedVc<Box<dyn Module>>,
+    pub module: ResolvedVc<Box<dyn Module>>,
 }
 
 impl CssGlobalImportIssue {
@@ -512,17 +511,16 @@ type FxModuleNameMap = FxIndexMap<ResolvedVc<Box<dyn Module>>, RcStr>;
 #[turbo_tasks::value(transparent)]
 struct ModuleNameMap(pub FxModuleNameMap);
 
+#[tracing::instrument(level = tracing::Level::INFO, name = "validate pages css imports", skip_all)]
 #[turbo_tasks::function]
 async fn validate_pages_css_imports(
     graph: Vc<SingleModuleGraph>,
     is_single_page: bool,
     entry: Vc<Box<dyn Module>>,
     app_module: ResolvedVc<Box<dyn Module>>,
-    module_name_map: ResolvedVc<ModuleNameMap>,
 ) -> Result<()> {
     let graph = &*graph.await?;
     let entry = entry.to_resolved().await?;
-    let module_name_map = module_name_map.await?;
 
     let entries = if !is_single_page {
         if !graph.has_entry_module(entry) {
@@ -534,33 +532,31 @@ async fn validate_pages_css_imports(
         Either::Right(graph.entry_modules())
     };
 
+    let mut candidates = vec![];
+
     graph.traverse_edges_from_entries(entries, |parent_info, node| {
         let module = node.module;
 
-        // If the module being imported isn't a global css module, there is nothing to validate.
-        let module_is_global_css =
-            ResolvedVc::try_downcast_type::<CssModuleAsset>(module).is_some();
+        // If we're at a root node, there is nothing importing this module and we can skip
+        // any further validations.
+        let Some((parent_node, _)) = parent_info else {
+            return GraphTraversalAction::Continue;
+        };
+        let parent_module = parent_node.module;
 
-        if !module_is_global_css {
+        // Importing CSS from _app.js is always allowed.
+        if parent_module == app_module {
             return GraphTraversalAction::Continue;
         }
 
-        // We allow imports of global CSS files which are inside of `node_modules`.
-        let module_name_contains_node_modules = module_name_map
-            .get(&module)
-            .is_some_and(|s| s.contains("node_modules"));
+        // If the module being imported isn't a global css module, there is nothing to validate.
+        let module_is_global_css =
+            ResolvedVc::try_downcast_type::<CssModuleAsset>(module).is_some();
 
-        if module_name_contains_node_modules {
+        if !module_is_global_css {
             return GraphTraversalAction::Continue;
         }
 
-        // If we're at a root node, there is nothing importing this module and we can skip
-        // any further validations.
-        let Some((parent_node, _)) = parent_info else {
-            return GraphTraversalAction::Continue;
-        };
-
-        let parent_module = parent_node.module;
         let parent_is_css_module = ResolvedVc::try_downcast_type::<ModuleCssAsset>(parent_module)
             .is_some()
             || ResolvedVc::try_downcast_type::<CssModuleAsset>(parent_module).is_some();
@@ -574,14 +570,38 @@ async fn validate_pages_css_imports(
         // the same as the app module. If it isn't we know it isn't a valid place to import global
         // css.
         if parent_module != app_module {
-            CssGlobalImportIssue::new(parent_module, module)
-                .resolved_cell()
-                .emit();
+            candidates.push(CssGlobalImportIssue::new(parent_module, module))
         }
 
         GraphTraversalAction::Continue
     })?;
 
+    candidates
+        .into_iter()
+        .map(async |issue| {
+            // We allow imports of global CSS files which are inside of `node_modules`.
+            Ok(
+                if !issue
+                    .module
+                    .ident()
+                    .path()
+                    .await?
+                    .path
+                    .contains("/node_modules/")
+                {
+                    Some(issue)
+                } else {
+                    None
+                },
+            )
+        })
+        .try_flat_join()
+        .await?
+        .into_iter()
+        .for_each(|issue| {
+            issue.resolved_cell().emit();
+        });
+
     Ok(())
 }
 
@@ -785,54 +805,18 @@ impl GlobalBuildInformation {
         entry: Vc<Box<dyn Module>>,
         app_module: Vc<Box<dyn Module>>,
     ) -> Result<()> {
-        let span = tracing::info_span!("validate pages css imports");
-        async move {
-            let graphs = &self.bare_graphs.await?.graphs;
-
-            // We need to collect the module names here to pass into the
-            // `validate_pages_css_imports` function. This is because the function is
-            // called for each graph, and we need to know the module names of the parent
-            // modules to determine if the import is valid. We can't do this in the
-            // called function because it's within a closure that can't resolve turbo tasks.
-            let graph_to_module_ident_tuples = async |graph: &ResolvedVc<SingleModuleGraph>| {
-                graph
-                    .await?
-                    .graph
-                    .node_weights()
-                    .map(async |n| Ok((n.module(), n.module().ident().to_string().owned().await?)))
-                    .try_join()
-                    .await
-            };
-
-            let identifier_map = graphs
-                .iter()
-                .map(graph_to_module_ident_tuples)
-                .try_join()
-                .await?
-                .into_iter()
-                .flatten()
-                .collect::<FxIndexMap<_, _>>();
-            let identifier_map = ModuleNameMap(identifier_map).cell();
+        let graphs = &self.bare_graphs.await?.graphs;
 
-            graphs
-                .iter()
-                .map(|graph| {
-                    validate_pages_css_imports(
-                        **graph,
-                        self.is_single_page,
-                        entry,
-                        app_module,
-                        identifier_map,
-                    )
+        graphs
+            .iter()
+            .map(|graph| {
+                validate_pages_css_imports(**graph, self.is_single_page, entry, app_module)
                     .as_side_effect()
-                })
-                .try_join()
-                .await?;
+            })
+            .try_join()
+            .await?;
 
-            Ok(())
-        }
-        .instrument(span)
-        .await
+        Ok(())
     }
 }