diff --git a/cypress/integration/default/dynamic-routes.spec.ts b/cypress/integration/default/dynamic-routes.spec.ts index 31c2177695..266f6faa43 100644 --- a/cypress/integration/default/dynamic-routes.spec.ts +++ b/cypress/integration/default/dynamic-routes.spec.ts @@ -49,7 +49,7 @@ describe('Dynamic Routing', () => { cy.request({ url: '/getStaticProps/3/', headers: { 'x-nf-debug-logging': '1' }, failOnStatusCode: false }).then( (res) => { expect(res.status).to.eq(404) - expect(res.headers).to.have.property('x-nf-render-mode', 'odb') + expect(res.headers).to.not.have.property('x-nf-render-mode') expect(res.body).to.contain('Custom 404') }, ) @@ -102,7 +102,7 @@ describe('Dynamic Routing', () => { failOnStatusCode: false, }).then((res) => { expect(res.status).to.eq(404) - expect(res.headers).to.have.property('x-nf-render-mode', 'odb') + expect(res.headers).to.not.have.property('x-nf-render-mode') expect(res.body).to.contain('Custom 404') }) }) diff --git a/packages/runtime/src/helpers/redirects.ts b/packages/runtime/src/helpers/redirects.ts index 764117596e..22d777c88b 100644 --- a/packages/runtime/src/helpers/redirects.ts +++ b/packages/runtime/src/helpers/redirects.ts @@ -19,6 +19,7 @@ import { isApiRoute, redirectsForNextRoute, redirectsForNextRouteWithData, + redirectsForNext404Route, routeToDataRoute, } from './utils' @@ -187,6 +188,7 @@ const generateDynamicRewrites = ({ basePath, buildId, i18n, + is404Isr, }: { dynamicRoutes: RoutesManifest['dynamicRoutes'] prerenderedDynamicRoutes: PrerenderManifest['dynamicRoutes'] @@ -194,6 +196,7 @@ const generateDynamicRewrites = ({ i18n: NextConfig['i18n'] buildId: string middleware: Array + is404Isr: boolean }): { dynamicRoutesThatMatchMiddleware: Array dynamicRewrites: NetlifyConfig['redirects'] @@ -207,9 +210,11 @@ const generateDynamicRewrites = ({ if (route.page in prerenderedDynamicRoutes) { if (matchesMiddleware(middleware, route.page)) { dynamicRoutesThatMatchMiddleware.push(route.page) + } else if (prerenderedDynamicRoutes[route.page].fallback === false && !is404Isr) { + dynamicRewrites.push(...redirectsForNext404Route({ route: route.page, buildId, basePath, i18n })) } else { dynamicRewrites.push( - ...redirectsForNextRoute({ buildId, route: route.page, basePath, to: ODB_FUNCTION_PATH, status: 200, i18n }), + ...redirectsForNextRoute({ route: route.page, buildId, basePath, to: ODB_FUNCTION_PATH, i18n }), ) } } else { @@ -263,6 +268,10 @@ export const generateRedirects = async ({ const staticRouteEntries = Object.entries(prerenderedStaticRoutes) + const is404Isr = staticRouteEntries.some( + ([route, { initialRevalidateSeconds }]) => is404Route(route, i18n) && initialRevalidateSeconds !== false, + ) + const routesThatMatchMiddleware: Array = [] const { staticRoutePaths, staticIsrRewrites, staticIsrRoutesThatMatchMiddleware } = generateStaticIsrRewrites({ @@ -295,6 +304,7 @@ export const generateRedirects = async ({ basePath, buildId, i18n, + is404Isr, }) netlifyConfig.redirects.push(...dynamicRewrites) routesThatMatchMiddleware.push(...dynamicRoutesThatMatchMiddleware) diff --git a/packages/runtime/src/helpers/utils.ts b/packages/runtime/src/helpers/utils.ts index 527409c8fb..35e5487ba4 100644 --- a/packages/runtime/src/helpers/utils.ts +++ b/packages/runtime/src/helpers/utils.ts @@ -72,9 +72,18 @@ export const netlifyRoutesForNextRouteWithData = ({ route, dataRoute }: { route: export const routeToDataRoute = (route: string, buildId: string, locale?: string) => `/_next/data/${buildId}${locale ? `/${locale}` : ''}${route === '/' ? '/index' : route}.json` -const netlifyRoutesForNextRoute = (route: string, buildId: string, i18n?: I18n): Array => { +const netlifyRoutesForNextRoute = ( + route: string, + buildId: string, + i18n?: I18n, +): Array<{ redirect: string; locale: string | false }> => { if (!i18n?.locales?.length) { - return netlifyRoutesForNextRouteWithData({ route, dataRoute: routeToDataRoute(route, buildId) }) + return netlifyRoutesForNextRouteWithData({ route, dataRoute: routeToDataRoute(route, buildId) }).map( + (redirect) => ({ + redirect, + locale: false, + }), + ) } const { locales, defaultLocale } = i18n const routes = [] @@ -87,7 +96,10 @@ const netlifyRoutesForNextRoute = (route: string, buildId: string, i18n?: I18n): ...netlifyRoutesForNextRouteWithData({ route: locale === defaultLocale ? route : `/${locale}${route}`, dataRoute, - }), + }).map((redirect) => ({ + redirect, + locale, + })), ) }) return routes @@ -115,13 +127,33 @@ export const redirectsForNextRoute = ({ status?: number force?: boolean }): NetlifyConfig['redirects'] => - netlifyRoutesForNextRoute(route, buildId, i18n).map((redirect) => ({ + netlifyRoutesForNextRoute(route, buildId, i18n).map(({ redirect }) => ({ from: `${basePath}${redirect}`, to, status, force, })) +export const redirectsForNext404Route = ({ + route, + buildId, + basePath, + i18n, + force = false, +}: { + route: string + buildId: string + basePath: string + i18n: I18n + force?: boolean +}): NetlifyConfig['redirects'] => + netlifyRoutesForNextRoute(route, buildId, i18n).map(({ redirect, locale }) => ({ + from: `${basePath}${redirect}`, + to: locale ? `${basePath}/server/pages/${locale}/404.html` : `${basePath}/server/pages/404.html`, + status: 404, + force, + })) + export const redirectsForNextRouteWithData = ({ route, dataRoute, diff --git a/test/helpers/utils.spec.ts b/test/helpers/utils.spec.ts index ae56d3d933..e02bd3fb42 100644 --- a/test/helpers/utils.spec.ts +++ b/test/helpers/utils.spec.ts @@ -1,17 +1,24 @@ import Chance from 'chance' import { ExperimentalConfig } from 'next/dist/server/config-shared' -import { getCustomImageResponseHeaders, getRemotePatterns, ImagesConfig } from '../../packages/runtime/src/helpers/utils' +import { + getCustomImageResponseHeaders, + getRemotePatterns, + ImagesConfig, + redirectsForNext404Route, +} from '../../packages/runtime/src/helpers/utils' const chance = new Chance() describe('getCustomImageResponseHeaders', () => { it('returns null when no custom image response headers are found', () => { - const mockHeaders = [{ - for: '/test', - values: { - 'X-Foo': chance.string() - } - }] + const mockHeaders = [ + { + for: '/test', + values: { + 'X-Foo': chance.string(), + }, + }, + ] expect(getCustomImageResponseHeaders(mockHeaders)).toBe(null) }) @@ -19,12 +26,14 @@ describe('getCustomImageResponseHeaders', () => { it('returns header values when custom image response headers are found', () => { const mockFooValue = chance.string() - const mockHeaders = [{ - for: '/_next/image/', - values: { - 'X-Foo': mockFooValue - } - }] + const mockHeaders = [ + { + for: '/_next/image/', + values: { + 'X-Foo': mockFooValue, + }, + }, + ] const result = getCustomImageResponseHeaders(mockHeaders) expect(result).toStrictEqual({ @@ -38,31 +47,23 @@ describe('getRemotePatterns', () => { let mockImages beforeEach(() => { mockExperimentalConfig = { - images: {} + images: {}, } as ExperimentalConfig - + mockImages = { - deviceSizes: [ - 640, 750, 828, - 1080, 1200, 1920, - 2048, 3840 - ], - imageSizes: [ - 16, 32, 48, 64, - 96, 128, 256, 384 - ], + deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840], + imageSizes: [16, 32, 48, 64, 96, 128, 256, 384], path: '/_next/image', loader: 'default', domains: [], disableStaticImages: false, minimumCacheTTL: 60, - formats: [ 'image/avif', 'image/webp' ], + formats: ['image/avif', 'image/webp'], dangerouslyAllowSVG: false, contentSecurityPolicy: "script-src 'none'; frame-src 'none'; sandbox;", unoptimized: false, - remotePatterns: [] + remotePatterns: [], } as ImagesConfig - }) it('returns the remote patterns found under experimental.images', () => { @@ -73,7 +74,7 @@ describe('getRemotePatterns', () => { }, ] const result = getRemotePatterns(mockExperimentalConfig, mockImages) - + expect(result).toStrictEqual(mockExperimentalConfig.images?.remotePatterns) }) @@ -85,12 +86,49 @@ describe('getRemotePatterns', () => { }, ] const result = getRemotePatterns(mockExperimentalConfig, mockImages) - - expect(result).toStrictEqual(mockImages.remotePatterns) + + expect(result).toStrictEqual(mockImages.remotePatterns) }) - + it('returns an empty array', () => { const result = getRemotePatterns(mockExperimentalConfig, mockImages) expect(result).toStrictEqual([]) }) }) + +describe('redirectsForNext404Route', () => { + it('returns static 404 redirects', () => { + const mockRoute = { + route: '/test', + buildId: 'test', + basePath: '', + i18n: null, + } + + expect(redirectsForNext404Route(mockRoute)).toStrictEqual([ + { force: false, from: '/_next/data/test/test.json', status: 404, to: '/server/pages/404.html' }, + { force: false, from: '/test', status: 404, to: '/server/pages/404.html' }, + ]) + }) + + it('returns localised static 404 redirects when i18n locales are provided', () => { + const mockRoute = { + route: '/test', + buildId: 'test', + basePath: '', + i18n: { + defaultLocale: 'en', + locales: ['en', 'es', 'fr'], + }, + } + + expect(redirectsForNext404Route(mockRoute)).toStrictEqual([ + { force: false, from: '/_next/data/test/en/test.json', status: 404, to: '/server/pages/en/404.html' }, + { force: false, from: '/test', status: 404, to: '/server/pages/en/404.html' }, + { force: false, from: '/_next/data/test/es/test.json', status: 404, to: '/server/pages/es/404.html' }, + { force: false, from: '/es/test', status: 404, to: '/server/pages/es/404.html' }, + { force: false, from: '/_next/data/test/fr/test.json', status: 404, to: '/server/pages/fr/404.html' }, + { force: false, from: '/fr/test', status: 404, to: '/server/pages/fr/404.html' }, + ]) + }) +})