Skip to content

Commit cff6a5d

Browse files
mischnicmischnic
authored
Turbopack: remove duplicate traversal implementations (#85853)
We had two implementations of various traversals: first for the SingleModuleGraph, and then also the more general version that can cross module graphs. Let's just use the second for everything (with just a single graph in the case of `SingleModuleGraph`) This turned out to be a much bigger change than I had anticipated, but this was long overdue anyway.
1 parent 473ae4b commit cff6a5d

File tree

13 files changed

+460
-783
lines changed

13 files changed

+460
-783
lines changed

‎crates/next-api/src/analyze.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,10 +419,10 @@ pub async fn analyze_module_graphs(module_graphs: Vc<ModuleGraphs>) -> Result<Vc
419419
module_graph.traverse_all_edges_unordered(|(parent_node, reference), node| {
420420
match reference.chunking_type {
421421
ChunkingType::Async => {
422-
all_async_edges.insert((parent_node.module, node.module));
422+
all_async_edges.insert((parent_node, node));
423423
}
424424
_ => {
425-
all_edges.insert((parent_node.module, node.module));
425+
all_edges.insert((parent_node, node));
426426
}
427427
}
428428
Ok(())

‎crates/next-api/src/client_references.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,7 @@ pub async fn map_client_references(
3434
let graph = graph.await?;
3535
let manifest = graph
3636
.iter_nodes()
37-
.map(|node| async move {
38-
let module = node.module;
39-
37+
.map(|module| async move {
4038
if let Some(client_reference_module) =
4139
ResolvedVc::try_downcast_type::<EcmascriptClientReferenceModule>(module)
4240
{

‎crates/next-api/src/dynamic_imports.rs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ use turbopack_core::{
3535
availability_info::AvailabilityInfo,
3636
},
3737
module::Module,
38-
module_graph::{ModuleGraph, SingleModuleGraph, SingleModuleGraphModuleNode},
38+
module_graph::{ModuleGraph, SingleModuleGraph},
3939
output::OutputAssets,
4040
};
4141

@@ -125,30 +125,28 @@ pub async fn map_next_dynamic(graph: Vc<SingleModuleGraph>) -> Result<Vc<Dynamic
125125
let actions = graph
126126
.await?
127127
.iter_nodes()
128-
.map(|node| async move {
129-
let SingleModuleGraphModuleNode { module } = node;
130-
128+
.map(|module| async move {
131129
if module
132130
.ident()
133131
.await?
134132
.layer
135133
.as_ref()
136134
.is_some_and(|layer| layer.name() == "app-client" || layer.name() == "client")
137135
&& let Some(dynamic_entry_module) =
138-
ResolvedVc::try_downcast_type::<NextDynamicEntryModule>(*module)
136+
ResolvedVc::try_downcast_type::<NextDynamicEntryModule>(module)
139137
{
140138
return Ok(Some((
141-
*module,
139+
module,
142140
DynamicImportEntriesMapType::DynamicEntry(dynamic_entry_module),
143141
)));
144142
}
145143
// TODO add this check once these modules have the correct layer
146144
// if layer.is_some_and(|layer| &**layer == "app-rsc") {
147145
if let Some(client_reference_module) =
148-
ResolvedVc::try_downcast_type::<EcmascriptClientReferenceModule>(*module)
146+
ResolvedVc::try_downcast_type::<EcmascriptClientReferenceModule>(module)
149147
{
150148
return Ok(Some((
151-
*module,
149+
module,
152150
DynamicImportEntriesMapType::ClientReference(client_reference_module),
153151
)));
154152
}

‎crates/next-api/src/module_graph.rs‎

Lines changed: 108 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ impl NextDynamicGraph {
7474
let span = tracing::info_span!("collect next/dynamic imports for endpoint");
7575
async move {
7676
let data = &*self.data.await?;
77-
let graph = &*self.graph.await?;
77+
let graph = self.graph.await?;
7878

7979
#[derive(Clone, PartialEq, Eq)]
8080
enum VisitState {
@@ -96,48 +96,56 @@ impl NextDynamicGraph {
9696

9797
// module -> the client reference entry (if any)
9898
let mut state_map = FxHashMap::default();
99-
graph.traverse_edges_from_entries(entries, |parent_info, node| {
100-
let module = node.module;
101-
let Some((parent_node, _)) = parent_info else {
102-
state_map.insert(module, VisitState::Entry);
103-
return GraphTraversalAction::Continue;
104-
};
105-
let parent_module = parent_node.module;
106-
107-
let module_type = data.get(&module);
108-
let parent_state = state_map.get(&parent_module).unwrap().clone();
109-
let parent_client_reference =
110-
if let Some(DynamicImportEntriesMapType::ClientReference(module)) = module_type
111-
{
112-
Some(ClientReferenceType::EcmascriptClientReference(*module))
113-
} else if let VisitState::InClientReference(ty) = parent_state {
114-
Some(ty)
115-
} else {
116-
None
99+
graph.read().traverse_edges_from_entries_dfs(
100+
entries,
101+
&mut (),
102+
|parent_info, node, _| {
103+
let module = node;
104+
let Some((parent_node, _)) = parent_info else {
105+
state_map.insert(module, VisitState::Entry);
106+
return Ok(GraphTraversalAction::Continue);
117107
};
108+
let parent_module = parent_node;
118109

119-
match module_type {
120-
Some(DynamicImportEntriesMapType::DynamicEntry(dynamic_entry)) => {
121-
result.push((*dynamic_entry, parent_client_reference));
110+
let module_type = data.get(&module);
111+
let parent_state = state_map.get(&parent_module).unwrap().clone();
112+
let parent_client_reference =
113+
if let Some(DynamicImportEntriesMapType::ClientReference(module)) =
114+
module_type
115+
{
116+
Some(ClientReferenceType::EcmascriptClientReference(*module))
117+
} else if let VisitState::InClientReference(ty) = parent_state {
118+
Some(ty)
119+
} else {
120+
None
121+
};
122122

123-
state_map.insert(module, parent_state);
124-
GraphTraversalAction::Skip
125-
}
126-
Some(DynamicImportEntriesMapType::ClientReference(client_reference)) => {
127-
state_map.insert(
128-
module,
129-
VisitState::InClientReference(
130-
ClientReferenceType::EcmascriptClientReference(*client_reference),
131-
),
132-
);
133-
GraphTraversalAction::Continue
134-
}
135-
None => {
136-
state_map.insert(module, parent_state);
137-
GraphTraversalAction::Continue
138-
}
139-
}
140-
})?;
123+
Ok(match module_type {
124+
Some(DynamicImportEntriesMapType::DynamicEntry(dynamic_entry)) => {
125+
result.push((*dynamic_entry, parent_client_reference));
126+
127+
state_map.insert(module, parent_state);
128+
GraphTraversalAction::Skip
129+
}
130+
Some(DynamicImportEntriesMapType::ClientReference(client_reference)) => {
131+
state_map.insert(
132+
module,
133+
VisitState::InClientReference(
134+
ClientReferenceType::EcmascriptClientReference(
135+
*client_reference,
136+
),
137+
),
138+
);
139+
GraphTraversalAction::Continue
140+
}
141+
None => {
142+
state_map.insert(module, parent_state);
143+
GraphTraversalAction::Continue
144+
}
145+
})
146+
},
147+
|_, _, _| Ok(()),
148+
)?;
141149
Ok(Vc::cell(result))
142150
}
143151
.instrument(span)
@@ -184,19 +192,25 @@ impl ServerActionsGraph {
184192
Cow::Borrowed(data)
185193
} else {
186194
// The graph contains the whole app, traverse and collect all reachable imports.
187-
let graph = &*self.graph.await?;
195+
let graph = self.graph.await?;
188196

189197
if !graph.has_entry_module(entry) {
190198
// the graph doesn't contain the entry, e.g. for the additional module graph
191199
return Ok(Vc::cell(Default::default()));
192200
}
193201

194202
let mut result = FxIndexMap::default();
195-
graph.traverse_from_entry(entry, |node| {
196-
if let Some(node_data) = data.get(&node.module) {
197-
result.insert(node.module, *node_data);
198-
}
199-
})?;
203+
graph.read().traverse_nodes_from_entries_dfs(
204+
vec![entry],
205+
&mut result,
206+
|node, result| {
207+
if let Some(node_data) = data.get(&node) {
208+
result.insert(node, *node_data);
209+
}
210+
Ok(GraphTraversalAction::Continue)
211+
},
212+
|_, _| Ok(()),
213+
)?;
200214
Cow::Owned(result)
201215
};
202216

@@ -274,7 +288,7 @@ impl ClientReferencesGraph {
274288
let span = tracing::info_span!("collect client references for endpoint");
275289
async move {
276290
let data = &*self.data.await?;
277-
let graph = &*self.graph.await?;
291+
let graph = self.graph.await?;
278292

279293
let entries = if !self.is_single_page {
280294
if !graph.has_entry_module(entry) {
@@ -293,12 +307,13 @@ impl ClientReferencesGraph {
293307

294308
let mut server_components = FxIndexSet::default();
295309

310+
let graph = graph.read();
296311
// Perform a DFS traversal to find all server components included by this page.
297-
graph.traverse_nodes_from_entries(
312+
graph.traverse_nodes_from_entries_dfs(
298313
entries,
299314
&mut (),
300315
|node, _| {
301-
let module_type = data.get(&node.module);
316+
let module_type = data.get(&node);
302317
Ok(match module_type {
303318
Some(
304319
ClientManifestEntryType::EcmascriptClientReference { .. }
@@ -310,14 +325,14 @@ impl ClientReferencesGraph {
310325
},
311326
|node, _| {
312327
if let Some(server_util_module) =
313-
ResolvedVc::try_downcast_type::<NextServerUtilityModule>(node.module)
328+
ResolvedVc::try_downcast_type::<NextServerUtilityModule>(node)
314329
{
315330
// Server utility used by the template, not a server component
316331
server_utils.insert(server_util_module);
317332
return Ok(());
318333
}
319334

320-
let module_type = data.get(&node.module);
335+
let module_type = data.get(&node);
321336

322337
let ty = match module_type {
323338
Some(ClientManifestEntryType::EcmascriptClientReference {
@@ -350,11 +365,11 @@ impl ClientReferencesGraph {
350365
// necessarily rendered at the same time (not-found, or parallel routes), we need to
351366
// determine the order of client references individually for each server component.
352367
for sc in server_components.iter().copied() {
353-
graph.traverse_nodes_from_entries(
368+
graph.traverse_nodes_from_entries_dfs(
354369
std::iter::once(ResolvedVc::upcast(sc)),
355370
&mut (),
356371
|node, _| {
357-
let module = node.module;
372+
let module = node;
358373
let module_type = data.get(&module);
359374

360375
Ok(match module_type {
@@ -366,7 +381,7 @@ impl ClientReferencesGraph {
366381
})
367382
},
368383
|node, _| {
369-
let module = node.module;
384+
let module = node;
370385
if let Some(server_util_module) =
371386
ResolvedVc::try_downcast_type::<NextServerUtilityModule>(module)
372387
{
@@ -506,7 +521,7 @@ async fn validate_pages_css_imports(
506521
entry: Vc<Box<dyn Module>>,
507522
app_module: ResolvedVc<Box<dyn Module>>,
508523
) -> Result<()> {
509-
let graph = &*graph.await?;
524+
let graph = graph.await?;
510525
let entry = entry.to_resolved().await?;
511526

512527
let entries = if !is_single_page {
@@ -521,47 +536,52 @@ async fn validate_pages_css_imports(
521536

522537
let mut candidates = vec![];
523538

524-
graph.traverse_edges_from_entries(entries, |parent_info, node| {
525-
let module = node.module;
539+
graph.read().traverse_edges_from_entries_dfs(
540+
entries,
541+
&mut (),
542+
|parent_info, node, _| {
543+
let module = node;
526544

527-
// If we're at a root node, there is nothing importing this module and we can skip
528-
// any further validations.
529-
let Some((parent_node, _)) = parent_info else {
530-
return GraphTraversalAction::Continue;
531-
};
532-
let parent_module = parent_node.module;
545+
// If we're at a root node, there is nothing importing this module and we can skip
546+
// any further validations.
547+
let Some((parent_node, _)) = parent_info else {
548+
return Ok(GraphTraversalAction::Continue);
549+
};
550+
let parent_module = parent_node;
533551

534-
// Importing CSS from _app.js is always allowed.
535-
if parent_module == app_module {
536-
return GraphTraversalAction::Continue;
537-
}
552+
// Importing CSS from _app.js is always allowed.
553+
if parent_module == app_module {
554+
return Ok(GraphTraversalAction::Continue);
555+
}
538556

539-
// If the module being imported isn't a global css module, there is nothing to validate.
540-
let module_is_global_css =
541-
ResolvedVc::try_downcast_type::<CssModuleAsset>(module).is_some();
557+
// If the module being imported isn't a global css module, there is nothing to validate.
558+
let module_is_global_css =
559+
ResolvedVc::try_downcast_type::<CssModuleAsset>(module).is_some();
542560

543-
if !module_is_global_css {
544-
return GraphTraversalAction::Continue;
545-
}
561+
if !module_is_global_css {
562+
return Ok(GraphTraversalAction::Continue);
563+
}
546564

547-
let parent_is_css_module = ResolvedVc::try_downcast_type::<ModuleCssAsset>(parent_module)
548-
.is_some()
549-
|| ResolvedVc::try_downcast_type::<CssModuleAsset>(parent_module).is_some();
565+
let parent_is_css_module =
566+
ResolvedVc::try_downcast_type::<ModuleCssAsset>(parent_module).is_some()
567+
|| ResolvedVc::try_downcast_type::<CssModuleAsset>(parent_module).is_some();
550568

551-
// We also always allow .module css/scss/sass files to import global css files as well.
552-
if parent_is_css_module {
553-
return GraphTraversalAction::Continue;
554-
}
569+
// We also always allow .module css/scss/sass files to import global css files as well.
570+
if parent_is_css_module {
571+
return Ok(GraphTraversalAction::Continue);
572+
}
555573

556-
// If all of the above invariants have been checked, we look to see if the parent module is
557-
// the same as the app module. If it isn't we know it isn't a valid place to import global
558-
// css.
559-
if parent_module != app_module {
560-
candidates.push(CssGlobalImportIssue::new(parent_module, module))
561-
}
574+
// If all of the above invariants have been checked, we look to see if the parent module
575+
// is the same as the app module. If it isn't we know it isn't a valid place
576+
// to import global css.
577+
if parent_module != app_module {
578+
candidates.push(CssGlobalImportIssue::new(parent_module, module))
579+
}
562580

563-
GraphTraversalAction::Continue
564-
})?;
581+
Ok(GraphTraversalAction::Continue)
582+
},
583+
|_, _, _| Ok(()),
584+
)?;
565585

566586
candidates
567587
.into_iter()

0 commit comments

Comments
 (0)