Skip to content

Commit

Permalink
Fix fallback detection logic when multiple generateStaticParams are n…
Browse files Browse the repository at this point in the history
…eeded (#47982)

### What?

Our current logic of detecting if a route allows dynamic params or not
(`fallback`) is flawed, and this PR fixes it.

### Why?

Right now, if no `generateStaticParams` is specified we return
`fallback: undefined` during dev. However, for an app with multiple
params, it may have multiple `generateStaticParams` defined in different
levels. If some level isn't covered by any `generateStaticParams`, we
still can't determine the fallback value.

### How?

I added a naive implementation to check if all params are covered by
`generateStaticParams` in the current or inner layers.

Closes NEXT-946
  • Loading branch information
shuding committed Apr 7, 2023
1 parent 98dfbc0 commit 6b9af3e
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 13 deletions.
19 changes: 16 additions & 3 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,7 @@ export type AppConfig = {
}
export type GenerateParams = Array<{
config?: AppConfig
isDynamicSegment?: boolean
segmentPath: string
getStaticPaths?: GetStaticPaths
generateStaticParams?: any
Expand Down Expand Up @@ -1133,9 +1134,11 @@ export const collectGenerateParams = async (
const config = collectAppConfig(mod)

const isClientComponent = isClientReference(mod)
const isDynamicSegment = segment[0] && /^\[.+\]$/.test(segment[0])

const result = {
isLayout,
isDynamicSegment,
segmentPath: `/${parentSegments.join('/')}${
segment[0] && parentSegments.length > 0 ? '/' : ''
}${segment[0]}`,
Expand All @@ -1152,7 +1155,11 @@ export const collectGenerateParams = async (

if (result.config || result.generateStaticParams || result.getStaticPaths) {
generateParams.push(result)
} else if (isDynamicSegment) {
// It is a dynamic route, but no config was provided
generateParams.push(result)
}

return collectGenerateParams(
segment[1]?.children,
parentSegments,
Expand Down Expand Up @@ -1243,7 +1250,7 @@ export async function buildAppStaticPaths({
// if generateStaticParams is being used we iterate over them
// collecting them from each level
type Params = Array<Record<string, string | string[]>>
let hadGenerateParams = false
let hadAllParamsGenerated = false

const buildParams = async (
paramsItems: Params = [{}],
Expand All @@ -1258,9 +1265,15 @@ export async function buildAppStaticPaths({
typeof curGenerate.generateStaticParams !== 'function' &&
idx < generateParams.length
) {
if (curGenerate.isDynamicSegment) {
// This dynamic level has no generateStaticParams so we change
// this flag to false, but it could be covered by a later
// generateStaticParams so it could be set back to true.
hadAllParamsGenerated = false
}
return buildParams(paramsItems, idx + 1)
}
hadGenerateParams = true
hadAllParamsGenerated = true

const newParams = []

Expand All @@ -1287,7 +1300,7 @@ export async function buildAppStaticPaths({
(generate) => generate.config?.dynamicParams === false
)

if (!hadGenerateParams) {
if (!hadAllParamsGenerated) {
return {
paths: undefined,
fallback:
Expand Down
21 changes: 11 additions & 10 deletions test/e2e/app-dir/app-static/app-static.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ createNextDescribe(
'A required parameter (slug) was not provided as a string received object'
)
})

it('should correctly handle multi-level generateStaticParams when some levels are missing', async () => {
const browser = await next.browser('/flight/foo/bar')
const v = ~~(Math.random() * 1000)
await browser.eval(`document.cookie = "test-cookie=${v}"`)
await browser.elementByCss('button').click()
await check(async () => {
return await browser.elementByCss('h1').text()
}, v.toString())
})
}

it('should correctly skip caching POST fetch for POST handler', async () => {
Expand Down Expand Up @@ -216,6 +226,7 @@ createNextDescribe(
'dynamic-error/[id]/page.js',
'dynamic-no-gen-params-ssr/[slug]/page.js',
'dynamic-no-gen-params/[slug]/page.js',
'flight/[slug]/[slug2]/page.js',
'force-dynamic-catch-all/[slug]/[[...id]]/page.js',
'force-dynamic-no-prerender/[id]/page.js',
'force-dynamic-prerender/[slug]/page.js',
Expand Down Expand Up @@ -591,16 +602,6 @@ createNextDescribe(
'^\\/partial\\-gen\\-params\\-no\\-additional\\-slug\\/([^\\/]+?)\\/([^\\/]+?)(?:\\/)?$'
),
},
'/partial-gen-params/[lang]/[slug]': {
dataRoute: '/partial-gen-params/[lang]/[slug].rsc',
dataRouteRegex: normalizeRegEx(
'^\\/partial\\-gen\\-params\\/([^\\/]+?)\\/([^\\/]+?)\\.rsc$'
),
fallback: null,
routeRegex: normalizeRegEx(
'^\\/partial\\-gen\\-params\\/([^\\/]+?)\\/([^\\/]+?)(?:\\/)?$'
),
},
'/force-static/[slug]': {
dataRoute: '/force-static/[slug].rsc',
dataRouteRegex: normalizeRegEx(
Expand Down
13 changes: 13 additions & 0 deletions test/e2e/app-dir/app-static/app/flight/[slug]/[slug2]/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { cookies } from 'next/headers'

import Refresh from './refresh'

export default function Page() {
const cookieValue = cookies().get('test-cookie')
return (
<>
<Refresh />
<h1>{cookieValue?.value}</h1>
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use client'

import { useRouter } from 'next/navigation'

export default function Refresh() {
const router = useRouter()

return <button onClick={() => router.refresh()}>Refresh</button>
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/app-static/app/flight/[slug]/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Layout({ children }) {
return children
}

export function generateStaticParams() {
return [{ slug: 'foo' }]
}

0 comments on commit 6b9af3e

Please sign in to comment.