Skip to content

Commit 9931cc0

Browse files
ijjkztanner
ijjk
and
ztanner
authored
Fix fallback route params case in app-page handler (#91737)
## Summary - add production-route-shape fixture coverage for `/[teamSlug]/[project]/settings/domains` - add revalidation helpers (API and server action) and browser controls used by the regression test - reproduce the failing production sequence by priming, revalidating, then loading a page with in-view `next/link` and navigating repeatedly x-ref: #91627 x-ref: #91603 ## Testing - `IS_WEBPACK_TEST=1 NEXT_SKIP_ISOLATE=1 NEXT_TEST_MODE=start pnpm testheadless test/e2e/app-dir/segment-cache/vary-params-base-dynamic/vary-params-base-dynamic.test.ts -t "production route shape"` (passes) - `NEXT_ENABLE_ADAPTER=1 pnpm test-deploy test/e2e/app-dir/segment-cache/vary-params-base-dynamic/vary-params-base-dynamic.test.ts -t "production route shape"` - `NEXT_ENABLE_ADAPTER=1 pnpm test-deploy test/e2e/app-dir/segment-cache/vary-params-base-dynamic/vary-params-base-dynamic.test.ts` (fails in the two production-shape cases) --------- Co-authored-by: Zack Tanner <[email protected]>
1 parent b592e6d commit 9931cc0

15 files changed

Lines changed: 1094 additions & 84 deletions

File tree

‎packages/next/src/build/templates/app-page.ts‎

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import {
3131
import { checkIsAppPPREnabled } from '../../server/lib/experimental/ppr' with { 'turbopack-transition': 'next-server-utility' }
3232
import {
3333
getFallbackRouteParams,
34+
getPlaceholderFallbackRouteParams,
35+
buildDynamicSegmentPlaceholder,
3436
createOpaqueFallbackRouteParams,
3537
type OpaqueFallbackRouteParams,
3638
} from '../../server/request/fallback-params' with { 'turbopack-transition': 'next-server-utility' }
@@ -116,10 +118,7 @@ import { RedirectStatusCode } from '../../client/components/redirect-status-code
116118
import { InvariantError } from '../../shared/lib/invariant-error' with { 'turbopack-transition': 'next-server-utility' }
117119
import { scheduleOnNextTick } from '../../lib/scheduler' with { 'turbopack-transition': 'next-server-utility' }
118120
import { isInterceptionRouteAppPath } from '../../shared/lib/router/utils/interception-routes' with { 'turbopack-transition': 'next-server-utility' }
119-
import {
120-
getParamProperties,
121-
getSegmentParam,
122-
} from '../../shared/lib/router/utils/get-segment-param' with { 'turbopack-transition': 'next-server-utility' }
121+
import { getSegmentParam } from '../../shared/lib/router/utils/get-segment-param' with { 'turbopack-transition': 'next-server-utility' }
123122

124123
export * from '../../server/app-render/entry-base' with { 'turbopack-transition': 'next-server-utility' }
125124

@@ -141,22 +140,6 @@ export const routeModule = new AppPageRouteModule({
141140
relativeProjectDir: process.env.__NEXT_RELATIVE_PROJECT_DIR || '',
142141
})
143142

144-
function buildDynamicSegmentPlaceholder(
145-
param: Pick<FallbackRouteParam, 'paramName' | 'paramType'>
146-
): string {
147-
const { repeat, optional } = getParamProperties(param.paramType)
148-
149-
if (optional) {
150-
return `[[...${param.paramName}]]`
151-
}
152-
153-
if (repeat) {
154-
return `[...${param.paramName}]`
155-
}
156-
157-
return `[${param.paramName}]`
158-
}
159-
160143
/**
161144
* Builds the cache key for the most complete prerenderable shell we can derive
162145
* from the shell that matched this request. Only params that can still be
@@ -1339,6 +1322,31 @@ export async function handler(
13391322
}
13401323
}
13411324

1325+
const placeholderFallbackRouteParams =
1326+
// When a request carries dynamic placeholder values (e.g. "[slug]"),
1327+
// defer only the unresolved subset instead of forcing all fallback
1328+
// params to suspend.
1329+
!routeModule.isDev &&
1330+
pageIsDynamic &&
1331+
prerenderInfo?.fallbackRouteParams
1332+
? getPlaceholderFallbackRouteParams(
1333+
params as
1334+
| Record<string, undefined | string | string[]>
1335+
| undefined,
1336+
prerenderInfo.fallbackRouteParams
1337+
)
1338+
: null
1339+
1340+
const fallbackRouteParamsForRender =
1341+
placeholderFallbackRouteParams &&
1342+
placeholderFallbackRouteParams.length > 0
1343+
? placeholderFallbackRouteParams
1344+
: prerenderInfo?.fallbackRouteParams
1345+
1346+
const hasPlaceholderFallbackRouteParams =
1347+
placeholderFallbackRouteParams != null &&
1348+
placeholderFallbackRouteParams.length > 0
1349+
13421350
// When route-module.ts resolved partial nxtP* params during
13431351
// background revalidation, filter fallbackRouteParams to only the
13441352
// params that are still unresolved. This lets doRender produce an
@@ -1359,9 +1367,10 @@ export async function handler(
13591367
// non-prerendered URL, use the prerender manifest's fallback route
13601368
// params which correctly identifies which params are unknown.
13611369
((isProduction && getRequestMeta(req, 'renderFallbackShell')) ||
1370+
hasPlaceholderFallbackRouteParams ||
13621371
(isDebugStaticShell && !isPrerendered)) &&
1363-
prerenderInfo?.fallbackRouteParams
1364-
? createOpaqueFallbackRouteParams(prerenderInfo.fallbackRouteParams)
1372+
fallbackRouteParamsForRender
1373+
? createOpaqueFallbackRouteParams(fallbackRouteParamsForRender)
13651374
: // For intermediate shells where some params are resolved and
13661375
// others still have placeholders, use the filtered subset so the
13671376
// prerender suspends only for the unresolved params.
@@ -1386,7 +1395,7 @@ export async function handler(
13861395
prerenderInfo?.fallbackRouteParams
13871396
) {
13881397
const fallbackParams = createOpaqueFallbackRouteParams(
1389-
prerenderInfo.fallbackRouteParams
1398+
fallbackRouteParamsForRender ?? prerenderInfo.fallbackRouteParams
13901399
)
13911400

13921401
if (fallbackParams) {

‎packages/next/src/server/request/fallback-params.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {
2+
buildDynamicSegmentPlaceholder,
23
createOpaqueFallbackRouteParams,
34
getFallbackRouteParams,
5+
getPlaceholderFallbackRouteParams,
46
} from './fallback-params'
57
import type { FallbackRouteParam } from '../../build/static-paths/types'
68
import type AppPageRouteModule from '../route-modules/app-page/module'
@@ -72,6 +74,54 @@ describe('createOpaqueFallbackRouteParams', () => {
7274
})
7375
})
7476

77+
describe('placeholder fallback route params', () => {
78+
it('builds route placeholders by dynamic param type', () => {
79+
expect(
80+
buildDynamicSegmentPlaceholder({
81+
paramName: 'slug',
82+
paramType: 'dynamic',
83+
})
84+
).toBe('[slug]')
85+
expect(
86+
buildDynamicSegmentPlaceholder({
87+
paramName: 'slug',
88+
paramType: 'catchall',
89+
})
90+
).toBe('[...slug]')
91+
expect(
92+
buildDynamicSegmentPlaceholder({
93+
paramName: 'slug',
94+
paramType: 'optional-catchall',
95+
})
96+
).toBe('[[...slug]]')
97+
})
98+
99+
it('returns only fallback params that are still placeholders', () => {
100+
const fallbackParams: readonly FallbackRouteParam[] = [
101+
{ paramName: 'team', paramType: 'dynamic' },
102+
{ paramName: 'project', paramType: 'dynamic' },
103+
{ paramName: 'slug', paramType: 'catchall' },
104+
{ paramName: 'optional', paramType: 'optional-catchall' },
105+
]
106+
107+
const result = getPlaceholderFallbackRouteParams(
108+
{
109+
team: '[team]',
110+
project: 'dashboard',
111+
slug: ['[...slug]'],
112+
optional: '[[...optional]]',
113+
},
114+
fallbackParams
115+
)
116+
117+
expect(result).toEqual([
118+
{ paramName: 'team', paramType: 'dynamic' },
119+
{ paramName: 'slug', paramType: 'catchall' },
120+
{ paramName: 'optional', paramType: 'optional-catchall' },
121+
])
122+
})
123+
})
124+
75125
describe('getFallbackRouteParams', () => {
76126
describe('Regular Routes (children segments)', () => {
77127
it('should extract single dynamic segment from children route', () => {

‎packages/next/src/server/request/fallback-params.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { dynamicParamTypes } from '../app-render/get-short-dynamic-param-type'
55
import type AppPageRouteModule from '../route-modules/app-page/module'
66
import { parseAppRoute } from '../../shared/lib/router/routes/app'
77
import { extractPathnameRouteParamSegmentsFromLoaderTree } from '../../build/static-paths/app/extract-pathname-route-param-segments-from-loader-tree'
8+
import { getParamProperties } from '../../shared/lib/router/utils/get-segment-param'
89

910
export type OpaqueFallbackRouteParamValue = [
1011
/**
@@ -74,6 +75,37 @@ export function createOpaqueFallbackRouteParams(
7475
return keys
7576
}
7677

78+
export function buildDynamicSegmentPlaceholder(
79+
param: Pick<FallbackRouteParam, 'paramName' | 'paramType'>
80+
): string {
81+
const { repeat, optional } = getParamProperties(param.paramType)
82+
83+
if (optional) {
84+
return `[[...${param.paramName}]]`
85+
}
86+
87+
if (repeat) {
88+
return `[...${param.paramName}]`
89+
}
90+
91+
return `[${param.paramName}]`
92+
}
93+
94+
export function getPlaceholderFallbackRouteParams(
95+
params: Record<string, undefined | string | string[]> | undefined,
96+
fallbackRouteParams: readonly FallbackRouteParam[]
97+
): FallbackRouteParam[] {
98+
return fallbackRouteParams.filter((param) => {
99+
const placeholder = buildDynamicSegmentPlaceholder(param)
100+
const value = params?.[param.paramName]
101+
102+
return (
103+
value === placeholder ||
104+
(Array.isArray(value) && value.length === 1 && value[0] === placeholder)
105+
)
106+
})
107+
}
108+
77109
/**
78110
* Gets the fallback route params for a given page. This is an expensive
79111
* operation because it requires parsing the loader tree to extract the fallback

‎test/e2e/app-dir/segment-cache/vary-params-base-dynamic/app/[teamSlug]/[project]/(domains)/settings/domains/loading.tsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function ProjectDomainsSettingsLoading() {
2+
return (
3+
<div data-team-project-settings-domains-loading="true">
4+
Loading project domains settings...
5+
</div>
6+
)
7+
}

‎test/e2e/app-dir/segment-cache/vary-params-base-dynamic/app/[teamSlug]/[project]/(domains)/settings/domains/page.tsx‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { cacheLife } from 'next/cache'
2+
import Link from 'next/link'
3+
import { Suspense } from 'react'
4+
import { LinkAccordion } from '../../../../../../components/link-accordion'
5+
6+
type Params = { teamSlug: string; project: string }
7+
8+
export default function ProjectDomainsSettingsPage({
9+
params,
10+
}: {
11+
params: Promise<Params>
12+
}) {
13+
return (
14+
<div id="team-project-settings-domains-page">
15+
<Suspense
16+
fallback={
17+
<div data-loading="true">
18+
Loading project settings domains page...
19+
</div>
20+
}
21+
>
22+
<ProjectDomainsSettingsContent params={params} />
23+
</Suspense>
24+
<nav data-nav-link-list="true">
25+
<ul>
26+
<li>
27+
<Link
28+
href="/acme/dashboard/settings"
29+
data-nav-link="/acme/dashboard/settings"
30+
>
31+
Navigate: acme/dashboard/settings
32+
</Link>
33+
</li>
34+
<li>
35+
<Link
36+
href="/globex/portal/settings"
37+
data-nav-link="/globex/portal/settings"
38+
>
39+
Navigate: globex/portal/settings
40+
</Link>
41+
</li>
42+
<li>
43+
<Link
44+
href="/acme/dashboard/settings/domains"
45+
data-nav-link="/acme/dashboard/settings/domains"
46+
>
47+
Navigate: acme/dashboard/settings/domains
48+
</Link>
49+
</li>
50+
<li>
51+
<Link
52+
href="/globex/portal/settings/domains"
53+
data-nav-link="/globex/portal/settings/domains"
54+
>
55+
Navigate: globex/portal/settings/domains
56+
</Link>
57+
</li>
58+
</ul>
59+
</nav>
60+
<div data-related-link-list="true">
61+
<LinkAccordion href="/acme/dashboard/settings/domains">
62+
Related route: acme/dashboard/settings/domains
63+
</LinkAccordion>
64+
<LinkAccordion href="/globex/portal/settings/domains">
65+
Related route: globex/portal/settings/domains
66+
</LinkAccordion>
67+
</div>
68+
</div>
69+
)
70+
}
71+
72+
async function ProjectDomainsSettingsContent({
73+
params,
74+
}: {
75+
params: Promise<Params>
76+
}) {
77+
'use cache'
78+
cacheLife({ stale: 0, revalidate: 1, expire: 60 })
79+
80+
const { teamSlug, project } = await params
81+
const marker = Date.now()
82+
83+
return (
84+
<div data-team-project-settings-domains-content="true">
85+
{`Project domains settings content - team: ${teamSlug}, project: ${project}, marker: ${marker}`}
86+
</div>
87+
)
88+
}

‎test/e2e/app-dir/segment-cache/vary-params-base-dynamic/app/[teamSlug]/[project]/(domains)/settings/layout.tsx‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { ReactNode } from 'react'
2+
3+
export default function ProjectSettingsLayout({
4+
children,
5+
}: {
6+
children: ReactNode
7+
}) {
8+
return (
9+
<div data-project-settings-layout="domains-variant">
10+
<h2>Project Settings Layout (domains variant)</h2>
11+
{children}
12+
</div>
13+
)
14+
}

‎test/e2e/app-dir/segment-cache/vary-params-base-dynamic/app/[teamSlug]/[project]/page.tsx‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { cacheLife } from 'next/cache'
2+
import Link from 'next/link'
23
import { Suspense } from 'react'
4+
import { LinkAccordion } from '../../../components/link-accordion'
35

46
type Params = { teamSlug: string; project: string }
57

@@ -15,6 +17,71 @@ export default function TeamProjectPage({
1517
>
1618
<TeamProjectContent params={params} />
1719
</Suspense>
20+
<nav data-nav-link-list="true">
21+
<ul>
22+
<li>
23+
<Link href="/" data-nav-link="/">
24+
Navigate: home
25+
</Link>
26+
</li>
27+
<li>
28+
<Link href="/acme/dashboard" data-nav-link="/acme/dashboard">
29+
Navigate: acme/dashboard
30+
</Link>
31+
</li>
32+
<li>
33+
<Link href="/globex/portal" data-nav-link="/globex/portal">
34+
Navigate: globex/portal
35+
</Link>
36+
</li>
37+
<li>
38+
<Link
39+
href="/acme/dashboard/settings"
40+
data-nav-link="/acme/dashboard/settings"
41+
>
42+
Navigate: acme/dashboard/settings
43+
</Link>
44+
</li>
45+
<li>
46+
<Link
47+
href="/globex/portal/settings"
48+
data-nav-link="/globex/portal/settings"
49+
>
50+
Navigate: globex/portal/settings
51+
</Link>
52+
</li>
53+
<li>
54+
<Link
55+
href="/acme/dashboard/settings/domains"
56+
data-nav-link="/acme/dashboard/settings/domains"
57+
>
58+
Navigate: acme/dashboard/settings/domains
59+
</Link>
60+
</li>
61+
<li>
62+
<Link
63+
href="/globex/portal/settings/domains"
64+
data-nav-link="/globex/portal/settings/domains"
65+
>
66+
Navigate: globex/portal/settings/domains
67+
</Link>
68+
</li>
69+
</ul>
70+
</nav>
71+
<div data-related-link-list="true">
72+
<LinkAccordion href="/acme/dashboard">
73+
Related route: acme/dashboard
74+
</LinkAccordion>
75+
<LinkAccordion href="/globex/portal">
76+
Related route: globex/portal
77+
</LinkAccordion>
78+
<LinkAccordion href="/acme/dashboard/settings/domains">
79+
Related route: acme/dashboard/settings/domains
80+
</LinkAccordion>
81+
<LinkAccordion href="/globex/portal/settings/domains">
82+
Related route: globex/portal/settings/domains
83+
</LinkAccordion>
84+
</div>
1885
</div>
1986
)
2087
}

0 commit comments

Comments
 (0)