From 4b4b9a725f9a6bb54babb5fd5b8e0f9c3e98cb3f Mon Sep 17 00:00:00 2001 From: pieh Date: Wed, 9 Oct 2024 17:24:14 +0200 Subject: [PATCH 1/6] test: add test case for fallback: true --- tests/e2e/page-router.test.ts | 146 +++++++++++++++--- .../pages/fallback-true/[slug].js | 43 ++++++ .../page-router/pages/fallback-true/[slug].js | 43 ++++++ tests/integration/cache-handler.test.ts | 3 + tests/integration/static.test.ts | 3 + 5 files changed, 217 insertions(+), 21 deletions(-) create mode 100644 tests/fixtures/page-router-base-path-i18n/pages/fallback-true/[slug].js create mode 100644 tests/fixtures/page-router/pages/fallback-true/[slug].js diff --git a/tests/e2e/page-router.test.ts b/tests/e2e/page-router.test.ts index 2124ba53c7..92cab2ac1b 100644 --- a/tests/e2e/page-router.test.ts +++ b/tests/e2e/page-router.test.ts @@ -51,30 +51,41 @@ export async function check( test.describe('Simple Page Router (no basePath, no i18n)', () => { test.describe('On-demand revalidate works correctly', () => { - for (const { label, prerendered, pagePath, revalidateApiBasePath, expectedH1Content } of [ + for (const { + label, + useFallback, + prerendered, + pagePath, + revalidateApiBasePath, + expectedH1Content, + } of [ { - label: 'prerendered page with static path and awaited res.revalidate()', + label: + 'prerendered page with static path with fallback: blocking and awaited res.revalidate()', prerendered: true, pagePath: '/static/revalidate-manual', revalidateApiBasePath: '/api/revalidate', expectedH1Content: 'Show #71', }, { - label: 'prerendered page with dynamic path and awaited res.revalidate()', + label: + 'prerendered page with dynamic path with fallback: blocking and awaited res.revalidate()', prerendered: true, pagePath: '/products/prerendered', revalidateApiBasePath: '/api/revalidate', expectedH1Content: 'Product prerendered', }, { - label: 'not prerendered page with dynamic path and awaited res.revalidate()', + label: + 'not prerendered page with dynamic path with fallback: blocking and awaited res.revalidate()', prerendered: false, pagePath: '/products/not-prerendered', revalidateApiBasePath: '/api/revalidate', expectedH1Content: 'Product not-prerendered', }, { - label: 'not prerendered page with dynamic path and not awaited res.revalidate()', + label: + 'not prerendered page with dynamic path with fallback: blocking and not awaited res.revalidate()', prerendered: false, pagePath: '/products/not-prerendered-and-not-awaited-revalidation', revalidateApiBasePath: '/api/revalidate-no-await', @@ -82,7 +93,7 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => { }, { label: - 'prerendered page with dynamic path and awaited res.revalidate() - non-ASCII variant', + 'prerendered page with dynamic path with fallback: blocking and awaited res.revalidate() - non-ASCII variant', prerendered: true, pagePath: '/products/事前レンダリング,test', revalidateApiBasePath: '/api/revalidate', @@ -90,12 +101,29 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => { }, { label: - 'not prerendered page with dynamic path and awaited res.revalidate() - non-ASCII variant', + 'not prerendered page with dynamic path with fallback: blocking and awaited res.revalidate() - non-ASCII variant', prerendered: false, pagePath: '/products/事前レンダリングされていない,test', revalidateApiBasePath: '/api/revalidate', expectedH1Content: 'Product 事前レンダリングされていない,test', }, + { + label: + 'prerendered page with dynamic path with fallback: true and awaited res.revalidate()', + prerendered: true, + pagePath: '/fallback-true/prerendered', + revalidateApiBasePath: '/api/revalidate', + expectedH1Content: 'Product prerendered', + }, + { + label: + 'not prerendered page with dynamic path with fallback: true and awaited res.revalidate()', + prerendered: false, + useFallback: true, + pagePath: '/fallback-true/not-prerendered', + revalidateApiBasePath: '/api/revalidate', + expectedH1Content: 'Product not-prerendered', + }, ]) { test(label, async ({ page, pollUntilHeadersMatch, pageRouter }) => { // in case there is retry or some other test did hit that path before @@ -126,11 +154,23 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => { const headers1 = response1?.headers() || {} expect(response1?.status()).toBe(200) expect(headers1['x-nextjs-cache']).toBeUndefined() - expect(headers1['netlify-cache-tag']).toBe(`_n_t_${encodeURI(pagePath).toLowerCase()}`) + + const fallbackWasServed = + useFallback && headers1['cache-status'].includes('"Next.js"; fwd=miss') + expect(headers1['netlify-cache-tag']).toBe( + fallbackWasServed ? undefined : `_n_t_${encodeURI(pagePath).toLowerCase()}`, + ) expect(headers1['netlify-cdn-cache-control']).toBe( - 's-maxage=31536000, stale-while-revalidate=31536000, durable', + fallbackWasServed + ? undefined + : 's-maxage=31536000, stale-while-revalidate=31536000, durable', ) + if (fallbackWasServed) { + const loading = await page.textContent('[data-testid="loading"]') + expect(loading, 'Fallback should be shown').toBe('Loading...') + } + const date1 = await page.textContent('[data-testid="date-now"]') const h1 = await page.textContent('h1') expect(h1).toBe(expectedH1Content) @@ -446,7 +486,14 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => { test.describe('Page Router with basePath and i18n', () => { test.describe('Static revalidate works correctly', () => { - for (const { label, prerendered, pagePath, revalidateApiBasePath, expectedH1Content } of [ + for (const { + label, + useFallback, + prerendered, + pagePath, + revalidateApiBasePath, + expectedH1Content, + } of [ { label: 'prerendered page with static path and awaited res.revalidate()', prerendered: true, @@ -455,21 +502,24 @@ test.describe('Page Router with basePath and i18n', () => { expectedH1Content: 'Show #71', }, { - label: 'prerendered page with dynamic path and awaited res.revalidate()', + label: + 'prerendered page with dynamic path with fallback: blocking and awaited res.revalidate()', prerendered: true, pagePath: '/products/prerendered', revalidateApiBasePath: '/api/revalidate', expectedH1Content: 'Product prerendered', }, { - label: 'not prerendered page with dynamic path and awaited res.revalidate()', + label: + 'not prerendered page with dynamic path with fallback: blocking and awaited res.revalidate()', prerendered: false, pagePath: '/products/not-prerendered', revalidateApiBasePath: '/api/revalidate', expectedH1Content: 'Product not-prerendered', }, { - label: 'not prerendered page with dynamic path and not awaited res.revalidate()', + label: + 'not prerendered page with dynamic path with fallback: blocking and not awaited res.revalidate()', prerendered: false, pagePath: '/products/not-prerendered-and-not-awaited-revalidation', revalidateApiBasePath: '/api/revalidate-no-await', @@ -477,7 +527,7 @@ test.describe('Page Router with basePath and i18n', () => { }, { label: - 'prerendered page with dynamic path and awaited res.revalidate() - non-ASCII variant', + 'prerendered page with dynamic path with fallback: blocking and awaited res.revalidate() - non-ASCII variant', prerendered: true, pagePath: '/products/事前レンダリング,test', revalidateApiBasePath: '/api/revalidate', @@ -485,12 +535,29 @@ test.describe('Page Router with basePath and i18n', () => { }, { label: - 'not prerendered page with dynamic path and awaited res.revalidate() - non-ASCII variant', + 'not prerendered page with dynamic path with fallback: blocking and awaited res.revalidate() - non-ASCII variant', prerendered: false, pagePath: '/products/事前レンダリングされていない,test', revalidateApiBasePath: '/api/revalidate', expectedH1Content: 'Product 事前レンダリングされていない,test', }, + { + label: + 'prerendered page with dynamic path with fallback: true and awaited res.revalidate()', + prerendered: true, + pagePath: '/fallback-true/prerendered', + revalidateApiBasePath: '/api/revalidate', + expectedH1Content: 'Product prerendered', + }, + { + label: + 'not prerendered page with dynamic path with fallback: true and awaited res.revalidate()', + prerendered: false, + useFallback: true, + pagePath: '/fallback-true/not-prerendered', + revalidateApiBasePath: '/api/revalidate', + expectedH1Content: 'Product not-prerendered', + }, ]) { test.describe(label, () => { test(`default locale`, async ({ page, pollUntilHeadersMatch, pageRouterBasePathI18n }) => { @@ -528,13 +595,26 @@ test.describe('Page Router with basePath and i18n', () => { const headers1ImplicitLocale = response1ImplicitLocale?.headers() || {} expect(response1ImplicitLocale?.status()).toBe(200) expect(headers1ImplicitLocale['x-nextjs-cache']).toBeUndefined() + + const fallbackWasServedImplicitLocale = + useFallback && headers1ImplicitLocale['cache-status'].includes('"Next.js"; fwd=miss') + expect(headers1ImplicitLocale['netlify-cache-tag']).toBe( - `_n_t_/en${encodeURI(pagePath).toLowerCase()}`, + fallbackWasServedImplicitLocale + ? undefined + : `_n_t_/en${encodeURI(pagePath).toLowerCase()}`, ) expect(headers1ImplicitLocale['netlify-cdn-cache-control']).toBe( - 's-maxage=31536000, stale-while-revalidate=31536000, durable', + fallbackWasServedImplicitLocale + ? undefined + : 's-maxage=31536000, stale-while-revalidate=31536000, durable', ) + if (fallbackWasServedImplicitLocale) { + const loading = await page.textContent('[data-testid="loading"]') + expect(loading, 'Fallback should be shown').toBe('Loading...') + } + const date1ImplicitLocale = await page.textContent('[data-testid="date-now"]') const h1ImplicitLocale = await page.textContent('h1') expect(h1ImplicitLocale).toBe(expectedH1Content) @@ -556,13 +636,25 @@ test.describe('Page Router with basePath and i18n', () => { const headers1ExplicitLocale = response1ExplicitLocale?.headers() || {} expect(response1ExplicitLocale?.status()).toBe(200) expect(headers1ExplicitLocale['x-nextjs-cache']).toBeUndefined() + + const fallbackWasServedExplicitLocale = + useFallback && headers1ExplicitLocale['cache-status'].includes('"Next.js"; fwd=miss') expect(headers1ExplicitLocale['netlify-cache-tag']).toBe( - `_n_t_/en${encodeURI(pagePath).toLowerCase()}`, + fallbackWasServedExplicitLocale + ? undefined + : `_n_t_/en${encodeURI(pagePath).toLowerCase()}`, ) expect(headers1ExplicitLocale['netlify-cdn-cache-control']).toBe( - 's-maxage=31536000, stale-while-revalidate=31536000, durable', + fallbackWasServedExplicitLocale + ? undefined + : 's-maxage=31536000, stale-while-revalidate=31536000, durable', ) + if (fallbackWasServedExplicitLocale) { + const loading = await page.textContent('[data-testid="loading"]') + expect(loading, 'Fallback should be shown').toBe('Loading...') + } + const date1ExplicitLocale = await page.textContent('[data-testid="date-now"]') const h1ExplicitLocale = await page.textContent('h1') expect(h1ExplicitLocale).toBe(expectedH1Content) @@ -910,11 +1002,23 @@ test.describe('Page Router with basePath and i18n', () => { const headers1 = response1?.headers() || {} expect(response1?.status()).toBe(200) expect(headers1['x-nextjs-cache']).toBeUndefined() - expect(headers1['netlify-cache-tag']).toBe(`_n_t_/de${encodeURI(pagePath).toLowerCase()}`) + + const fallbackWasServed = + useFallback && headers1['cache-status'].includes('"Next.js"; fwd=miss') + expect(headers1['netlify-cache-tag']).toBe( + fallbackWasServed ? undefined : `_n_t_/de${encodeURI(pagePath).toLowerCase()}`, + ) expect(headers1['netlify-cdn-cache-control']).toBe( - 's-maxage=31536000, stale-while-revalidate=31536000, durable', + fallbackWasServed + ? undefined + : 's-maxage=31536000, stale-while-revalidate=31536000, durable', ) + if (fallbackWasServed) { + const loading = await page.textContent('[data-testid="loading"]') + expect(loading, 'Fallback should be shown').toBe('Loading...') + } + const date1 = await page.textContent('[data-testid="date-now"]') const h1 = await page.textContent('h1') expect(h1).toBe(expectedH1Content) diff --git a/tests/fixtures/page-router-base-path-i18n/pages/fallback-true/[slug].js b/tests/fixtures/page-router-base-path-i18n/pages/fallback-true/[slug].js new file mode 100644 index 0000000000..5e85c57657 --- /dev/null +++ b/tests/fixtures/page-router-base-path-i18n/pages/fallback-true/[slug].js @@ -0,0 +1,43 @@ +import { useRouter } from 'next/router' + +const Product = ({ time, slug }) => { + const router = useRouter() + + if (router.isFallback) { + return Loading... + } + + return ( +
+

Product {slug}

+

+ This page uses getStaticProps() and getStaticPaths() to pre-fetch a Product + {time} +

+
+ ) +} + +export async function getStaticProps({ params }) { + return { + props: { + time: new Date().toISOString(), + slug: params.slug, + }, + } +} + +export const getStaticPaths = () => { + return { + paths: [ + { + params: { + slug: 'prerendered', + }, + }, + ], + fallback: true, + } +} + +export default Product diff --git a/tests/fixtures/page-router/pages/fallback-true/[slug].js b/tests/fixtures/page-router/pages/fallback-true/[slug].js new file mode 100644 index 0000000000..5e85c57657 --- /dev/null +++ b/tests/fixtures/page-router/pages/fallback-true/[slug].js @@ -0,0 +1,43 @@ +import { useRouter } from 'next/router' + +const Product = ({ time, slug }) => { + const router = useRouter() + + if (router.isFallback) { + return Loading... + } + + return ( +
+

Product {slug}

+

+ This page uses getStaticProps() and getStaticPaths() to pre-fetch a Product + {time} +

+
+ ) +} + +export async function getStaticProps({ params }) { + return { + props: { + time: new Date().toISOString(), + slug: params.slug, + }, + } +} + +export const getStaticPaths = () => { + return { + paths: [ + { + params: { + slug: 'prerendered', + }, + }, + ], + fallback: true, + } +} + +export default Product diff --git a/tests/integration/cache-handler.test.ts b/tests/integration/cache-handler.test.ts index 42397a34ff..6dc7a41a37 100644 --- a/tests/integration/cache-handler.test.ts +++ b/tests/integration/cache-handler.test.ts @@ -43,6 +43,8 @@ describe('page router', () => { // check if the blob entries where successful set on the build plugin const blobEntries = await getBlobEntries(ctx) expect(blobEntries.map(({ key }) => decodeBlobKey(key.substring(0, 50))).sort()).toEqual([ + '/fallback-true/[slug]', + '/fallback-true/prerendered', // the real key is much longer and ends in a hash, but we only assert on the first 50 chars to make it easier '/products/an-incredibly-long-product-', '/products/prerendered', @@ -53,6 +55,7 @@ describe('page router', () => { '/static/revalidate-slow-data', '404.html', '500.html', + 'fallback-true/[slug].html', 'static/fully-static.html', ]) diff --git a/tests/integration/static.test.ts b/tests/integration/static.test.ts index c5457eac07..c0fb42fd74 100644 --- a/tests/integration/static.test.ts +++ b/tests/integration/static.test.ts @@ -35,6 +35,8 @@ test('requesting a non existing page route that needs to be const entries = await getBlobEntries(ctx) expect(entries.map(({ key }) => decodeBlobKey(key.substring(0, 50))).sort()).toEqual([ + '/fallback-true/[slug]', + '/fallback-true/prerendered', '/products/an-incredibly-long-product-', '/products/prerendered', '/products/事前レンダリング,te', @@ -44,6 +46,7 @@ test('requesting a non existing page route that needs to be '/static/revalidate-slow-data', '404.html', '500.html', + 'fallback-true/[slug].html', 'static/fully-static.html', // the real key is much longer and ends in a hash, but we only assert on the first 50 chars to make it easier ]) From 057d33964d307dcd3c65799d3fd4e807549f66a6 Mon Sep 17 00:00:00 2001 From: pieh Date: Tue, 8 Oct 2024 15:04:46 +0200 Subject: [PATCH 2/6] fix: create cache entries for fallback pages to support next@canary --- src/build/content/prerendered.ts | 58 ++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/src/build/content/prerendered.ts b/src/build/content/prerendered.ts index 6c45a74472..e7dce0d61f 100644 --- a/src/build/content/prerendered.ts +++ b/src/build/content/prerendered.ts @@ -1,6 +1,7 @@ import { existsSync } from 'node:fs' import { mkdir, readFile, writeFile } from 'node:fs/promises' import { join } from 'node:path' +import { join as posixJoin } from 'node:path/posix' import { trace } from '@opentelemetry/api' import { wrapTracer } from '@opentelemetry/api/experimental' @@ -41,17 +42,28 @@ const writeCacheEntry = async ( } /** - * Normalize routes by stripping leading slashes and ensuring root path is index + * Normalize routes by ensuring leading slashes and ensuring root path is index */ -const routeToFilePath = (path: string) => (path === '/' ? '/index' : path) +const routeToFilePath = (path: string) => { + if (path === '/') { + return '/index' + } + + if (path.startsWith('/')) { + return path + } + + return `/${path}` +} const buildPagesCacheValue = async ( path: string, shouldUseEnumKind: boolean, + shouldSkipJson = false, ): Promise => ({ kind: shouldUseEnumKind ? 'PAGES' : 'PAGE', html: await readFile(`${path}.html`, 'utf-8'), - pageData: JSON.parse(await readFile(`${path}.json`, 'utf-8')), + pageData: shouldSkipJson ? {} : JSON.parse(await readFile(`${path}.json`, 'utf-8')), headers: undefined, status: undefined, }) @@ -146,8 +158,8 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise }) : false - await Promise.all( - Object.entries(manifest.routes).map( + await Promise.all([ + ...Object.entries(manifest.routes).map( ([route, meta]): Promise => limitConcurrentPrerenderContentHandling(async () => { const lastModified = meta.initialRevalidateSeconds @@ -195,7 +207,41 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise await writeCacheEntry(key, value, lastModified, ctx) }), ), - ) + ...Object.entries(manifest.dynamicRoutes).map(async ([route, meta]) => { + // fallback can be `string | false | null` + // - `string` - when user use pages router with `fallback: true`, and then it's html file path + // - `null` - when user use pages router with `fallback: 'block'` or app router with `export const dynamicParams = true` + // - `false` - when user use pages router with `fallback: false` or app router with `export const dynamicParams = false` + if (typeof meta.fallback === 'string') { + // https://github.com/vercel/next.js/pull/68603 started using route cache to serve fallbacks + // so we have to seed blobs with fallback entries + + // create cache entry for pages router with `fallback: true` case + await limitConcurrentPrerenderContentHandling(async () => { + // dynamic routes don't have entries for each locale so we have to generate them + // ourselves. If i18n is not used we use empty string as "locale" to be able to use + // same handling wether i18n is used or not + const locales = ctx.buildConfig.i18n?.locales ?? [''] + + const lastModified = Date.now() + for (const locale of locales) { + const key = routeToFilePath(posixJoin(locale, route)) + const value = await buildPagesCacheValue( + join(ctx.publishDir, 'server/pages', key), + shouldUseEnumKind, + true, // there is no corresponding json file for fallback, so we are skipping it for this entry + ) + // Netlify Forms are not support and require a workaround + if (value.kind === 'PAGE' || value.kind === 'PAGES' || value.kind === 'APP_PAGE') { + verifyNetlifyForms(ctx, value.html) + } + + await writeCacheEntry(key, value, lastModified, ctx) + } + }) + } + }), + ]) // app router 404 pages are not in the prerender manifest // so we need to check for them manually From df703e86dcc0ed17828fbc1c906a63befe22c03d Mon Sep 17 00:00:00 2001 From: pieh Date: Wed, 9 Oct 2024 17:23:58 +0200 Subject: [PATCH 3/6] fix: don't permamently cache fallback html --- src/build/content/prerendered.ts | 13 ++++++++++ src/run/constants.ts | 1 + src/run/handlers/request-context.cts | 2 +- src/run/headers.test.ts | 8 +++--- src/run/headers.ts | 7 ++++-- src/run/next.cts | 37 +++++++++++++++++++++++----- 6 files changed, 55 insertions(+), 13 deletions(-) diff --git a/src/build/content/prerendered.ts b/src/build/content/prerendered.ts index e7dce0d61f..03854c6104 100644 --- a/src/build/content/prerendered.ts +++ b/src/build/content/prerendered.ts @@ -9,6 +9,8 @@ import { glob } from 'fast-glob' import pLimit from 'p-limit' import { satisfies } from 'semver' +import { FS_BLOBS_MANIFEST } from '../../run/constants.js' +import { type FSBlobsManifest } from '../../run/next.cjs' import { encodeBlobKey } from '../../shared/blobkey.js' import type { CachedFetchValueForMultipleVersions, @@ -158,6 +160,11 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise }) : false + const fsBlobsManifest: FSBlobsManifest = { + fallbackPaths: [], + outputRoot: ctx.distDir, + } + await Promise.all([ ...Object.entries(manifest.routes).map( ([route, meta]): Promise => @@ -237,6 +244,8 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise } await writeCacheEntry(key, value, lastModified, ctx) + + fsBlobsManifest.fallbackPaths.push(`${key}.html`) } }) } @@ -254,6 +263,10 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise ) await writeCacheEntry(key, value, lastModified, ctx) } + await writeFile( + join(ctx.serverHandlerDir, FS_BLOBS_MANIFEST), + JSON.stringify(fsBlobsManifest), + ) } catch (error) { ctx.failBuild('Failed assembling prerendered content for upload', error) } diff --git a/src/run/constants.ts b/src/run/constants.ts index ebf5daaa32..c27f338b3e 100644 --- a/src/run/constants.ts +++ b/src/run/constants.ts @@ -5,3 +5,4 @@ export const MODULE_DIR = fileURLToPath(new URL('.', import.meta.url)) export const PLUGIN_DIR = resolve(`${MODULE_DIR}../../..`) // a file where we store the required-server-files config object in to access during runtime export const RUN_CONFIG = 'run-config.json' +export const FS_BLOBS_MANIFEST = 'fs-blobs-manifest.json' diff --git a/src/run/handlers/request-context.cts b/src/run/handlers/request-context.cts index c54427f74a..b71969327e 100644 --- a/src/run/handlers/request-context.cts +++ b/src/run/handlers/request-context.cts @@ -11,7 +11,7 @@ export type RequestContext = { responseCacheGetLastModified?: number responseCacheKey?: string responseCacheTags?: string[] - usedFsRead?: boolean + usedFsReadForNonFallback?: boolean didPagesRouterOnDemandRevalidate?: boolean serverTiming?: string routeHandlerRevalidate?: NetlifyCachedRouteValue['revalidate'] diff --git a/src/run/headers.test.ts b/src/run/headers.test.ts index 6e09f8958a..5afcd2e502 100644 --- a/src/run/headers.test.ts +++ b/src/run/headers.test.ts @@ -321,7 +321,7 @@ describe('headers', () => { }) }) - test('should not set any headers if "cache-control" is not set and "requestContext.usedFsRead" is not truthy', () => { + test('should not set any headers if "cache-control" is not set and "requestContext.usedFsReadForNonFallback" is not truthy', () => { const headers = new Headers() const request = new Request(defaultUrl) vi.spyOn(headers, 'set') @@ -331,15 +331,15 @@ describe('headers', () => { expect(headers.set).toHaveBeenCalledTimes(0) }) - test('should set permanent, durable "netlify-cdn-cache-control" if "cache-control" is not set and "requestContext.usedFsRead" is truthy', () => { + test('should set permanent, durable "netlify-cdn-cache-control" if "cache-control" is not set and "requestContext.usedFsReadForNonFallback" is truthy', () => { const headers = new Headers() const request = new Request(defaultUrl) vi.spyOn(headers, 'set') const requestContext = createRequestContext() - requestContext.usedFsRead = true + requestContext.usedFsReadForNonFallback = true - setCacheControlHeaders(headers, request, requestContext, true) + setCacheControlHeaders(headers, request, requestContext) expect(headers.set).toHaveBeenNthCalledWith( 1, diff --git a/src/run/headers.ts b/src/run/headers.ts index 411edf0052..53811b8169 100644 --- a/src/run/headers.ts +++ b/src/run/headers.ts @@ -263,7 +263,7 @@ export const setCacheControlHeaders = ( cacheControl === null && !headers.has('cdn-cache-control') && !headers.has('netlify-cdn-cache-control') && - requestContext.usedFsRead + requestContext.usedFsReadForNonFallback ) { // handle CDN Cache Control on static files headers.set('cache-control', 'public, max-age=0, must-revalidate') @@ -272,7 +272,10 @@ export const setCacheControlHeaders = ( } export const setCacheTagsHeaders = (headers: Headers, requestContext: RequestContext) => { - if (requestContext.responseCacheTags) { + if ( + requestContext.responseCacheTags && + (headers.has('cache-control') || headers.has('netlify-cdn-cache-control')) + ) { headers.set('netlify-cache-tag', requestContext.responseCacheTags.join(',')) } } diff --git a/src/run/next.cts b/src/run/next.cts index b99515e393..d90c34618f 100644 --- a/src/run/next.cts +++ b/src/run/next.cts @@ -1,5 +1,5 @@ -import fs from 'fs/promises' -import { relative, resolve } from 'path' +import fs, { readFile } from 'fs/promises' +import { join, relative, resolve } from 'path' // @ts-expect-error no types installed import { patchFs } from 'fs-monkey' @@ -80,6 +80,27 @@ console.timeEnd('import next server') type FS = typeof import('fs') +export type FSBlobsManifest = { + fallbackPaths: string[] + outputRoot: string +} + +function normalizeStaticAssetPath(path: string) { + return path.startsWith('/') ? path : `/${path}` +} + +let fsBlobsManifestPromise: Promise | undefined +const getFSBlobsManifest = (): Promise => { + if (!fsBlobsManifestPromise) { + fsBlobsManifestPromise = (async () => { + const { FS_BLOBS_MANIFEST, PLUGIN_DIR } = await import('./constants.js') + return JSON.parse(await readFile(resolve(PLUGIN_DIR, FS_BLOBS_MANIFEST), 'utf-8')) + })() + } + + return fsBlobsManifestPromise +} + export async function getMockedRequestHandlers(...args: Parameters) { const tracer = getTracer() return tracer.withActiveSpan('mocked request handler', async () => { @@ -96,13 +117,17 @@ export async function getMockedRequestHandlers(...args: Parameters Date: Wed, 16 Oct 2024 12:35:06 +0200 Subject: [PATCH 4/6] Update src/build/content/prerendered.ts Co-authored-by: Rob Stanford --- src/build/content/prerendered.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/build/content/prerendered.ts b/src/build/content/prerendered.ts index 03854c6104..be58e50b2f 100644 --- a/src/build/content/prerendered.ts +++ b/src/build/content/prerendered.ts @@ -44,7 +44,7 @@ const writeCacheEntry = async ( } /** - * Normalize routes by ensuring leading slashes and ensuring root path is index + * Normalize routes by ensuring leading slashes and ensuring root path is /index */ const routeToFilePath = (path: string) => { if (path === '/') { From e5de50e28c7837f1bbe0d062b81f0a3875d49a81 Mon Sep 17 00:00:00 2001 From: pieh Date: Wed, 16 Oct 2024 13:58:26 +0200 Subject: [PATCH 5/6] chore: don't use extra fallback manifest and instead store html and boolean wether that is fallback html in single blob --- src/build/content/prerendered.ts | 54 +++++--------------------------- src/build/content/static.test.ts | 1 + src/build/content/static.ts | 12 ++++++- src/build/plugin-context.ts | 36 +++++++++++++++++++++ src/run/constants.ts | 1 - src/run/next.cts | 38 +++++++--------------- 6 files changed, 67 insertions(+), 75 deletions(-) diff --git a/src/build/content/prerendered.ts b/src/build/content/prerendered.ts index be58e50b2f..3510aefe12 100644 --- a/src/build/content/prerendered.ts +++ b/src/build/content/prerendered.ts @@ -1,7 +1,6 @@ import { existsSync } from 'node:fs' import { mkdir, readFile, writeFile } from 'node:fs/promises' import { join } from 'node:path' -import { join as posixJoin } from 'node:path/posix' import { trace } from '@opentelemetry/api' import { wrapTracer } from '@opentelemetry/api/experimental' @@ -9,8 +8,6 @@ import { glob } from 'fast-glob' import pLimit from 'p-limit' import { satisfies } from 'semver' -import { FS_BLOBS_MANIFEST } from '../../run/constants.js' -import { type FSBlobsManifest } from '../../run/next.cjs' import { encodeBlobKey } from '../../shared/blobkey.js' import type { CachedFetchValueForMultipleVersions, @@ -160,11 +157,6 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise }) : false - const fsBlobsManifest: FSBlobsManifest = { - fallbackPaths: [], - outputRoot: ctx.distDir, - } - await Promise.all([ ...Object.entries(manifest.routes).map( ([route, meta]): Promise => @@ -214,41 +206,15 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise await writeCacheEntry(key, value, lastModified, ctx) }), ), - ...Object.entries(manifest.dynamicRoutes).map(async ([route, meta]) => { - // fallback can be `string | false | null` - // - `string` - when user use pages router with `fallback: true`, and then it's html file path - // - `null` - when user use pages router with `fallback: 'block'` or app router with `export const dynamicParams = true` - // - `false` - when user use pages router with `fallback: false` or app router with `export const dynamicParams = false` - if (typeof meta.fallback === 'string') { - // https://github.com/vercel/next.js/pull/68603 started using route cache to serve fallbacks - // so we have to seed blobs with fallback entries - - // create cache entry for pages router with `fallback: true` case - await limitConcurrentPrerenderContentHandling(async () => { - // dynamic routes don't have entries for each locale so we have to generate them - // ourselves. If i18n is not used we use empty string as "locale" to be able to use - // same handling wether i18n is used or not - const locales = ctx.buildConfig.i18n?.locales ?? [''] + ...ctx.getFallbacks(manifest).map(async (route) => { + const key = routeToFilePath(route) + const value = await buildPagesCacheValue( + join(ctx.publishDir, 'server/pages', key), + shouldUseEnumKind, + true, // there is no corresponding json file for fallback, so we are skipping it for this entry + ) - const lastModified = Date.now() - for (const locale of locales) { - const key = routeToFilePath(posixJoin(locale, route)) - const value = await buildPagesCacheValue( - join(ctx.publishDir, 'server/pages', key), - shouldUseEnumKind, - true, // there is no corresponding json file for fallback, so we are skipping it for this entry - ) - // Netlify Forms are not support and require a workaround - if (value.kind === 'PAGE' || value.kind === 'PAGES' || value.kind === 'APP_PAGE') { - verifyNetlifyForms(ctx, value.html) - } - - await writeCacheEntry(key, value, lastModified, ctx) - - fsBlobsManifest.fallbackPaths.push(`${key}.html`) - } - }) - } + await writeCacheEntry(key, value, Date.now(), ctx) }), ]) @@ -263,10 +229,6 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise ) await writeCacheEntry(key, value, lastModified, ctx) } - await writeFile( - join(ctx.serverHandlerDir, FS_BLOBS_MANIFEST), - JSON.stringify(fsBlobsManifest), - ) } catch (error) { ctx.failBuild('Failed assembling prerendered content for upload', error) } diff --git a/src/build/content/static.test.ts b/src/build/content/static.test.ts index d26610e11d..55e7e04204 100644 --- a/src/build/content/static.test.ts +++ b/src/build/content/static.test.ts @@ -25,6 +25,7 @@ const createFsFixtureWithBasePath = ( ) => { return createFsFixture( { + [join(ctx.publishDir, 'prerender-manifest.json')]: JSON.stringify({ dynamicRoutes: [] }), ...fixture, [join(ctx.publishDir, 'routes-manifest.json')]: JSON.stringify({ basePath }), [join(ctx.publishDir, 'required-server-files.json')]: JSON.stringify({ diff --git a/src/build/content/static.ts b/src/build/content/static.ts index 4079695bd4..8231ec1fd2 100644 --- a/src/build/content/static.ts +++ b/src/build/content/static.ts @@ -6,6 +6,7 @@ import { trace } from '@opentelemetry/api' import { wrapTracer } from '@opentelemetry/api/experimental' import glob from 'fast-glob' +import type { HtmlBlob } from '../../run/next.cjs' import { encodeBlobKey } from '../../shared/blobkey.js' import { PluginContext } from '../plugin-context.js' import { verifyNetlifyForms } from '../verification.js' @@ -25,6 +26,8 @@ export const copyStaticContent = async (ctx: PluginContext): Promise => { extglob: true, }) + const fallbacks = ctx.getFallbacks(await ctx.getPrerenderManifest()) + try { await mkdir(destDir, { recursive: true }) await Promise.all( @@ -33,7 +36,14 @@ export const copyStaticContent = async (ctx: PluginContext): Promise => { .map(async (path): Promise => { const html = await readFile(join(srcDir, path), 'utf-8') verifyNetlifyForms(ctx, html) - await writeFile(join(destDir, await encodeBlobKey(path)), html, 'utf-8') + + const isFallback = fallbacks.includes(path.slice(0, -5)) + + await writeFile( + join(destDir, await encodeBlobKey(path)), + JSON.stringify({ html, isFallback } satisfies HtmlBlob), + 'utf-8', + ) }), ) } catch (error) { diff --git a/src/build/plugin-context.ts b/src/build/plugin-context.ts index e678cd9364..74c0ffc329 100644 --- a/src/build/plugin-context.ts +++ b/src/build/plugin-context.ts @@ -334,6 +334,42 @@ export class PluginContext { return this.#nextVersion } + #fallbacks: string[] | null = null + /** + * Get an array of localized fallback routes + * + * Example return value for non-i18n site: `['blog/[slug]']` + * + * Example return value for i18n site: `['en/blog/[slug]', 'fr/blog/[slug]']` + */ + getFallbacks(prerenderManifest: PrerenderManifest): string[] { + if (!this.#fallbacks) { + // dynamic routes don't have entries for each locale so we have to generate them + // ourselves. If i18n is not used we use empty string as "locale" to be able to use + // same handling wether i18n is used or not + const locales = this.buildConfig.i18n?.locales ?? [''] + + this.#fallbacks = Object.entries(prerenderManifest.dynamicRoutes).reduce( + (fallbacks, [route, meta]) => { + // fallback can be `string | false | null` + // - `string` - when user use pages router with `fallback: true`, and then it's html file path + // - `null` - when user use pages router with `fallback: 'block'` or app router with `export const dynamicParams = true` + // - `false` - when user use pages router with `fallback: false` or app router with `export const dynamicParams = false` + if (typeof meta.fallback === 'string') { + for (const locale of locales) { + const localizedRoute = posixJoin(locale, route.replace(/^\/+/g, '')) + fallbacks.push(localizedRoute) + } + } + return fallbacks + }, + [] as string[], + ) + } + + return this.#fallbacks + } + /** Fails a build with a message and an optional error */ failBuild(message: string, error?: unknown): never { return this.utils.build.failBuild(message, error instanceof Error ? { error } : undefined) diff --git a/src/run/constants.ts b/src/run/constants.ts index c27f338b3e..ebf5daaa32 100644 --- a/src/run/constants.ts +++ b/src/run/constants.ts @@ -5,4 +5,3 @@ export const MODULE_DIR = fileURLToPath(new URL('.', import.meta.url)) export const PLUGIN_DIR = resolve(`${MODULE_DIR}../../..`) // a file where we store the required-server-files config object in to access during runtime export const RUN_CONFIG = 'run-config.json' -export const FS_BLOBS_MANIFEST = 'fs-blobs-manifest.json' diff --git a/src/run/next.cts b/src/run/next.cts index d90c34618f..84f378ddb6 100644 --- a/src/run/next.cts +++ b/src/run/next.cts @@ -1,5 +1,5 @@ -import fs, { readFile } from 'fs/promises' -import { join, relative, resolve } from 'path' +import fs from 'fs/promises' +import { relative, resolve } from 'path' // @ts-expect-error no types installed import { patchFs } from 'fs-monkey' @@ -80,25 +80,9 @@ console.timeEnd('import next server') type FS = typeof import('fs') -export type FSBlobsManifest = { - fallbackPaths: string[] - outputRoot: string -} - -function normalizeStaticAssetPath(path: string) { - return path.startsWith('/') ? path : `/${path}` -} - -let fsBlobsManifestPromise: Promise | undefined -const getFSBlobsManifest = (): Promise => { - if (!fsBlobsManifestPromise) { - fsBlobsManifestPromise = (async () => { - const { FS_BLOBS_MANIFEST, PLUGIN_DIR } = await import('./constants.js') - return JSON.parse(await readFile(resolve(PLUGIN_DIR, FS_BLOBS_MANIFEST), 'utf-8')) - })() - } - - return fsBlobsManifestPromise +export type HtmlBlob = { + html: string + isFallback: boolean } export async function getMockedRequestHandlers(...args: Parameters) { @@ -117,20 +101,20 @@ export async function getMockedRequestHandlers(...args: Parameters Date: Wed, 16 Oct 2024 16:24:58 +0200 Subject: [PATCH 6/6] test: add some unit tests about static html blobs and fallbacks --- src/build/content/static.test.ts | 250 +++++++++++++++++++++++++------ 1 file changed, 206 insertions(+), 44 deletions(-) diff --git a/src/build/content/static.test.ts b/src/build/content/static.test.ts index 55e7e04204..ed1927b298 100644 --- a/src/build/content/static.test.ts +++ b/src/build/content/static.test.ts @@ -1,12 +1,13 @@ -import { Buffer } from 'node:buffer' +import { readFile } from 'node:fs/promises' import { join } from 'node:path' import { inspect } from 'node:util' import type { NetlifyPluginOptions } from '@netlify/build' import glob from 'fast-glob' +import type { PrerenderManifest } from 'next/dist/build/index.js' import { beforeEach, describe, expect, Mock, test, vi } from 'vitest' -import { mockFileSystem } from '../../../tests/index.js' +import { decodeBlobKey, encodeBlobKey, mockFileSystem } from '../../../tests/index.js' import { type FixtureTestContext } from '../../../tests/utils/contexts.js' import { createFsFixture } from '../../../tests/utils/fixture.js' import { PluginContext, RequiredServerFilesManifest } from '../plugin-context.js' @@ -21,11 +22,22 @@ type Context = FixtureTestContext & { const createFsFixtureWithBasePath = ( fixture: Record, ctx: Omit, - basePath = '', + + { + basePath = '', + // eslint-disable-next-line unicorn/no-useless-undefined + i18n = undefined, + dynamicRoutes = {}, + }: { + basePath?: string + i18n?: Pick, 'locales'> + dynamicRoutes?: { + [route: string]: Pick + } + } = {}, ) => { return createFsFixture( { - [join(ctx.publishDir, 'prerender-manifest.json')]: JSON.stringify({ dynamicRoutes: [] }), ...fixture, [join(ctx.publishDir, 'routes-manifest.json')]: JSON.stringify({ basePath }), [join(ctx.publishDir, 'required-server-files.json')]: JSON.stringify({ @@ -33,8 +45,10 @@ const createFsFixtureWithBasePath = ( appDir: ctx.relativeAppDir, config: { distDir: ctx.publishDir, + i18n, }, } as Pick), + [join(ctx.publishDir, 'prerender-manifest.json')]: JSON.stringify({ dynamicRoutes }), }, ctx, ) @@ -122,7 +136,7 @@ describe('Regular Repository layout', () => { '.next/static/sub-dir/test2.js': '', }, ctx, - '/base/path', + { basePath: '/base/path' }, ) await copyStaticAssets(pluginContext) @@ -169,7 +183,7 @@ describe('Regular Repository layout', () => { 'public/another-asset.json': '', }, ctx, - '/base/path', + { basePath: '/base/path' }, ) await copyStaticAssets(pluginContext) @@ -183,26 +197,100 @@ describe('Regular Repository layout', () => { ) }) - test('should copy the static pages to the publish directory if there are no corresponding JSON files', async ({ - pluginContext, - ...ctx - }) => { - await createFsFixtureWithBasePath( - { - '.next/server/pages/test.html': '', - '.next/server/pages/test2.html': '', - '.next/server/pages/test3.json': '', - }, - ctx, - ) + describe('should copy the static pages to the publish directory if there are no corresponding JSON files and mark wether html file is a fallback', () => { + test('no i18n', async ({ pluginContext, ...ctx }) => { + await createFsFixtureWithBasePath( + { + '.next/server/pages/test.html': '', + '.next/server/pages/test2.html': '', + '.next/server/pages/test3.json': '', + '.next/server/pages/blog/[slug].html': '', + }, + ctx, + { + dynamicRoutes: { + '/blog/[slug]': { + fallback: '/blog/[slug].html', + }, + }, + }, + ) - await copyStaticContent(pluginContext) - const files = await glob('**/*', { cwd: pluginContext.blobDir, dot: true }) + await copyStaticContent(pluginContext) + const files = await glob('**/*', { cwd: pluginContext.blobDir, dot: true }) + + const expectedStaticPages = ['blog/[slug].html', 'test.html', 'test2.html'] + const expectedFallbacks = new Set(['blog/[slug].html']) + + expect(files.map((path) => decodeBlobKey(path)).sort()).toEqual(expectedStaticPages) + + for (const page of expectedStaticPages) { + const expectedIsFallback = expectedFallbacks.has(page) + + const blob = JSON.parse( + await readFile(join(pluginContext.blobDir, await encodeBlobKey(page)), 'utf-8'), + ) - expect(files.map((path) => Buffer.from(path, 'base64').toString('utf-8')).sort()).toEqual([ - 'test.html', - 'test2.html', - ]) + expect(blob, `${page} should ${expectedIsFallback ? '' : 'not '}be a fallback`).toEqual({ + html: '', + isFallback: expectedIsFallback, + }) + } + }) + + test('with i18n', async ({ pluginContext, ...ctx }) => { + await createFsFixtureWithBasePath( + { + '.next/server/pages/de/test.html': '', + '.next/server/pages/de/test2.html': '', + '.next/server/pages/de/test3.json': '', + '.next/server/pages/de/blog/[slug].html': '', + '.next/server/pages/en/test.html': '', + '.next/server/pages/en/test2.html': '', + '.next/server/pages/en/test3.json': '', + '.next/server/pages/en/blog/[slug].html': '', + }, + ctx, + { + dynamicRoutes: { + '/blog/[slug]': { + fallback: '/blog/[slug].html', + }, + }, + i18n: { + locales: ['en', 'de'], + }, + }, + ) + + await copyStaticContent(pluginContext) + const files = await glob('**/*', { cwd: pluginContext.blobDir, dot: true }) + + const expectedStaticPages = [ + 'de/blog/[slug].html', + 'de/test.html', + 'de/test2.html', + 'en/blog/[slug].html', + 'en/test.html', + 'en/test2.html', + ] + const expectedFallbacks = new Set(['en/blog/[slug].html', 'de/blog/[slug].html']) + + expect(files.map((path) => decodeBlobKey(path)).sort()).toEqual(expectedStaticPages) + + for (const page of expectedStaticPages) { + const expectedIsFallback = expectedFallbacks.has(page) + + const blob = JSON.parse( + await readFile(join(pluginContext.blobDir, await encodeBlobKey(page)), 'utf-8'), + ) + + expect(blob, `${page} should ${expectedIsFallback ? '' : 'not '}be a fallback`).toEqual({ + html: '', + isFallback: expectedIsFallback, + }) + } + }) }) test('should not copy the static pages to the publish directory if there are corresponding JSON files', async ({ @@ -270,7 +358,7 @@ describe('Mono Repository', () => { 'apps/app-1/.next/static/sub-dir/test2.js': '', }, ctx, - '/base/path', + { basePath: '/base/path' }, ) await copyStaticAssets(pluginContext) @@ -317,7 +405,7 @@ describe('Mono Repository', () => { 'apps/app-1/public/another-asset.json': '', }, ctx, - '/base/path', + { basePath: '/base/path' }, ) await copyStaticAssets(pluginContext) @@ -331,26 +419,100 @@ describe('Mono Repository', () => { ) }) - test('should copy the static pages to the publish directory if there are no corresponding JSON files', async ({ - pluginContext, - ...ctx - }) => { - await createFsFixtureWithBasePath( - { - 'apps/app-1/.next/server/pages/test.html': '', - 'apps/app-1/.next/server/pages/test2.html': '', - 'apps/app-1/.next/server/pages/test3.json': '', - }, - ctx, - ) + describe('should copy the static pages to the publish directory if there are no corresponding JSON files and mark wether html file is a fallback', () => { + test('no i18n', async ({ pluginContext, ...ctx }) => { + await createFsFixtureWithBasePath( + { + 'apps/app-1/.next/server/pages/test.html': '', + 'apps/app-1/.next/server/pages/test2.html': '', + 'apps/app-1/.next/server/pages/test3.json': '', + 'apps/app-1/.next/server/pages/blog/[slug].html': '', + }, + ctx, + { + dynamicRoutes: { + '/blog/[slug]': { + fallback: '/blog/[slug].html', + }, + }, + }, + ) - await copyStaticContent(pluginContext) - const files = await glob('**/*', { cwd: pluginContext.blobDir, dot: true }) + await copyStaticContent(pluginContext) + const files = await glob('**/*', { cwd: pluginContext.blobDir, dot: true }) + + const expectedStaticPages = ['blog/[slug].html', 'test.html', 'test2.html'] + const expectedFallbacks = new Set(['blog/[slug].html']) + + expect(files.map((path) => decodeBlobKey(path)).sort()).toEqual(expectedStaticPages) + + for (const page of expectedStaticPages) { + const expectedIsFallback = expectedFallbacks.has(page) + + const blob = JSON.parse( + await readFile(join(pluginContext.blobDir, await encodeBlobKey(page)), 'utf-8'), + ) - expect(files.map((path) => Buffer.from(path, 'base64').toString('utf-8')).sort()).toEqual([ - 'test.html', - 'test2.html', - ]) + expect(blob, `${page} should ${expectedIsFallback ? '' : 'not '}be a fallback`).toEqual({ + html: '', + isFallback: expectedIsFallback, + }) + } + }) + + test('with i18n', async ({ pluginContext, ...ctx }) => { + await createFsFixtureWithBasePath( + { + 'apps/app-1/.next/server/pages/de/test.html': '', + 'apps/app-1/.next/server/pages/de/test2.html': '', + 'apps/app-1/.next/server/pages/de/test3.json': '', + 'apps/app-1/.next/server/pages/de/blog/[slug].html': '', + 'apps/app-1/.next/server/pages/en/test.html': '', + 'apps/app-1/.next/server/pages/en/test2.html': '', + 'apps/app-1/.next/server/pages/en/test3.json': '', + 'apps/app-1/.next/server/pages/en/blog/[slug].html': '', + }, + ctx, + { + dynamicRoutes: { + '/blog/[slug]': { + fallback: '/blog/[slug].html', + }, + }, + i18n: { + locales: ['en', 'de'], + }, + }, + ) + + await copyStaticContent(pluginContext) + const files = await glob('**/*', { cwd: pluginContext.blobDir, dot: true }) + + const expectedStaticPages = [ + 'de/blog/[slug].html', + 'de/test.html', + 'de/test2.html', + 'en/blog/[slug].html', + 'en/test.html', + 'en/test2.html', + ] + const expectedFallbacks = new Set(['en/blog/[slug].html', 'de/blog/[slug].html']) + + expect(files.map((path) => decodeBlobKey(path)).sort()).toEqual(expectedStaticPages) + + for (const page of expectedStaticPages) { + const expectedIsFallback = expectedFallbacks.has(page) + + const blob = JSON.parse( + await readFile(join(pluginContext.blobDir, await encodeBlobKey(page)), 'utf-8'), + ) + + expect(blob, `${page} should ${expectedIsFallback ? '' : 'not '}be a fallback`).toEqual({ + html: '', + isFallback: expectedIsFallback, + }) + } + }) }) test('should not copy the static pages to the publish directory if there are corresponding JSON files', async ({