From 7eccedc2f97b49d0d6ba6d49b5f3ce256471c991 Mon Sep 17 00:00:00 2001 From: hrmny <8845940+ForsakenHarmony@users.noreply.github.com> Date: Tue, 17 Sep 2024 15:01:14 +0200 Subject: [PATCH] fix(turbopack): intercepting routes should have default slots (#69929) ### What? Intercepting routes should only override the slot they are in and not affect other parts of the tree. This PR makes sure we replace all other slots with the default route. Closes PACK-3219 Fixes #69299 Closes #69575 --- crates/next-core/src/app_structure.rs | 118 ++++++++++++------ crates/next-core/src/next_app/mod.rs | 17 +++ .../app/@modal/(.)cart/page.tsx | 7 ++ .../app/@modal/[...segments]/page.tsx | 12 ++ .../app/@modal/default.tsx | 7 ++ .../app/@slot/[...segments]/page.tsx | 7 ++ .../app/@slot/page.tsx | 7 ++ .../app/[...segments]/page.tsx | 7 ++ .../app/cart/page.tsx | 7 ++ .../app/layout.tsx | 30 +++++ .../app/page.tsx | 12 ++ .../next.config.js | 6 + ...l-routes-and-interception-catchall.test.ts | 41 ++++++ 13 files changed, 241 insertions(+), 37 deletions(-) create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/(.)cart/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/[...segments]/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/default.tsx create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@slot/[...segments]/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@slot/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/app/[...segments]/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/app/cart/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/app/layout.tsx create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/app/page.tsx create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/next.config.js create mode 100644 test/e2e/app-dir/parallel-routes-and-interception-catchall/parallel-routes-and-interception-catchall.test.ts diff --git a/crates/next-core/src/app_structure.rs b/crates/next-core/src/app_structure.rs index cacbffda9b6e4..4f6bafbed457a 100644 --- a/crates/next-core/src/app_structure.rs +++ b/crates/next-core/src/app_structure.rs @@ -422,7 +422,7 @@ impl LoaderTree { false } - /// Returns whether or not the only match in this tree is for a catch-all + /// Returns whether the only match in this tree is for a catch-all /// route. pub fn has_only_catchall(&self) -> bool { if &*self.segment == "__PAGE__" && !self.page.is_catchall() { @@ -438,6 +438,21 @@ impl LoaderTree { true } + /// Returns true if this loader tree contains an intercepting route match. + pub fn is_intercepting(&self) -> bool { + if self.page.is_intercepting() && self.has_page() { + return true; + } + + for (_, tree) in &self.parallel_routes { + if tree.is_intercepting() { + return true; + } + } + + false + } + /// Returns the specificity of the page (i.e. the number of segments /// affecting the path) pub fn get_specificity(&self) -> usize { @@ -940,49 +955,49 @@ fn directory_tree_to_loader_tree_internal( } } - if tree.parallel_routes.is_empty() { - tree.segment = "__DEFAULT__".into(); - if let Some(default) = components.default { - tree.components = Components { - default: Some(default), - ..Default::default() - }; - } else if current_level_is_parallel_route { - // default fallback component - tree.components = Components { - default: Some( - get_next_package(app_dir) - .join("dist/client/components/parallel-route-default.js".into()), - ), - ..Default::default() + // make sure we don't have a match for other slots if there's an intercepting route match + // we only check subtrees as the current level could trigger `is_intercepting` + if tree + .parallel_routes + .iter() + .any(|(_, parallel_tree)| parallel_tree.is_intercepting()) + { + let mut keys_to_replace = Vec::new(); + + for (key, parallel_tree) in &tree.parallel_routes { + if !parallel_tree.is_intercepting() { + keys_to_replace.push(key.clone()); + } + } + + for key in keys_to_replace { + let subdir_name: RcStr = format!("@{}", key).into(); + + let default = if key == "children" { + components.default + } else if let Some(subdirectory) = directory_tree.subdirectories.get(&subdir_name) { + subdirectory.components.default + } else { + None }; + + tree.parallel_routes.insert( + key, + default_route_tree(app_dir, global_metadata, app_page.clone(), default), + ); + } + } + + if tree.parallel_routes.is_empty() { + if components.default.is_some() || current_level_is_parallel_route { + tree = default_route_tree(app_dir, global_metadata, app_page, components.default); } else { return Ok(None); } } else if tree.parallel_routes.get("children").is_none() { tree.parallel_routes.insert( "children".into(), - LoaderTree { - page: app_page.clone(), - segment: "__DEFAULT__".into(), - parallel_routes: IndexMap::new(), - components: if let Some(default) = components.default { - Components { - default: Some(default), - ..Default::default() - } - } else { - // default fallback component - Components { - default: Some( - get_next_package(app_dir) - .join("dist/client/components/parallel-route-default.js".into()), - ), - ..Default::default() - } - }, - global_metadata, - }, + default_route_tree(app_dir, global_metadata, app_page, components.default), ); } @@ -997,6 +1012,35 @@ fn directory_tree_to_loader_tree_internal( Ok(Some(tree)) } +fn default_route_tree( + app_dir: Vc, + global_metadata: Vc, + app_page: AppPage, + default_component: Option>, +) -> LoaderTree { + LoaderTree { + page: app_page.clone(), + segment: "__DEFAULT__".into(), + parallel_routes: IndexMap::new(), + components: if let Some(default) = default_component { + Components { + default: Some(default), + ..Default::default() + } + } else { + // default fallback component + Components { + default: Some( + get_next_package(app_dir) + .join("dist/client/components/parallel-route-default.js".into()), + ), + ..Default::default() + } + }, + global_metadata, + } +} + #[turbo_tasks::function] async fn directory_tree_to_entrypoints_internal( app_dir: Vc, diff --git a/crates/next-core/src/next_app/mod.rs b/crates/next-core/src/next_app/mod.rs index 37b177c355035..1508764798531 100644 --- a/crates/next-core/src/next_app/mod.rs +++ b/crates/next-core/src/next_app/mod.rs @@ -259,6 +259,23 @@ impl AppPage { ) } + pub fn is_intercepting(&self) -> bool { + let segment = if self.is_complete() { + // The `PageType` is the last segment for completed pages. + self.0.iter().nth_back(1) + } else { + self.0.last() + }; + + matches!( + segment, + Some(PageSegment::Static(segment)) + if segment.starts_with("(.)") + || segment.starts_with("(..)") + || segment.starts_with("(...)") + ) + } + pub fn complete(&self, page_type: PageType) -> Result { self.clone_push(PageSegment::PageType(page_type)) } diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/(.)cart/page.tsx b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/(.)cart/page.tsx new file mode 100644 index 0000000000000..e823f944f01e9 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/(.)cart/page.tsx @@ -0,0 +1,7 @@ +export default async function CartModalPage() { + return ( +
+

Cart Modal

+
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/[...segments]/page.tsx b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/[...segments]/page.tsx new file mode 100644 index 0000000000000..e7eeda786dcd7 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/[...segments]/page.tsx @@ -0,0 +1,12 @@ +// This component is required for the cart modal to hide when the user navigates to a new page +// https://nextjs.org/docs/app/building-your-application/routing/parallel-routes#closing-the-modal +// Since client-side navigations to a route that no longer match the slot will remain visible, +// we need to match the slot to a route that returns null to close the modal. + +export default function ModalCartCatchAll() { + return ( +
+

Cart Catch All

+
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/default.tsx b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/default.tsx new file mode 100644 index 0000000000000..6662c7dad128d --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@modal/default.tsx @@ -0,0 +1,7 @@ +export default function ModalCartDefault() { + return ( +
+

Cart Default

+
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@slot/[...segments]/page.tsx b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@slot/[...segments]/page.tsx new file mode 100644 index 0000000000000..6ec7f5a9ceb71 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@slot/[...segments]/page.tsx @@ -0,0 +1,7 @@ +export default function CatchAll() { + return ( +
+

Slot Catch All

+
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@slot/page.tsx b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@slot/page.tsx new file mode 100644 index 0000000000000..9c8a692084a98 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/@slot/page.tsx @@ -0,0 +1,7 @@ +export default function SlotRoot() { + return ( +
+

Slot Page

+
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/[...segments]/page.tsx b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/[...segments]/page.tsx new file mode 100644 index 0000000000000..f3ae689fd6ed6 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/[...segments]/page.tsx @@ -0,0 +1,7 @@ +export default function CatchAll() { + return ( +
+

Root Catch All

+
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/cart/page.tsx b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/cart/page.tsx new file mode 100644 index 0000000000000..a60f83de59ff3 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/cart/page.tsx @@ -0,0 +1,7 @@ +export default async function CartPage() { + return ( +
+

Cart Page

+
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/layout.tsx b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/layout.tsx new file mode 100644 index 0000000000000..66f5297e1722c --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/layout.tsx @@ -0,0 +1,30 @@ +import React from 'react' + +export default function RootLayout({ + children, + modal, + slot, +}: Readonly<{ + children: React.ReactNode + modal: React.ReactNode + slot: React.ReactNode +}>) { + return ( + + +
+

Children

+
{children}
+
+
+

Modal

+ +
+
+

Slot

+
{slot}
+
+ + + ) +} diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/page.tsx b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/page.tsx new file mode 100644 index 0000000000000..81e351cbd509e --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/app/page.tsx @@ -0,0 +1,12 @@ +import Link from 'next/link' + +export default function Home() { + return ( +
+

Home Page

+ Open cart +
+ Open catch all +
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/next.config.js b/test/e2e/app-dir/parallel-routes-and-interception-catchall/next.config.js new file mode 100644 index 0000000000000..807126e4cf0bf --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/next.config.js @@ -0,0 +1,6 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = {} + +module.exports = nextConfig diff --git a/test/e2e/app-dir/parallel-routes-and-interception-catchall/parallel-routes-and-interception-catchall.test.ts b/test/e2e/app-dir/parallel-routes-and-interception-catchall/parallel-routes-and-interception-catchall.test.ts new file mode 100644 index 0000000000000..6cbf73211b3d9 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-and-interception-catchall/parallel-routes-and-interception-catchall.test.ts @@ -0,0 +1,41 @@ +import { nextTestSetup } from 'e2e-utils' + +describe('parallel-routes-and-interception-catchall', () => { + const { next } = nextTestSetup({ + files: __dirname, + }) + + it('should render intercepted route and preserve other slots', async () => { + const browser = await next.browser('/') + + const homeContent = 'Home Page\n\nOpen cart\nOpen catch all' + const slotContent = 'Slot Page' + + expect(await browser.elementById('children-slot').text()).toBe(homeContent) + expect(await browser.elementById('slot-slot').text()).toBe(slotContent) + + // Check if navigation to modal route works + await browser + .elementByCss('[href="/cart"]') + .click() + .waitForElementByCss('#cart-modal-intercept') + + expect(await browser.elementById('cart-modal-intercept').text()).toBe( + 'Cart Modal' + ) + + // Children slot should still be the same + expect(await browser.elementById('children-slot').text()).toBe(homeContent) + expect(await browser.elementById('slot-slot').text()).toBe(slotContent) + + // Check if url matches even though it was intercepted. + expect(await browser.url()).toBe(next.url + '/cart') + + // Trigger a refresh, this should load the normal page, not the modal. + await browser.refresh().waitForElementByCss('#cart-page') + expect(await browser.elementById('children-slot').text()).toBe('Cart Page') + + // Check if the url matches still. + expect(await browser.url()).toBe(next.url + '/cart') + }) +})