fallback shell should not error when dynamic due to params access even with dynamic = "error" (#70534)

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

Author: gnoff

packages/next/src/server/request/params.ts

@@ -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>
@@ -259,12 +255,13 @@ function makeErroringExoticParams(
           Object.defineProperty(augmentedUnderlying, prop, {
             get() {
               const expression = describeStringPropertyAccess('params', prop)
-              if (staticGenerationStore.dynamicShouldError) {
-                throwWithStaticGenerationBailoutErrorWithDynamicError(
-                  staticGenerationStore.route,
-                  expression
-                )
-              } else if (prerenderStore) {
+              // In most dynamic APIs we also throw if `dynamic = "error"` however
+              // for params is only dynamic when we're generating a fallback shell
+              // and even when `dynamic = "error"` we still support generating dynamic
+              // fallback shells
+              // TODO remove this comment when dynamicIO is the default since there
+              // will be no `dynamic = "error"`
+              if (prerenderStore) {
                 postponeWithTracking(
                   staticGenerationStore.route,
                   expression,
@@ -282,12 +279,13 @@ function makeErroringExoticParams(
           Object.defineProperty(promise, prop, {
             get() {
               const expression = describeStringPropertyAccess('params', prop)
-              if (staticGenerationStore.dynamicShouldError) {
-                throwWithStaticGenerationBailoutErrorWithDynamicError(
-                  staticGenerationStore.route,
-                  expression
-                )
-              } else if (prerenderStore) {
+              // In most dynamic APIs we also throw if `dynamic = "error"` however
+              // for params is only dynamic when we're generating a fallback shell
+              // and even when `dynamic = "error"` we still support generating dynamic
+              // fallback shells
+              // TODO remove this comment when dynamicIO is the default since there
+              // will be no `dynamic = "error"`
+              if (prerenderStore) {
                 postponeWithTracking(
                   staticGenerationStore.route,
                   expression,

test/e2e/app-dir/ppr-full/app/fallback/dynamic/error/[slug]/layout.jsx

@@ -0,0 +1,10 @@
+export const dynamic = 'error'
+
+export default function Layout({ children }) {
+  return (
+    <>
+      <div data-layout={Math.random().toString(16).slice(2)} />
+      {children}
+    </>
+  )
+}

test/e2e/app-dir/ppr-full/app/fallback/dynamic/error/[slug]/page.jsx

@@ -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>
+  )
+}

test/e2e/app-dir/ppr-full/ppr-full.test.ts

@@ -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 () => {