Skip to content

Commit

Permalink
Fix interception/detail routes being triggered by router refreshes
Browse files Browse the repository at this point in the history
  • Loading branch information
ztanner committed Mar 14, 2024
1 parent 6556a34 commit 72d48ea
Show file tree
Hide file tree
Showing 24 changed files with 280 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { applyFlightData } from '../apply-flight-data'
import type { CacheNode } from '../../../../shared/lib/app-router-context.shared-runtime'
import { createEmptyCacheNode } from '../../app-router'
import { handleSegmentMismatch } from '../handle-segment-mismatch'
import { hasInterceptionRouteInCurrentTree } from './has-interception-route-in-current-tree'

// A version of refresh reducer that keeps the cache around instead of wiping all of it.
function fastRefreshReducerImpl(
Expand All @@ -27,12 +28,16 @@ function fastRefreshReducerImpl(
mutable.preserveCustomHistoryState = false

const cache: CacheNode = createEmptyCacheNode()
// If the current tree was intercepted, the nextUrl should be included in the request.
// This is to ensure that the refresh request doesn't get intercepted, accidentally triggering the interception route.
const includeNextUrl = hasInterceptionRouteInCurrentTree(state.tree)

// TODO-APP: verify that `href` is not an external url.
// Fetch data from the root of the tree.
cache.lazyData = fetchServerResponse(
new URL(href, origin),
[state.tree[0], state.tree[1], state.tree[2], 'refetch'],
state.nextUrl,
includeNextUrl ? state.nextUrl : null,
state.buildId
)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import type { FlightRouterState } from '../../../../server/app-render/types'
import { isInterceptionRouteAppPath } from '../../../../server/future/helpers/interception-routes'

export function hasInterceptionRouteInCurrentTree([
segment,
parallelRoutes,
]: FlightRouterState): boolean {
// If we have a dynamic segment, it's marked as an interception route by the presence of the `i` suffix.
if (Array.isArray(segment) && (segment[2] === 'di' || segment[2] === 'ci')) {
return true
}

// If segment is not an array, apply the existing string-based check
if (typeof segment === 'string' && isInterceptionRouteAppPath(segment)) {
return true
}

// Iterate through parallelRoutes if they exist
if (parallelRoutes) {
for (const key in parallelRoutes) {
if (hasInterceptionRouteInCurrentTree(parallelRoutes[key])) {
return true
}
}
}

return false
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type { CacheNode } from '../../../../shared/lib/app-router-context.shared
import { fillLazyItemsTillLeafWithHead } from '../fill-lazy-items-till-leaf-with-head'
import { createEmptyCacheNode } from '../../app-router'
import { handleSegmentMismatch } from '../handle-segment-mismatch'
import { hasInterceptionRouteInCurrentTree } from './has-interception-route-in-current-tree'

export function refreshReducer(
state: ReadonlyReducerState,
Expand All @@ -28,12 +29,17 @@ export function refreshReducer(
mutable.preserveCustomHistoryState = false

const cache: CacheNode = createEmptyCacheNode()

// If the current tree was intercepted, the nextUrl should be included in the request.
// This is to ensure that the refresh request doesn't get intercepted, accidentally triggering the interception route.
const includeNextUrl = hasInterceptionRouteInCurrentTree(state.tree)

// TODO-APP: verify that `href` is not an external url.
// Fetch data from the root of the tree.
cache.lazyData = fetchServerResponse(
new URL(href, origin),
[currentTree[0], currentTree[1], currentTree[2], 'refetch'],
state.nextUrl,
includeNextUrl ? state.nextUrl : null,
state.buildId
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import type { CacheNode } from '../../../../shared/lib/app-router-context.shared
import { handleMutable } from '../handle-mutable'
import { fillLazyItemsTillLeafWithHead } from '../fill-lazy-items-till-leaf-with-head'
import { createEmptyCacheNode } from '../../app-router'
import { extractPathFromFlightRouterState } from '../compute-changed-path'
import { hasInterceptionRouteInCurrentTree } from './has-interception-route-in-current-tree'
import { handleSegmentMismatch } from '../handle-segment-mismatch'

type FetchServerActionResult = {
Expand All @@ -57,12 +57,12 @@ async function fetchServerAction(
): Promise<FetchServerActionResult> {
const body = await encodeReply(actionArgs)

const newNextUrl = extractPathFromFlightRouterState(state.tree)
// only pass along the `nextUrl` param (used for interception routes) if it exists and
// if it's different from the current `nextUrl`. This indicates the route has already been intercepted,
// and so the action should be as well. Otherwise the server action might be intercepted
// with the wrong action id (ie, one that corresponds with the intercepted route)
const includeNextUrl = state.nextUrl && state.nextUrl !== newNextUrl
// only pass along the `nextUrl` param (used for interception routes) if it exists in the current tree.
// This indicates the route has already been intercepted, and so the action should be as well.
// Otherwise the server action might be intercepted with the wrong action id
// (ie, one that corresponds with the intercepted route)
const includeNextUrl =
state.nextUrl && hasInterceptionRouteInCurrentTree(state.tree)

const res = await fetch('', {
method: 'POST',
Expand Down
6 changes: 4 additions & 2 deletions packages/next/src/server/app-render/get-segment-param.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,23 @@ export function getSegmentParam(segment: string): {

if (segment.startsWith('[[...') && segment.endsWith(']]')) {
return {
// TODO-APP: Optional catchall does not currently work with parallel routes,
// so for now aren't handling a potential interception marker.
type: 'optional-catchall',
param: segment.slice(5, -2),
}
}

if (segment.startsWith('[...') && segment.endsWith(']')) {
return {
type: 'catchall',
type: interceptionMarker ? 'catchall-intercepted' : 'catchall',
param: segment.slice(4, -1),
}
}

if (segment.startsWith('[') && segment.endsWith(']')) {
return {
type: 'dynamic',
type: interceptionMarker ? 'dynamic-intercepted' : 'dynamic',
param: segment.slice(1, -1),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ export const dynamicParamTypes: Record<
DynamicParamTypesShort
> = {
catchall: 'c',
'catchall-intercepted': 'ci',
'optional-catchall': 'oc',
dynamic: 'd',
'dynamic-intercepted': 'di',
}

/**
Expand Down
11 changes: 8 additions & 3 deletions packages/next/src/server/app-render/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ import type { SwrDelta } from '../lib/revalidate'

import s from 'next/dist/compiled/superstruct'

export type DynamicParamTypes = 'catchall' | 'optional-catchall' | 'dynamic'

const dynamicParamTypesSchema = s.enums(['c', 'oc', 'd'])
export type DynamicParamTypes =
| 'catchall'
| 'catchall-intercepted'
| 'optional-catchall'
| 'dynamic'
| 'dynamic-intercepted'

const dynamicParamTypesSchema = s.enums(['c', 'ci', 'oc', 'd', 'di'])

export type DynamicParamTypesShort = s.Infer<typeof dynamicParamTypesSchema>

Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/server/dev/on-demand-entry-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,12 @@ function convertDynamicParamTypeToSyntax(
) {
switch (dynamicParamTypeShort) {
case 'c':
case 'ci':
return `[...${param}]`
case 'oc':
return `[[...${param}]]`
case 'd':
case 'di':
return `[${param}]`
default:
throw new Error('Unknown dynamic param type')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Page() {
return (
<div>
<h2 id="detail-title">Detail Page (Intercepted)</h2>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use client'
import { useRouter } from 'next/navigation'

export default function Page({ params }: { params: { dynamic: string } }) {
const router = useRouter()
return (
<div>
<h2 id="detail-title">Detail Page (Intercepted)</h2>
<p id="params">{params.dynamic}</p>
<span suppressHydrationWarning id="random-number">
{Math.random()}
</span>
<button onClick={() => router.refresh()}>Refresh</button>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use client'
import { useRouter } from 'next/navigation'

export default function Page({ params }: { params: { dynamic: string } }) {
const router = useRouter()
return (
<div>
<h2 id="detail-title">Detail Page (Non-Intercepted)</h2>
<p id="params">{params.dynamic}</p>
<span suppressHydrationWarning id="random-number">
{Math.random()}
</span>
<button onClick={() => router.refresh()}>Refresh</button>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import React from 'react'

export default function Layout({
children,
interception2,
}: {
children: React.ReactNode
interception2: React.ReactNode
}) {
return (
<div>
{children}
{interception2}
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import Link from 'next/link'

export default function Page() {
return (
<div>
<Link href="/catchall/foobar">Dynamic Catchall Interception</Link>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use client'
import { useRouter } from 'next/navigation'

export default function Page() {
const router = useRouter()
return (
<div>
<h2 id="detail-title">Detail Page (Non-Intercepted)</h2>
<span suppressHydrationWarning id="random-number">
{Math.random()}
</span>
<button onClick={() => router.refresh()}>Refresh</button>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use client'
import { useRouter } from 'next/navigation'

export default function Page({ params }: { params: { dynamic: string } }) {
const router = useRouter()
return (
<div>
<h2 id="detail-title">Detail Page (Intercepted)</h2>
<p id="params">{params.dynamic}</p>
<span suppressHydrationWarning id="random-number">
{Math.random()}
</span>
<button onClick={() => router.refresh()}>Refresh</button>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use client'
import { useRouter } from 'next/navigation'

export default function Page({ params }: { params: { dynamic: string } }) {
const router = useRouter()
return (
<div>
<h2 id="detail-title">Detail Page (Non-Intercepted)</h2>
<p id="params">{params.dynamic}</p>
<span suppressHydrationWarning id="random-number">
{Math.random()}
</span>
<button onClick={() => router.refresh()}>Refresh</button>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Default() {
return null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import React from 'react'

export default function Layout({
children,
interception2,
}: {
children: React.ReactNode
interception2: React.ReactNode
}) {
return (
<div>
{children}
{interception2}
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import Link from 'next/link'

export default function Page() {
return (
<div>
<Link href="/dynamic/foobar">Dynamic Interception</Link>
</div>
)
}
1 change: 1 addition & 0 deletions test/e2e/app-dir/parallel-routes-revalidation/app/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export default async function Home() {
<Link href="/revalidate-modal">Open Revalidate Modal</Link>
<Link href="/refresh-modal">Open Refresh Modal</Link>
<Link href="/redirect-modal">Open Redirect Modal</Link>
<Link href="/detail-page">Intercepted Detail Page</Link>
<div id="random-number">{randomNumber}</div>
<div>
<h1>Current Data</h1>
Expand Down
Loading

0 comments on commit 72d48ea

Please sign in to comment.