From ff05d9b7b51cb803d75ba75bf22209db12babafd Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 22 Feb 2023 23:59:19 +0100 Subject: [PATCH] Skip pre-rendering the default param when no params are provided (#46265) Closes #45850. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm build && pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md) --------- Co-authored-by: JJ Kasper --- packages/next/src/build/index.ts | 41 ++++++++++++------ .../e2e/app-dir/app-static/app-static.test.ts | 43 ------------------- 2 files changed, 29 insertions(+), 55 deletions(-) diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index d4255c6735595..f897fed7dc12f 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -1434,24 +1434,41 @@ export default async function build( ssgPageRoutes = workerResult.prerenderRoutes isSsg = true } - if ( - (!isDynamicRoute(page) || - !workerResult.prerenderRoutes?.length) && - workerResult.appConfig?.revalidate !== 0 - ) { - appStaticPaths.set(originalAppPath, [page]) - appStaticPathsEncoded.set(originalAppPath, [page]) - isStatic = true + + const appConfig = workerResult.appConfig || {} + if (workerResult.appConfig?.revalidate !== 0) { + const isDynamic = isDynamicRoute(page) + const hasGenerateStaticParams = + !!workerResult.prerenderRoutes?.length + + if ( + // Mark the app as static if: + // - It has no dynamic param + // - It doesn't have generateStaticParams but `dynamic` is set to + // `error` or `force-static` + !isDynamic + ) { + appStaticPaths.set(originalAppPath, [page]) + appStaticPathsEncoded.set(originalAppPath, [page]) + isStatic = true + } else if ( + isDynamic && + !hasGenerateStaticParams && + (appConfig.dynamic === 'error' || + appConfig.dynamic === 'force-static') + ) { + appStaticPaths.set(originalAppPath, []) + appStaticPathsEncoded.set(originalAppPath, []) + isStatic = true + } } + if (workerResult.prerenderFallback) { // whether or not to allow requests for paths not // returned from generateStaticParams appDynamicParamPaths.add(originalAppPath) } - appDefaultConfigs.set( - originalAppPath, - workerResult.appConfig || {} - ) + appDefaultConfigs.set(originalAppPath, appConfig) } } else { if (isEdgeRuntime(pageRuntime)) { diff --git a/test/e2e/app-dir/app-static/app-static.test.ts b/test/e2e/app-dir/app-static/app-static.test.ts index a42d5a6df6551..e71e73363fbce 100644 --- a/test/e2e/app-dir/app-static/app-static.test.ts +++ b/test/e2e/app-dir/app-static/app-static.test.ts @@ -43,12 +43,8 @@ createNextDescribe( 'blog/tim.rsc', 'blog/tim/first-post.html', 'blog/tim/first-post.rsc', - 'dynamic-error/[id].html', - 'dynamic-error/[id].rsc', 'dynamic-error/[id]/page.js', 'dynamic-no-gen-params-ssr/[slug]/page.js', - 'dynamic-no-gen-params/[slug].html', - 'dynamic-no-gen-params/[slug].rsc', 'dynamic-no-gen-params/[slug]/page.js', 'force-dynamic-no-prerender/[id]/page.js', 'force-static/[slug]/page.js', @@ -79,11 +75,7 @@ createNextDescribe( 'ssr-auto/cache-no-store/page.js', 'ssr-auto/fetch-revalidate-zero/page.js', 'ssr-forced/page.js', - 'static-to-dynamic-error-forced/[id].html', - 'static-to-dynamic-error-forced/[id].rsc', 'static-to-dynamic-error-forced/[id]/page.js', - 'static-to-dynamic-error/[id].html', - 'static-to-dynamic-error/[id].rsc', 'static-to-dynamic-error/[id]/page.js', 'variable-revalidate-edge/no-store/page.js', 'variable-revalidate-edge/revalidate-3/page.js', @@ -217,16 +209,6 @@ createNextDescribe( fallback: null, routeRegex: '^\\/dynamic\\-error\\/([^\\/]+?)(?:\\/)?$', }, - '/dynamic-no-gen-params/[slug]': { - dataRoute: '/dynamic-no-gen-params/[slug].rsc', - dataRouteRegex: normalizeRegEx( - '^\\/dynamic\\-no\\-gen\\-params\\/([^\\/]+?)\\.rsc$' - ), - fallback: null, - routeRegex: normalizeRegEx( - '^\\/dynamic\\-no\\-gen\\-params\\/([^\\/]+?)(?:\\/)?$' - ), - }, '/hooks/use-pathname/[slug]': { dataRoute: '/hooks/use-pathname/[slug].rsc', dataRouteRegex: normalizeRegEx( @@ -267,16 +249,6 @@ createNextDescribe( '^\\/static\\-to\\-dynamic\\-error\\-forced\\/([^\\/]+?)(?:\\/)?$' ), }, - '/static-to-dynamic-error/[id]': { - dataRoute: '/static-to-dynamic-error/[id].rsc', - dataRouteRegex: normalizeRegEx( - '^\\/static\\-to\\-dynamic\\-error\\/([^\\/]+?)\\.rsc$' - ), - fallback: null, - routeRegex: normalizeRegEx( - '^\\/static\\-to\\-dynamic\\-error\\/([^\\/]+?)(?:\\/)?$' - ), - }, }) }) @@ -324,21 +296,6 @@ createNextDescribe( expect($2('#page-data').text()).not.toBe(pageData) }) } else { - it('should properly error when static page switches to dynamic at runtime', async () => { - const res = await next.fetch( - '/static-to-dynamic-error/static-bailout-1' - ) - - expect(res.status).toBe(500) - - if (isNextStart) { - await check( - () => stripAnsi(next.cliOutput), - /Page changed from static to dynamic at runtime \/static-to-dynamic-error\/static-bailout-1, reason: cookies/ - ) - } - }) - it('should not error with dynamic server usage with force-static', async () => { const res = await next.fetch( '/static-to-dynamic-error-forced/static-bailout-1'