Skip to content

Commit

Permalink
fix: empty generateStaticParams should still create an ISR route (#73358
Browse files Browse the repository at this point in the history
)

Returning an empty array from `generateStaticParams` should still be
deployed as an ISR route to support the `dynamicParams = true` case,
where a route can be generated & cached on demand. At the moment the
only way to achieve this behavior is with `export const dynamic =
'force-static'`. As a result, these routes are being treated as dynamic
and never return a cached response.

This PR resolves the bug by fixing several distinct things:
- `prerenderedRoutes` does not need to be an array > 0 to mark a route
as being a static path. This will ensure the build output shows the
correct symbol and adds the path to the list of prerenders.
- we initialize `prerenderedRoutes` to an empty array if we determine
that the leaf-most segment (the thing rendering the current page) has a
`generateStaticParams` function
- the function responsible for collecting segments associated with a
page only considered the `children` branch, but for something like a
parallel/interception route, those segments would be inside of a
different parallel route key
- the regex for determining if a function was dynamic didn't consider
that the dynamic route might also contain an interception marker (eg
`(.)`)

This appears to have regressed in #68125

Closes NEXT-3905

---------

Co-authored-by: Wyatt Johnson <[email protected]>
  • Loading branch information
ztanner and wyattjoh authored Dec 10, 2024
1 parent 1ed81f7 commit 92b3dd2
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 13 deletions.
5 changes: 1 addition & 4 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2153,10 +2153,7 @@ export default async function build(
})
}

if (
workerResult.prerenderedRoutes &&
workerResult.prerenderedRoutes.length > 0
) {
if (workerResult.prerenderedRoutes) {
staticPaths.set(
originalAppPath,
workerResult.prerenderedRoutes
Expand Down
30 changes: 22 additions & 8 deletions packages/next/src/build/segment-config/app/app-segments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
import { isClientReference } from '../../../lib/client-reference'
import { getSegmentParam } from '../../../server/app-render/get-segment-param'
import { getLayoutOrPageModule } from '../../../server/lib/app-dir-module'
import { PAGE_SEGMENT_KEY } from '../../../shared/lib/segment'

type GenerateStaticParams = (options: { params?: Params }) => Promise<Params[]>

Expand Down Expand Up @@ -75,13 +76,16 @@ export type AppSegment = {
async function collectAppPageSegments(routeModule: AppPageRouteModule) {
const segments: AppSegment[] = []

let current = routeModule.userland.loaderTree
while (current) {
const [name, parallelRoutes] = current
const { mod: userland, filePath } = await getLayoutOrPageModule(current)
// Helper function to process a loader tree path
async function processLoaderTree(
loaderTree: any,
currentSegments: AppSegment[] = []
): Promise<void> {
const [name, parallelRoutes] = loaderTree
const { mod: userland, filePath } = await getLayoutOrPageModule(loaderTree)

const isClientComponent: boolean = userland && isClientReference(userland)
const isDynamicSegment = /^\[.*\]$/.test(name)
const isDynamicSegment = /\[.*\]$/.test(name)
const param = isDynamicSegment ? getSegmentParam(name)?.param : undefined

const segment: AppSegment = {
Expand All @@ -100,12 +104,22 @@ async function collectAppPageSegments(routeModule: AppPageRouteModule) {
attach(segment, userland, routeModule.definition.pathname)
}

segments.push(segment)
currentSegments.push(segment)

// If this is a page segment, we know we've reached a leaf node associated with the
// page we're collecting segments for. We can add the collected segments to our final result.
if (name === PAGE_SEGMENT_KEY) {
segments.push(...currentSegments)
}

// Use this route's parallel route children as the next segment.
current = parallelRoutes.children
// Recursively process parallel routes
for (const parallelRouteKey in parallelRoutes) {
const parallelRoute = parallelRoutes[parallelRouteKey]
await processLoaderTree(parallelRoute, [...currentSegments])
}
}

await processLoaderTree(routeModule.userland.loaderTree)
return segments
}

Expand Down
14 changes: 13 additions & 1 deletion packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1382,6 +1382,7 @@ export async function buildAppStaticPaths({
}
)

let lastDynamicSegmentHadGenerateStaticParams = false
for (const segment of segments) {
// Check to see if there are any missing params for segments that have
// dynamicParams set to false.
Expand All @@ -1402,6 +1403,15 @@ export async function buildAppStaticPaths({
)
}
}

if (
segment.isDynamicSegment &&
typeof segment.generateStaticParams !== 'function'
) {
lastDynamicSegmentHadGenerateStaticParams = false
} else if (typeof segment.generateStaticParams === 'function') {
lastDynamicSegmentHadGenerateStaticParams = true
}
}

// Determine if all the segments have had their parameters provided. If there
Expand Down Expand Up @@ -1437,7 +1447,9 @@ export async function buildAppStaticPaths({

let result: PartialStaticPathsResult = {
fallbackMode,
prerenderedRoutes: undefined,
prerenderedRoutes: lastDynamicSegmentHadGenerateStaticParams
? []
: undefined,
}

if (hadAllParamsGenerated && fallbackMode) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { Suspense } from 'react'

export default async function Page({
params,
}: {
params: Promise<{ slug: string }>
}) {
return (
<div>
Hello World
<div>
<Params params={params} />
</div>
</div>
)
}

async function Params({ params }: { params: Promise<{ slug: string }> }) {
return <Suspense>{(await params).slug}</Suspense>
}

export async function generateStaticParams() {
return []
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { ReactNode, Suspense } from 'react'
export default function Root({ children }: { children: ReactNode }) {
return (
<html>
<body>
<Suspense>{children}</Suspense>
</body>
</html>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>hello world</p>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { nextTestSetup } from 'e2e-utils'
import { retry } from 'next-test-utils'

describe('empty-generate-static-params', () => {
const { next, skipped } = nextTestSetup({
files: __dirname,
skipDeployment: true,
})

if (skipped) return

it('should mark the page with empty generateStaticParams as SSG in build output', async () => {
const isPPREnabled = process.env.__NEXT_EXPERIMENTAL_PPR === 'true'
expect(next.cliOutput).toContain(`${isPPREnabled ? '◐' : '●'} /[slug]`)
})

it('should be a cache miss on the initial render followed by a HIT after being generated', async () => {
const firstResponse = await next.fetch('/foo')
expect(firstResponse.status).toBe(200)

// With PPR enabled, the initial request doesn't send back a cache header
const isPPREnabled = process.env.__NEXT_EXPERIMENTAL_PPR === 'true'

expect(firstResponse.headers.get('x-nextjs-cache')).toBe(
isPPREnabled ? null : 'MISS'
)

retry(async () => {
const secondResponse = await next.fetch('/foo')
expect(secondResponse.status).toBe(200)
expect(secondResponse.headers.get('x-nextjs-cache')).toBe('HIT')
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {}

module.exports = nextConfig

0 comments on commit 92b3dd2

Please sign in to comment.