From 671ac35614167aab7810ca06fe2a3bbc4c1840ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donny/=EA=B0=95=EB=8F=99=EC=9C=A4?= Date: Wed, 24 Apr 2024 04:11:41 +0900 Subject: [PATCH] feat(turbopack): Check for duplicate parallel routes (#64181) ### What? Add a validation for parallel routes with the same path. This PR enables `test/e2e/app-dir/conflicting-page-segments/conflicting-page-segments.test.ts` ### Why? To match the behavior in the default mode. ### How? Closes PACK-2912 --- .../crates/next-core/src/app_structure.rs | 89 ++++++++++++++++++- test/turbopack-build-tests-manifest.json | 7 +- test/turbopack-dev-tests-manifest.json | 4 +- 3 files changed, 94 insertions(+), 6 deletions(-) diff --git a/packages/next-swc/crates/next-core/src/app_structure.rs b/packages/next-swc/crates/next-core/src/app_structure.rs index 744b633515..7fbe07e2e8 100644 --- a/packages/next-swc/crates/next-core/src/app_structure.rs +++ b/packages/next-swc/crates/next-core/src/app_structure.rs @@ -1,11 +1,13 @@ use std::collections::BTreeMap; use anyhow::{bail, Context, Result}; +use async_recursion::async_recursion; use indexmap::{ indexmap, map::{Entry, OccupiedEntry}, IndexMap, }; +use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; use tracing::Instrument; use turbo_tasks::{ @@ -25,7 +27,7 @@ use crate::{ match_global_metadata_file, match_local_metadata_file, normalize_metadata_route, GlobalMetadataFileMatch, MetadataFileMatch, }, - AppPage, AppPath, PageType, + AppPage, AppPath, PageSegment, PageType, }, next_config::NextConfig, next_import_map::get_next_package, @@ -709,6 +711,85 @@ fn directory_tree_to_entrypoints( ) } +#[turbo_tasks::value] +struct DuplicateParallelRouteIssue { + app_dir: Vc, + page: AppPage, +} + +#[turbo_tasks::value_impl] +impl Issue for DuplicateParallelRouteIssue { + #[turbo_tasks::function] + async fn file_path(self: Vc) -> Result> { + let this = self.await?; + Ok(this.app_dir.join(this.page.to_string())) + } + + #[turbo_tasks::function] + fn stage(self: Vc) -> Vc { + IssueStage::ProcessModule.cell() + } + + #[turbo_tasks::function] + fn title(self: Vc) -> Vc { + StyledString::Text( + "You cannot have two parallel pages that resolve to the same path.".to_string(), + ) + .cell() + } +} + +#[async_recursion] +async fn page_path_except_parallel(loader_tree: Vc) -> Result> { + let loader_tree = loader_tree.await?; + + if loader_tree.page.iter().any(|v| { + matches!( + v, + PageSegment::CatchAll(..) + | PageSegment::OptionalCatchAll(..) + | PageSegment::Parallel(..) + ) + }) { + return Ok(None); + } + + if loader_tree.components.await?.page.is_some() { + return Ok(Some(loader_tree.page.clone())); + } + + if let Some(children) = loader_tree.parallel_routes.get("children") { + return page_path_except_parallel(*children).await; + } + + Ok(None) +} + +async fn check_duplicate( + duplicate: &mut FxHashMap, + loader_tree_vc: Vc, + app_dir: Vc, +) -> Result<()> { + let loader_tree = loader_tree_vc.await?; + + let page_path = page_path_except_parallel(loader_tree_vc).await?; + + if let Some(page_path) = page_path { + if let Some(prev) = duplicate.insert(AppPath::from(page_path.clone()), page_path.clone()) { + if prev != page_path { + DuplicateParallelRouteIssue { + app_dir, + page: loader_tree.page.clone(), + } + .cell() + .emit(); + } + } + } + + Ok(()) +} + /// creates the loader tree for a specific route (pathname / [AppPath]) #[turbo_tasks::function] async fn directory_tree_to_loader_tree( @@ -802,6 +883,8 @@ async fn directory_tree_to_loader_tree( } } + let mut duplicate = FxHashMap::default(); + for (subdir_name, subdirectory) in &directory_tree.subdirectories { let parallel_route_key = match_parallel_route(subdir_name); @@ -841,6 +924,10 @@ async fn directory_tree_to_loader_tree( continue; } + if *subtree.has_page().await? { + check_duplicate(&mut duplicate, subtree, app_dir).await?; + } + if let Some(current_tree) = tree.parallel_routes.get("children") { if is_current_directory_catchall && *subtree.has_only_catchall().await? { // there's probably already a more specific page in the diff --git a/test/turbopack-build-tests-manifest.json b/test/turbopack-build-tests-manifest.json index 9f879798c5..6d0bb47e37 100644 --- a/test/turbopack-build-tests-manifest.json +++ b/test/turbopack-build-tests-manifest.json @@ -2232,7 +2232,8 @@ "parallel-routes-and-interception route intercepting should render modal when paired with parallel routes", "parallel-routes-and-interception parallel routes should display all parallel route params with useParams", "parallel-routes-and-interception route intercepting should render an intercepted route at the top level from a nested path", - "parallel-routes-and-interception route intercepting should render an intercepted route from a slot" + "parallel-routes-and-interception route intercepting should render an intercepted route from a slot", + "parallel-routes-and-interception route intercepting should render modal when paired with parallel routes" ], "pending": [], "flakey": [], @@ -2266,10 +2267,10 @@ "runtimeError": false }, "test/e2e/app-dir/parallel-routes-catchall-groups/parallel-routes-catchall-groups.test.ts": { - "passed": [ + "passed": [], + "failed": [ "parallel-routes-catchall-groups should work without throwing any errors about conflicting paths" ], - "failed": [], "pending": [], "flakey": [], "runtimeError": false diff --git a/test/turbopack-dev-tests-manifest.json b/test/turbopack-dev-tests-manifest.json index 4edd29dbd3..6425f64674 100644 --- a/test/turbopack-dev-tests-manifest.json +++ b/test/turbopack-dev-tests-manifest.json @@ -3341,10 +3341,10 @@ "runtimeError": false }, "test/e2e/app-dir/conflicting-page-segments/conflicting-page-segments.test.ts": { - "passed": [], - "failed": [ + "passed": [ "conflicting-page-segments should throw an error when a route groups causes a conflict with a parallel segment" ], + "failed": [], "pending": [], "flakey": [], "runtimeError": false