From df2398d2230e3ab9d25950c0be2272837701b847 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 6 Mar 2023 15:28:08 -0500 Subject: [PATCH 1/8] fix: marks pages in dev as supports dynamic html --- packages/next/src/server/base-server.ts | 8 ++++++- .../app/app/dynamic/[category]/page.js | 16 ++++++++++++++ test/e2e/app-dir/app/index.test.ts | 22 +++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 test/e2e/app-dir/app/app/dynamic/[category]/page.js diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 78b442efc7e64..a7e763d70f3b1 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -1317,6 +1317,11 @@ export default abstract class Server { opts.isBot = isBotRequest } + // In development, we always want to generate dynamic HTML. + if (opts.dev && opts.supportsDynamicHTML === false) { + opts.supportsDynamicHTML = true + } + const defaultLocale = isSSG ? this.nextConfig.i18n?.defaultLocale : query.__nextDefaultLocale @@ -1441,7 +1446,8 @@ export default abstract class Server { let isRevalidate = false const doRender: () => Promise = async () => { - const supportsDynamicHTML = !(isSSG || hasStaticPaths) + // In development, we always want to generate dynamic HTML. + const supportsDynamicHTML = opts.dev || !(isSSG || hasStaticPaths) const match = pathname !== '/_error' && !is404Page && !is500Page diff --git a/test/e2e/app-dir/app/app/dynamic/[category]/page.js b/test/e2e/app-dir/app/app/dynamic/[category]/page.js new file mode 100644 index 0000000000000..338decd6ff5e8 --- /dev/null +++ b/test/e2e/app-dir/app/app/dynamic/[category]/page.js @@ -0,0 +1,16 @@ +import { cookies } from 'next/headers' + +const Page = async () => { + const result = cookies().get('session')?.value ? 'has cookie' : 'no cookie' + return +} + +export const generateStaticParams = async () => { + return [{ category: 'frameworks' }] +} + +export const dynamic = 'force-dynamic' +export const dynamicParams = true +export const revalidate = 60 + +export default Page diff --git a/test/e2e/app-dir/app/index.test.ts b/test/e2e/app-dir/app/index.test.ts index 9e3d2586a6132..ad7e1b71dfa53 100644 --- a/test/e2e/app-dir/app/index.test.ts +++ b/test/e2e/app-dir/app/index.test.ts @@ -749,6 +749,28 @@ createNextDescribe( '{"category":"books","id":"hello-world"}' ) }) + + it('should allow dynamic routes to access cookies', async () => { + for (const category of ['books', 'frameworks']) { + for (let i = 0; i < 2; i++) { + let $ = await next.render$( + `/dynamic/${category}`, + {}, + { + headers: { + cookie: 'session=value', + }, + } + ) + + expect($('#cookie-result').text()).toBe('has cookie') + + $ = await next.render$(`/dynamic/${category}`) + + expect($('#cookie-result').text()).toBe('no cookie') + } + } + }) }) describe('catch-all routes', () => { From 3a6b73a5711837a0b2783a64fe03d0bffe46b653 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 7 Mar 2023 08:28:22 -0500 Subject: [PATCH 2/8] refactor: move test to other test suite --- .../e2e/app-dir/app-static/app-static.test.ts | 20 ++++++++++++++++ .../force-dynamic-prerender/[slug]}/page.js | 23 +++++++++++-------- test/e2e/app-dir/app/index.test.ts | 22 ------------------ 3 files changed, 33 insertions(+), 32 deletions(-) rename test/e2e/app-dir/{app/app/dynamic/[category] => app-static/app/force-dynamic-prerender/[slug]}/page.js (58%) 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 35cfeee3a585f..7b9366212c364 100644 --- a/test/e2e/app-dir/app-static/app-static.test.ts +++ b/test/e2e/app-dir/app-static/app-static.test.ts @@ -791,6 +791,26 @@ createNextDescribe( } }) + it('should allow dynamic routes to access cookies', async () => { + for (const slug of ['books', 'frameworks']) { + for (let i = 0; i < 2; i++) { + let $ = await next.render$( + `/force-dynamic-prerender/${slug}`, + {}, + { headers: { cookie: 'session=value' } } + ) + + expect($('#slug').text()).toBe(slug) + expect($('#cookie-result').text()).toBe('has cookie') + + $ = await next.render$(`/force-dynamic-prerender/${slug}`) + + expect($('#slug').text()).toBe(slug) + expect($('#cookie-result').text()).toBe('no cookie') + } + } + }) + it('should not error with generateStaticParams and dynamic data', async () => { const res = await next.fetch('/gen-params-dynamic/one') const html = await res.text() diff --git a/test/e2e/app-dir/app/app/dynamic/[category]/page.js b/test/e2e/app-dir/app-static/app/force-dynamic-prerender/[slug]/page.js similarity index 58% rename from test/e2e/app-dir/app/app/dynamic/[category]/page.js rename to test/e2e/app-dir/app-static/app/force-dynamic-prerender/[slug]/page.js index 338decd6ff5e8..829fbcb150e2e 100644 --- a/test/e2e/app-dir/app/app/dynamic/[category]/page.js +++ b/test/e2e/app-dir/app-static/app/force-dynamic-prerender/[slug]/page.js @@ -1,16 +1,19 @@ import { cookies } from 'next/headers' -const Page = async () => { - const result = cookies().get('session')?.value ? 'has cookie' : 'no cookie' - return -} - -export const generateStaticParams = async () => { - return [{ category: 'frameworks' }] -} - export const dynamic = 'force-dynamic' export const dynamicParams = true export const revalidate = 60 -export default Page +export const generateStaticParams = async () => { + return [{ slug: 'frameworks' }] +} + +export default function Page({ params }) { + const result = cookies().get('session')?.value ? 'has cookie' : 'no cookie' + return ( +
+
{params.slug}
+ +
+ ) +} diff --git a/test/e2e/app-dir/app/index.test.ts b/test/e2e/app-dir/app/index.test.ts index ad7e1b71dfa53..9e3d2586a6132 100644 --- a/test/e2e/app-dir/app/index.test.ts +++ b/test/e2e/app-dir/app/index.test.ts @@ -749,28 +749,6 @@ createNextDescribe( '{"category":"books","id":"hello-world"}' ) }) - - it('should allow dynamic routes to access cookies', async () => { - for (const category of ['books', 'frameworks']) { - for (let i = 0; i < 2; i++) { - let $ = await next.render$( - `/dynamic/${category}`, - {}, - { - headers: { - cookie: 'session=value', - }, - } - ) - - expect($('#cookie-result').text()).toBe('has cookie') - - $ = await next.render$(`/dynamic/${category}`) - - expect($('#cookie-result').text()).toBe('no cookie') - } - } - }) }) describe('catch-all routes', () => { From 6fbebffdc856c9de03f134bd5522ac375ffd753a Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 8 Mar 2023 10:49:28 +0100 Subject: [PATCH 3/8] Update packages/next/src/server/base-server.ts Co-authored-by: JJ Kasper --- packages/next/src/server/base-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 5218d9233291b..1922a2dba9eef 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -1316,7 +1316,7 @@ export default abstract class Server { } // In development, we always want to generate dynamic HTML. - if (opts.dev && opts.supportsDynamicHTML === false) { + if (isAppDir && opts.dev && opts.supportsDynamicHTML === false) { opts.supportsDynamicHTML = true } From 1414e26a66181959a2a9b64502c4c0cb486cac05 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 8 Mar 2023 12:53:43 -0800 Subject: [PATCH 4/8] fix --- packages/next/src/server/base-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 1922a2dba9eef..053ac03f33819 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -1316,7 +1316,7 @@ export default abstract class Server { } // In development, we always want to generate dynamic HTML. - if (isAppDir && opts.dev && opts.supportsDynamicHTML === false) { + if (isAppPath && opts.dev && opts.supportsDynamicHTML === false) { opts.supportsDynamicHTML = true } From 47b031d9e27c3a5ad57c743e83a81849f2a14b4b Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 8 Mar 2023 13:05:22 -0800 Subject: [PATCH 5/8] update test --- test/e2e/app-dir/app-static/app-static.test.ts | 1 + 1 file changed, 1 insertion(+) 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 294ad6e97adb6..c2f7223482bc9 100644 --- a/test/e2e/app-dir/app-static/app-static.test.ts +++ b/test/e2e/app-dir/app-static/app-static.test.ts @@ -69,6 +69,7 @@ createNextDescribe( 'dynamic-no-gen-params-ssr/[slug]/page.js', 'dynamic-no-gen-params/[slug]/page.js', 'force-dynamic-no-prerender/[id]/page.js', + 'force-dynamic-prerender/[slug]/page.js', 'force-static/[slug]/page.js', 'force-static/first.html', 'force-static/first.rsc', From 20f45e5a2c1a932e6eaebf9e18ca0dce1ac741d3 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 16 Mar 2023 15:52:01 -0600 Subject: [PATCH 6/8] fix: exclude data routes from dynamic html --- packages/next/src/server/base-server.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index cf80e93322e09..f303ad4a172b3 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -1318,7 +1318,12 @@ export default abstract class Server { } // In development, we always want to generate dynamic HTML. - if (isAppPath && opts.dev && opts.supportsDynamicHTML === false) { + if ( + !isDataReq && + isAppPath && + opts.dev && + opts.supportsDynamicHTML === false + ) { opts.supportsDynamicHTML = true } @@ -1447,7 +1452,8 @@ export default abstract class Server { const doRender: () => Promise = async () => { // In development, we always want to generate dynamic HTML. - const supportsDynamicHTML = opts.dev || !(isSSG || hasStaticPaths) + const supportsDynamicHTML = + (!isDataReq && opts.dev) || !(isSSG || hasStaticPaths) const match = pathname !== '/_error' && !is404Page && !is500Page From 9c193d7459980042b64dab571ada9b5e660dff5f Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 17 Mar 2023 15:32:58 -0600 Subject: [PATCH 7/8] fix: handle cases for fallback pages in app --- packages/next/src/server/base-server.ts | 41 ++++++++++++------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index f303ad4a172b3..39cfa6a265b7d 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -1655,43 +1655,42 @@ export default abstract class Server { fallbackMode = 'blocking' } + // We use `ssgCacheKey` here as it is normalized to match the encoding + // from getStaticPaths along with including the locale. + // + // We use the `resolvedUrlPathname` for the development case when this + // is an app path since it doesn't include locale information. + let staticPathKey = + ssgCacheKey ?? (opts.dev && isAppPath ? resolvedUrlPathname : null) + if (staticPathKey && query.amp) staticPathKey.replace(/\.amp$/, '') + const isPageIncludedInStaticPaths = + staticPathKey && staticPaths?.includes(staticPathKey) + // When we did not respond from cache, we need to choose to block on // rendering or return a skeleton. // - // * Data requests always block. - // - // * Blocking mode fallback always blocks. - // - // * Preview mode toggles all pages to be resolved in a blocking manner. - // - // * Non-dynamic pages should block (though this is an impossible + // - Data requests always block. + // - Blocking mode fallback always blocks. + // - Preview mode toggles all pages to be resolved in a blocking manner. + // - Non-dynamic pages should block (though this is an impossible // case in production). - // - // * Dynamic pages should return their skeleton if not defined in + // - Dynamic pages should return their skeleton if not defined in // getStaticPaths, then finish the data request on the client-side. // if ( process.env.NEXT_RUNTIME !== 'edge' && - this.minimalMode !== true && + !this.minimalMode && fallbackMode !== 'blocking' && - ssgCacheKey && + staticPathKey && !didRespond && !isPreviewMode && isDynamicPathname && - // Development should trigger fallback when the path is not in - // `getStaticPaths` - (isProduction || - !staticPaths || - !staticPaths.includes( - // we use ssgCacheKey here as it is normalized to match the - // encoding from getStaticPaths along with including the locale - query.amp ? ssgCacheKey.replace(/\.amp$/, '') : ssgCacheKey - )) + (isProduction || !staticPaths || !isPageIncludedInStaticPaths) ) { if ( // In development, fall through to render to handle missing // getStaticPaths. - (isProduction || staticPaths) && + (isProduction || (staticPaths && staticPaths?.length > 0)) && // When fallback isn't present, abort this render so we 404 fallbackMode !== 'static' ) { From 13dfdd3c59c5e786c0851ec67e6afacdf6eea804 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 17 Mar 2023 16:07:57 -0600 Subject: [PATCH 8/8] fix: fix typo for amp handling --- packages/next/src/server/base-server.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 8a2e8c9247640..f83dea4a8811e 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -1652,7 +1652,10 @@ export default abstract class Server { // is an app path since it doesn't include locale information. let staticPathKey = ssgCacheKey ?? (opts.dev && isAppPath ? resolvedUrlPathname : null) - if (staticPathKey && query.amp) staticPathKey.replace(/\.amp$/, '') + if (staticPathKey && query.amp) { + staticPathKey = staticPathKey.replace(/\.amp$/, '') + } + const isPageIncludedInStaticPaths = staticPathKey && staticPaths?.includes(staticPathKey)