Skip to content

Commit 6dc7c3c

Browse files
fix(turbopack): don't match empty route groups (#57647)
### What? Previously the matching just used the last match for children which could lead to empty groups or groups without page matches overriding the actual page. This PR makes sure this doesn't happen and emits an issue (which unfortunately doesn't get displayed yet) in case there are 2 `page` matches which would go into the children slot. Closes WEB-1895
1 parent 9d49afc commit 6dc7c3c

File tree

1 file changed

+43
-7
lines changed

1 file changed

+43
-7
lines changed

packages/next-swc/crates/next-core/src/app_structure.rs

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,25 @@ pub struct LoaderTree {
400400
pub global_metadata: Vc<GlobalMetadata>,
401401
}
402402

403+
#[turbo_tasks::value_impl]
404+
impl LoaderTree {
405+
/// Returns true if there's a page match in this loader tree.
406+
#[turbo_tasks::function]
407+
pub async fn has_page(&self) -> Result<Vc<bool>> {
408+
if self.segment == "__PAGE__" {
409+
return Ok(Vc::cell(true));
410+
}
411+
412+
for (_, tree) in &self.parallel_routes {
413+
if *tree.has_page().await? {
414+
return Ok(Vc::cell(true));
415+
}
416+
}
417+
418+
Ok(Vc::cell(false))
419+
}
420+
}
421+
403422
#[derive(
404423
Clone, PartialEq, Eq, Serialize, Deserialize, TraceRawVcs, ValueDebugFormat, Debug, TaskInput,
405424
)]
@@ -425,6 +444,10 @@ fn is_parallel_route(name: &str) -> bool {
425444
name.starts_with('@')
426445
}
427446

447+
fn is_group_route(name: &str) -> bool {
448+
name.starts_with('(') && name.ends_with(')')
449+
}
450+
428451
fn match_parallel_route(name: &str) -> Option<&str> {
429452
name.strip_prefix('@')
430453
}
@@ -677,14 +700,10 @@ async fn directory_tree_to_loader_tree(
677700
tree.segment = "children".to_string();
678701
}
679702

680-
let mut has_page = false;
681-
682703
if let Some(page) = (app_path == for_app_path)
683704
.then_some(components.page)
684705
.flatten()
685706
{
686-
has_page = true;
687-
688707
// When resolving metadata with corresponding module
689708
// (https://github.com/vercel/next.js/blob/aa1ee5995cdd92cc9a2236ce4b6aa2b67c9d32b2/packages/next/src/lib/metadata/resolve-metadata.ts#L340)
690709
// layout takes precedence over page (https://github.com/vercel/next.js/blob/aa1ee5995cdd92cc9a2236ce4b6aa2b67c9d32b2/packages/next/src/server/lib/app-dir-module.ts#L22)
@@ -751,9 +770,26 @@ async fn directory_tree_to_loader_tree(
751770
continue;
752771
}
753772

754-
// TODO: detect duplicate page in group segment
755-
if !has_page {
773+
// skip groups which don't have a page match.
774+
if is_group_route(subdir_name) && !*subtree.has_page().await? {
775+
continue;
776+
}
777+
778+
if !tree.parallel_routes.contains_key("children") {
756779
tree.parallel_routes.insert("children".to_string(), subtree);
780+
} else {
781+
// TODO: improve error message to have the full paths
782+
DirectoryTreeIssue {
783+
app_dir,
784+
message: Vc::cell(format!(
785+
"You cannot have two parallel pages that resolve to the same path. Route \
786+
{} has multiple matches in {}",
787+
for_app_path, app_page
788+
)),
789+
severity: IssueSeverity::Error.cell(),
790+
}
791+
.cell()
792+
.emit();
757793
}
758794
} else if let Some(key) = parallel_route_key {
759795
bail!(
@@ -772,7 +808,7 @@ async fn directory_tree_to_loader_tree(
772808
..Default::default()
773809
}
774810
.cell();
775-
} else if components.layout.is_some() || current_level_is_parallel_route {
811+
} else if current_level_is_parallel_route {
776812
// default fallback component
777813
tree.components = Components {
778814
default: Some(

0 commit comments

Comments
 (0)