Skip to content

Commit

Permalink
fix(turbopack): intercepting routes should have default slots (#69929)
Browse files Browse the repository at this point in the history
### 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
  • Loading branch information
ForsakenHarmony committed Sep 17, 2024
1 parent 4e14037 commit 7eccedc
Show file tree
Hide file tree
Showing 13 changed files with 241 additions and 37 deletions.
118 changes: 81 additions & 37 deletions crates/next-core/src/app_structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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 {
Expand Down Expand Up @@ -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),
);
}

Expand All @@ -997,6 +1012,35 @@ fn directory_tree_to_loader_tree_internal(
Ok(Some(tree))
}

fn default_route_tree(
app_dir: Vc<FileSystemPath>,
global_metadata: Vc<GlobalMetadata>,
app_page: AppPage,
default_component: Option<Vc<FileSystemPath>>,
) -> 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<FileSystemPath>,
Expand Down
17 changes: 17 additions & 0 deletions crates/next-core/src/next_app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
self.clone_push(PageSegment::PageType(page_type))
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default async function CartModalPage() {
return (
<div id="cart-modal-intercept">
<p>Cart Modal</p>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -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 (
<div id="cart-modal-catch-all">
<p>Cart Catch All</p>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function ModalCartDefault() {
return (
<div id="cart-modal-default">
<p>Cart Default</p>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function CatchAll() {
return (
<div id="slot-catch-all">
<p>Slot Catch All</p>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function SlotRoot() {
return (
<div id="slot-page">
<p>Slot Page</p>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function CatchAll() {
return (
<div id="root-catch-all">
<p>Root Catch All</p>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default async function CartPage() {
return (
<div id="cart-page">
<p>Cart Page</p>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -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 (
<html lang="en">
<body>
<section>
<h1>Children</h1>
<div id="children-slot">{children}</div>
</section>
<section>
<h1>Modal</h1>
<div id="modal-slot">{modal}</div>
</section>
<section>
<h1>Slot</h1>
<div id="slot-slot">{slot}</div>
</section>
</body>
</html>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Link from 'next/link'

export default function Home() {
return (
<div id="root-page">
<p>Home Page</p>
<Link href="/cart">Open cart</Link>
<br />
<Link href="/catch-all">Open catch all</Link>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {}

module.exports = nextConfig
Original file line number Diff line number Diff line change
@@ -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')
})
})

0 comments on commit 7eccedc

Please sign in to comment.