Skip to content

Commit c6cefce

Browse files
authored
fix(Turbopack): Fix duplicated layout rendering in edge cases (vercel#81948)
## Fix module reference handling in app_structure.rs creating duplicated layouts ### What? Fixed a bug in the `check_and_update_module_references` function by making the code always overrides the `not-found`, `unauthorized`, etc files at the top level group route layer. ### Why? The previous implementation had a bug that when a directory was setup like the added test case where the layouts from a lower directory would render when only the layouts at the level of the not-found page should have rendered. ### How? - Added the ability to force overriding of the `module` when running `check_and_update_module_references` - Split the logic between global and layer-specific modules Fixes #PACK-5081
1 parent e2b6a50 commit c6cefce

15 files changed

Lines changed: 174 additions & 9 deletions

File tree

crates/next-core/src/app_structure.rs

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -923,18 +923,31 @@ async fn directory_tree_to_loader_tree(
923923
/// set.
924924
/// * `file_path` - The file path to the default page if neither the current module nor the parent
925925
/// module is set.
926+
/// * `is_first_layer_group_route` - If true, the module will be overridden with the parent module
927+
/// if it is not set.
926928
async fn check_and_update_module_references(
927929
app_dir: FileSystemPath,
928930
module: &mut Option<FileSystemPath>,
929931
parent_module: &mut Option<FileSystemPath>,
930932
file_path: &str,
933+
is_first_layer_group_route: bool,
931934
) -> Result<()> {
932935
match (module.as_mut(), parent_module.as_mut()) {
936+
// If the module is set, update the parent module to the same value
933937
(Some(module), _) => *parent_module = Some(module.clone()),
934-
(None, Some(parent_module)) => *module = Some(parent_module.clone()),
938+
// If we are in a first layer group route and we have a parent module, we want to override
939+
// a nonexistent module with the parent module
940+
(None, Some(parent_module)) if is_first_layer_group_route => {
941+
*module = Some(parent_module.clone())
942+
}
943+
// If we are not in a first layer group route, and the module is not set, and the parent
944+
// module is set, we do nothing
945+
(None, Some(_)) => {}
946+
// If the module is not set, and the parent module is not set, we override with the default
947+
// page. This can only happen in the root directory because after this the parent module
948+
// will always be set.
935949
(None, None) => {
936-
let default_page = get_next_package(app_dir.clone()).await?.join(file_path)?;
937-
950+
let default_page = get_next_package(app_dir).await?.join(file_path)?;
938951
*module = Some(default_page.clone());
939952
*parent_module = Some(default_page);
940953
}
@@ -943,6 +956,25 @@ async fn check_and_update_module_references(
943956
Ok(())
944957
}
945958

959+
/// Checks if the current directory is the root directory and if the module is not set.
960+
/// If the module is not set, it will be set to the default page.
961+
///
962+
/// # Arguments
963+
/// * `app_dir` - The application directory.
964+
/// * `module` - The module to check and update if it is not set.
965+
/// * `file_path` - The file path to the default page if the module is not set.
966+
async fn check_and_update_global_module_references(
967+
app_dir: FileSystemPath,
968+
module: &mut Option<FileSystemPath>,
969+
file_path: &str,
970+
) -> Result<()> {
971+
if module.is_none() {
972+
*module = Some(get_next_package(app_dir).await?.join(file_path)?);
973+
}
974+
975+
Ok(())
976+
}
977+
946978
async fn directory_tree_to_loader_tree_internal(
947979
app_dir: FileSystemPath,
948980
global_metadata: Vc<GlobalMetadata>,
@@ -967,16 +999,19 @@ async fn directory_tree_to_loader_tree_internal(
967999

9681000
// the root directory in the app dir.
9691001
let is_root_directory = app_page.is_root();
970-
// an alternative root layout (in a route group which affects the page, but not
971-
// the path).
972-
let is_root_layout = app_path.is_root() && modules.layout.is_some();
9731002

974-
if is_root_directory || is_root_layout {
1003+
// If the first layer is a group route, we treat it as root layer
1004+
let is_first_layer_group_route = app_page.is_first_layer_group_route();
1005+
1006+
// Handle the non-global modules that should always be overridden for top level groups or set to
1007+
// the default page if they are not set.
1008+
if is_root_directory || is_first_layer_group_route {
9751009
check_and_update_module_references(
9761010
app_dir.clone(),
9771011
&mut modules.not_found,
9781012
&mut parent_modules.not_found,
9791013
"dist/client/components/builtin/not-found.js",
1014+
is_first_layer_group_route,
9801015
)
9811016
.await?;
9821017

@@ -985,6 +1020,7 @@ async fn directory_tree_to_loader_tree_internal(
9851020
&mut modules.forbidden,
9861021
&mut parent_modules.forbidden,
9871022
"dist/client/components/builtin/forbidden.js",
1023+
is_first_layer_group_route,
9881024
)
9891025
.await?;
9901026

@@ -993,13 +1029,15 @@ async fn directory_tree_to_loader_tree_internal(
9931029
&mut modules.unauthorized,
9941030
&mut parent_modules.unauthorized,
9951031
"dist/client/components/builtin/unauthorized.js",
1032+
is_first_layer_group_route,
9961033
)
9971034
.await?;
1035+
}
9981036

999-
check_and_update_module_references(
1037+
if is_root_directory {
1038+
check_and_update_global_module_references(
10001039
app_dir.clone(),
10011040
&mut modules.global_error,
1002-
&mut parent_modules.global_error,
10031041
"dist/client/components/builtin/global-error.js",
10041042
)
10051043
.await?;

crates/next-core/src/next_app/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,11 @@ impl AppPage {
293293
)
294294
}
295295

296+
/// Returns true if there is only one segment and it is a group.
297+
pub fn is_first_layer_group_route(&self) -> bool {
298+
self.0.len() == 1 && matches!(self.0.last(), Some(PageSegment::Group(_)))
299+
}
300+
296301
pub fn complete(&self, page_type: PageType) -> Result<Self> {
297302
self.clone_push(PageSegment::PageType(page_type))
298303
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import Header from '../../components/Header'
2+
import Footer from '../../components/Footer'
3+
4+
export default function DefaultLayout({ children }) {
5+
return (
6+
<>
7+
<Header />
8+
{children}
9+
<Footer />
10+
</>
11+
)
12+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { notFound } from 'next/navigation'
2+
3+
export default function NotFoundTemp() {
4+
notFound()
5+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
export default function SolutionsLayout({ children }) {
2+
return (
3+
<>
4+
<h1>Solutions Layout</h1>
5+
<main>{children}</main>
6+
<h1>Solutions Footer Test</h1>
7+
</>
8+
)
9+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
export default function Layout({ children }) {
2+
return (
3+
<>
4+
<h1>Default Layout</h1>
5+
{children}
6+
<h1>Default Footer Test</h1>
7+
</>
8+
)
9+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { NotFoundTemp } from '../components/NotFoundTemp'
2+
import DefaultLayout from './(default)/layout'
3+
4+
export default function NotFound() {
5+
return (
6+
<>
7+
<DefaultLayout>
8+
<NotFoundTemp />
9+
</DefaultLayout>
10+
</>
11+
)
12+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function Footer() {
2+
return (
3+
<footer id="footer">
4+
<h1>Footer</h1>
5+
</footer>
6+
)
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function Header() {
2+
return (
3+
<header>
4+
<h1 id="header">Header</h1>
5+
</header>
6+
)
7+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function NotFoundTemp() {
2+
return <div>Not Found</div>
3+
}

0 commit comments

Comments
 (0)