Skip to content

Commit

Permalink
fallback shell should not error when dynamic due to params access eve…
Browse files Browse the repository at this point in the history
…n with dynamic = "error"

When producing a fallback shell params is dynamic. Normally anything dynamic shoudl be a build error when `export const dynamic = "error"` is used. however for fallback shells we'll never have fully static shells, nor should we since the whole point is to produce a PPR shell that server a wide range of paths. In the refactor for async dynamic APIs I introduced a bug where fallback param dynamic also errored if `export const dynamic = "error"` was used. This change corrects this behavior and adds a corresponding test
  • Loading branch information
gnoff committed Sep 26, 2024
1 parent 0362f85 commit 3a6b271
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 17 deletions.
20 changes: 3 additions & 17 deletions packages/next/src/server/request/params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@ import {
type PrerenderStore,
} from '../app-render/prerender-async-storage.external'
import { InvariantError } from '../../shared/lib/invariant-error'
import {
makeResolvedReactPromise,
describeStringPropertyAccess,
throwWithStaticGenerationBailoutErrorWithDynamicError,
} from './utils'
import { makeResolvedReactPromise, describeStringPropertyAccess } from './utils'
import { makeHangingPromise } from '../dynamic-rendering-utils'

export type Params = Record<string, string | Array<string> | undefined>
Expand Down Expand Up @@ -259,12 +255,7 @@ function makeErroringExoticParams(
Object.defineProperty(augmentedUnderlying, prop, {
get() {
const expression = describeStringPropertyAccess('params', prop)
if (staticGenerationStore.dynamicShouldError) {
throwWithStaticGenerationBailoutErrorWithDynamicError(
staticGenerationStore.route,
expression
)
} else if (prerenderStore) {
if (prerenderStore) {
postponeWithTracking(
staticGenerationStore.route,
expression,
Expand All @@ -282,12 +273,7 @@ function makeErroringExoticParams(
Object.defineProperty(promise, prop, {
get() {
const expression = describeStringPropertyAccess('params', prop)
if (staticGenerationStore.dynamicShouldError) {
throwWithStaticGenerationBailoutErrorWithDynamicError(
staticGenerationStore.route,
expression
)
} else if (prerenderStore) {
if (prerenderStore) {
postponeWithTracking(
staticGenerationStore.route,
expression,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export const dynamic = 'error'

export default function Layout({ children }) {
return (
<>
<div data-layout={Math.random().toString(16).slice(2)} />
{children}
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { setTimeout } from 'timers/promises'

export default async function Page({ params }) {
await setTimeout(1000)

const { slug } = params

return (
<div>
<div data-slug={slug}>{slug}</div>
This page should be static
</div>
)
}
12 changes: 12 additions & 0 deletions test/e2e/app-dir/ppr-full/ppr-full.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,18 @@ describe('ppr-full', () => {
expect(revalidatedDynamicID).not.toBe(fallbackID)
})
})

/**
* This test is really here to just to force the the suite to have the expected route
* as part of the build. If this failed we'd get a build error and all the tests would fail
*/
it('will allow dynamic fallback shells even when static is enforced', async () => {
const random = Math.random().toString(16).slice(2)
const pathname = `/fallback/dynamic/params/revalidate-${random}`

let $ = await next.render$(pathname)
expect($('[data-slug]').text()).toBe(`revalidate-${random}`)
})
})

it('should allow client layouts without postponing fallback if params are not accessed', async () => {
Expand Down

0 comments on commit 3a6b271

Please sign in to comment.