From c6ff51fdaea1292fd4114f38d3d20995c572316b Mon Sep 17 00:00:00 2001 From: Matt Kane <m@mk.gg> Date: Thu, 27 Jan 2022 11:38:57 +0000 Subject: [PATCH 1/4] fix: don't use ODB for routes that match middleware --- demos/default/local-plugin/package-lock.json | 11 +- .../withFallbackAndMiddleware/[...slug].js | 57 +++++ .../withFallbackAndMiddleware/[id].js | 55 +++++ .../withFallbackAndMiddleware/_middleware.js | 7 + demos/default/pages/index.js | 18 +- src/helpers/files.ts | 19 +- src/helpers/redirects.ts | 68 +++++- src/helpers/utils.ts | 24 +-- test/__snapshots__/index.js.snap | 204 ++++++++++++++++++ 9 files changed, 416 insertions(+), 47 deletions(-) create mode 100644 demos/default/pages/getStaticProps/withFallbackAndMiddleware/[...slug].js create mode 100644 demos/default/pages/getStaticProps/withFallbackAndMiddleware/[id].js create mode 100644 demos/default/pages/getStaticProps/withFallbackAndMiddleware/_middleware.js diff --git a/demos/default/local-plugin/package-lock.json b/demos/default/local-plugin/package-lock.json index 9bd7b537cc..11a2e6611e 100644 --- a/demos/default/local-plugin/package-lock.json +++ b/demos/default/local-plugin/package-lock.json @@ -1,14 +1,5 @@ { "name": "local-plugin", "version": "1.0.0", - "lockfileVersion": 2, - "requires": true, - "packages": { - "": { - "name": "local-plugin", - "version": "1.0.0", - "hasInstallScript": true, - "license": "ISC" - } - } + "lockfileVersion": 1 } diff --git a/demos/default/pages/getStaticProps/withFallbackAndMiddleware/[...slug].js b/demos/default/pages/getStaticProps/withFallbackAndMiddleware/[...slug].js new file mode 100644 index 0000000000..5bcb1abc6b --- /dev/null +++ b/demos/default/pages/getStaticProps/withFallbackAndMiddleware/[...slug].js @@ -0,0 +1,57 @@ +import { useRouter } from 'next/router' +import Link from 'next/link' + +const Show = ({ show }) => { + const router = useRouter() + + // This is never shown on Netlify. We just need it for NextJS to be happy, + // because NextJS will render a fallback HTML page. + if (router.isFallback) { + return <div>Loading...</div> + } + + return ( + <div> + <p> + Check the network panel for the header <code>x-middleware-date</code> to ensure that it is running + </p> + + <hr /> + + <h1>Show #{show.id}</h1> + <p>{show.name}</p> + + <hr /> + + <Link href="/"> + <a>Go back home</a> + </Link> + </div> + ) +} + +export async function getStaticPaths() { + // Set the paths we want to pre-render + const paths = [{ params: { slug: ['my', 'path', '1'] } }, { params: { slug: ['my', 'path', '2'] } }] + + // We'll pre-render these paths at build time. + // { fallback: true } means other routes will be rendered at runtime. + return { paths, fallback: true } +} + +export async function getStaticProps({ params }) { + // The ID to render + const { slug } = params + const id = slug[slug.length - 1] + + const res = await fetch(`https://api.tvmaze.com/shows/${id}`) + const data = await res.json() + + return { + props: { + show: data, + }, + } +} + +export default Show diff --git a/demos/default/pages/getStaticProps/withFallbackAndMiddleware/[id].js b/demos/default/pages/getStaticProps/withFallbackAndMiddleware/[id].js new file mode 100644 index 0000000000..a1e1747b77 --- /dev/null +++ b/demos/default/pages/getStaticProps/withFallbackAndMiddleware/[id].js @@ -0,0 +1,55 @@ +import { useRouter } from 'next/router' +import Link from 'next/link' + +const Show = ({ show }) => { + const router = useRouter() + + // This is never shown on Netlify. We just need it for NextJS to be happy, + // because NextJS will render a fallback HTML page. + if (router.isFallback) { + return <div>Loading...</div> + } + + return ( + <div> + <p> + Check the network panel for the header <code>x-middleware-date</code> to ensure that it is running + </p> + <hr /> + + <h1>Show #{show.id}</h1> + <p>{show.name}</p> + + <hr /> + + <Link href="/"> + <a>Go back home</a> + </Link> + </div> + ) +} + +export async function getStaticPaths() { + // Set the paths we want to pre-render + const paths = [{ params: { id: '3' } }, { params: { id: '4' } }] + + // We'll pre-render these paths at build time. + // { fallback: true } means other routes will be rendered at runtime. + return { paths, fallback: true } +} + +export async function getStaticProps({ params }) { + // The ID to render + const { id } = params + + const res = await fetch(`https://api.tvmaze.com/shows/${id}`) + const data = await res.json() + + return { + props: { + show: data, + }, + } +} + +export default Show diff --git a/demos/default/pages/getStaticProps/withFallbackAndMiddleware/_middleware.js b/demos/default/pages/getStaticProps/withFallbackAndMiddleware/_middleware.js new file mode 100644 index 0000000000..1142194f07 --- /dev/null +++ b/demos/default/pages/getStaticProps/withFallbackAndMiddleware/_middleware.js @@ -0,0 +1,7 @@ +import { NextResponse } from 'next/server' + +export function middleware() { + const res = NextResponse.next() + res.headers.set('x-middleware-date', new Date().toISOString()) + return res +} diff --git a/demos/default/pages/index.js b/demos/default/pages/index.js index 2003c8f07e..a130accf60 100644 --- a/demos/default/pages/index.js +++ b/demos/default/pages/index.js @@ -8,7 +8,11 @@ const Index = ({ shows, nodeEnv }) => { return ( <div> - <img src="/next-on-netlify.png" alt="NextJS on Netlify Banner" className='self-center w-full max-h-80 max-w-5xl m-auto' /> + <img + src="/next-on-netlify.png" + alt="NextJS on Netlify Banner" + className="self-center w-full max-h-80 max-w-5xl m-auto" + /> <div> <Header /> @@ -18,7 +22,8 @@ const Index = ({ shows, nodeEnv }) => { <h2>Server-Side Rendering</h2> <p> - This page is server-side rendered. It fetches a random list of five TV shows from the TVmaze REST API. Refresh this page to see it change. + This page is server-side rendered. It fetches a random list of five TV shows from the TVmaze REST API. Refresh + this page to see it change. </p> <code>NODE_ENV: {nodeEnv}</code> @@ -86,8 +91,8 @@ const Index = ({ shows, nodeEnv }) => { <h2>Localization</h2> <p> - Localization (i18n) is supported! This demo uses <code>fr</code> with <code>en</code> as the default locale (at{' '} - <code>/</code>). + Localization (i18n) is supported! This demo uses <code>fr</code> with <code>en</code> as the default locale + (at <code>/</code>). </p> <strong>The current locale is {locale}</strong> <p>Click on the links below to see the above text change</p> @@ -175,6 +180,11 @@ const Index = ({ shows, nodeEnv }) => { <a>Middleware</a> </Link> </li> + <li> + <Link href="/getStaticProps/withFallbackAndMiddleware/4"> + <a>Middleware matching a pre-rendered dynamic route</a> + </Link> + </li> </ul> <h4>Preview mode</h4> <p>Preview mode: </p> diff --git a/src/helpers/files.ts b/src/helpers/files.ts index e7876aca7e..8b637ba656 100644 --- a/src/helpers/files.ts +++ b/src/helpers/files.ts @@ -58,6 +58,15 @@ export const matchesRewrite = (file: string, rewrites: Rewrites): boolean => { return matchesRedirect(file, rewrites.beforeFiles) } +export const getMiddleware = async (publish: string): Promise<Array<string>> => { + const manifestPath = join(publish, 'server', 'middleware-manifest.json') + if (existsSync(manifestPath)) { + const manifest = await readJson(manifestPath) + return manifest?.sortedMiddleware ?? [] + } + return [] +} + // eslint-disable-next-line max-lines-per-function export const moveStaticPages = async ({ netlifyConfig, @@ -75,14 +84,8 @@ export const moveStaticPages = async ({ const dataDir = join('_next', 'data', buildId) await ensureDir(dataDir) // Load the middleware manifest so we can check if a file matches it before moving - let middleware - const manifestPath = join(outputDir, 'middleware-manifest.json') - if (existsSync(manifestPath)) { - const manifest = await readJson(manifestPath) - if (manifest?.middleware) { - middleware = Object.keys(manifest.middleware).map((path) => path.slice(1)) - } - } + const middlewarePaths = await getMiddleware(netlifyConfig.build.publish) + const middleware = middlewarePaths.map((path) => path.slice(1)) const prerenderManifest: PrerenderManifest = await readJson( join(netlifyConfig.build.publish, 'prerender-manifest.json'), diff --git a/src/helpers/redirects.ts b/src/helpers/redirects.ts index 2cf6e7e5b9..3456f3bdf4 100644 --- a/src/helpers/redirects.ts +++ b/src/helpers/redirects.ts @@ -1,11 +1,15 @@ +/* eslint-disable max-lines */ import { NetlifyConfig } from '@netlify/build' +import { yellowBright } from 'chalk' import { readJSON } from 'fs-extra' import { NextConfig } from 'next' import { PrerenderManifest } from 'next/dist/build' +import { outdent } from 'outdent' import { join } from 'pathe' import { HANDLER_FUNCTION_PATH, HIDDEN_PATHS, ODB_FUNCTION_PATH } from '../constants' +import { getMiddleware } from './files' import { RoutesManifest } from './types' import { getApiRewrites, @@ -14,9 +18,11 @@ import { redirectsForNextRoute, redirectsForNextRouteWithData, routeToDataRoute, - targetForFallback, } from './utils' +const matchesMiddleware = (middleware: Array<string>, route: string): boolean => + middleware?.some((middlewarePath) => route.startsWith(middlewarePath)) + const generateLocaleRedirects = ({ i18n, basePath, @@ -65,6 +71,7 @@ export const generateStaticRedirects = ({ } } +// eslint-disable-next-line max-lines-per-function export const generateRedirects = async ({ netlifyConfig, nextConfig: { i18n, basePath, trailingSlash, appDir }, @@ -102,6 +109,32 @@ export const generateRedirects = async ({ ...(await getPreviewRewrites({ basePath, appDir })), ) + const middleware = await getMiddleware(netlifyConfig.build.publish) + const routesThatMatchMiddleware = new Set<string>() + + const handlerRewrite = (from: string) => ({ + from: `${basePath}${from}`, + to: HANDLER_FUNCTION_PATH, + status: 200, + }) + + // Routes that match middleware need to always use the SSR function + // This generates a rewrite for every middleware in every locale, both with and without a splat + netlifyConfig.redirects.push( + ...middleware + .map((route) => [ + handlerRewrite(`${route}`), + handlerRewrite(`${route}/*`), + handlerRewrite(routeToDataRoute(`${route}/*`, buildId)), + ...(i18n?.locales?.map((locale) => [ + handlerRewrite(`/${locale}${route}`), + handlerRewrite(`/${locale}${route}/*`), + handlerRewrite(routeToDataRoute(`${route}/*`, buildId, locale)), + ]) ?? []), + ]) + .flat(2), + ) + const staticRouteEntries = Object.entries(prerenderedStaticRoutes) const staticRoutePaths = new Set<string>() @@ -121,7 +154,9 @@ export const generateRedirects = async ({ if (i18n?.defaultLocale && route.startsWith(`/${i18n.defaultLocale}/`)) { route = route.slice(i18n.defaultLocale.length + 1) staticRoutePaths.add(route) - + if (matchesMiddleware(middleware, route)) { + routesThatMatchMiddleware.add(route) + } netlifyConfig.redirects.push( ...redirectsForNextRouteWithData({ route, @@ -132,6 +167,9 @@ export const generateRedirects = async ({ }), ) } else { + if (matchesMiddleware(middleware, route)) { + routesThatMatchMiddleware.add(route) + } // ISR routes use the ODB handler netlifyConfig.redirects.push( // No i18n, because the route is already localized @@ -155,11 +193,12 @@ export const generateRedirects = async ({ return } if (route.page in prerenderedDynamicRoutes) { - const { fallback } = prerenderedDynamicRoutes[route.page] - - const { to, status } = targetForFallback(fallback) - - netlifyConfig.redirects.push(...redirectsForNextRoute({ buildId, route: route.page, basePath, to, status, i18n })) + if (matchesMiddleware(middleware, route.page)) { + routesThatMatchMiddleware.add(route.page) + } + netlifyConfig.redirects.push( + ...redirectsForNextRoute({ buildId, route: route.page, basePath, to: ODB_FUNCTION_PATH, status: 200, i18n }), + ) } else { // If the route isn't prerendered, it's SSR netlifyConfig.redirects.push( @@ -174,4 +213,19 @@ export const generateRedirects = async ({ to: HANDLER_FUNCTION_PATH, status: 200, }) + + const middlewareMatches = routesThatMatchMiddleware.size + if (middlewareMatches > 0) { + console.log( + yellowBright(outdent` + There ${ + middlewareMatches === 1 + ? `is one statically-generated or ISR route` + : `are ${middlewareMatches} statically-generated or ISR routes` + } that match a middleware function, which means they will always be served from the SSR function and will not use ISR or be served from the CDN. + If this was not intended, ensure that your middleware only matches routes that you intend to use SSR. + `), + ) + } } +/* eslint-enable max-lines */ diff --git a/src/helpers/utils.ts b/src/helpers/utils.ts index 27ff79f960..7d8df002e9 100644 --- a/src/helpers/utils.ts +++ b/src/helpers/utils.ts @@ -2,13 +2,7 @@ import { NetlifyConfig } from '@netlify/build' import globby from 'globby' import { join } from 'pathe' -import { - OPTIONAL_CATCH_ALL_REGEX, - CATCH_ALL_REGEX, - DYNAMIC_PARAMETER_REGEX, - ODB_FUNCTION_PATH, - HANDLER_FUNCTION_PATH, -} from '../constants' +import { OPTIONAL_CATCH_ALL_REGEX, CATCH_ALL_REGEX, DYNAMIC_PARAMETER_REGEX, HANDLER_FUNCTION_PATH } from '../constants' import { I18n } from './types' @@ -51,8 +45,12 @@ export const netlifyRoutesForNextRouteWithData = ({ route, dataRoute }: { route: ...toNetlifyRoute(route), ] +export const endsWithDynamicSegment = (route: string) => /\/(\*|(:[a-z]+))$/.test(route) + export const routeToDataRoute = (route: string, buildId: string, locale?: string) => - `/_next/data/${buildId}${locale ? `/${locale}` : ''}${route === '/' ? '/index' : route}.json` + `/_next/data/${buildId}${locale ? `/${locale}` : ''}${route === '/' ? '/index' : route}${ + endsWithDynamicSegment(route) ? '' : '.json' + }` const netlifyRoutesForNextRoute = (route: string, buildId: string, i18n?: I18n): Array<string> => { if (!i18n?.locales?.length) { @@ -77,16 +75,6 @@ const netlifyRoutesForNextRoute = (route: string, buildId: string, i18n?: I18n): export const isApiRoute = (route: string) => route.startsWith('/api/') || route === '/api' -export const targetForFallback = (fallback: string | false) => { - if (fallback === null || fallback === false) { - // fallback = null mean "blocking", which uses ODB. For fallback=false then anything prerendered should 404. - // However i18n pages may not have been prerendered, so we still need to hit the origin - return { to: ODB_FUNCTION_PATH, status: 200 } - } - // fallback = true is also ODB - return { to: ODB_FUNCTION_PATH, status: 200 } -} - export const redirectsForNextRoute = ({ route, buildId, diff --git a/test/__snapshots__/index.js.snap b/test/__snapshots__/index.js.snap index 4ebd65561d..3f64b358f3 100644 --- a/test/__snapshots__/index.js.snap +++ b/test/__snapshots__/index.js.snap @@ -23,6 +23,9 @@ exports.resolvePages = () => { require.resolve('../../../.next/server/pages/getStaticProps/with-revalidate.js') require.resolve('../../../.next/server/pages/getStaticProps/withFallback/[...slug].js') require.resolve('../../../.next/server/pages/getStaticProps/withFallback/[id].js') + require.resolve('../../../.next/server/pages/getStaticProps/withFallbackAndMiddleware/[...slug].js') + require.resolve('../../../.next/server/pages/getStaticProps/withFallbackAndMiddleware/[id].js') + require.resolve('../../../.next/server/pages/getStaticProps/withFallbackAndMiddleware/_middleware.js') require.resolve('../../../.next/server/pages/getStaticProps/withFallbackBlocking/[id].js') require.resolve('../../../.next/server/pages/getStaticProps/withRevalidate/[id].js') require.resolve('../../../.next/server/pages/getStaticProps/withRevalidate/withFallback/[id].js') @@ -58,6 +61,9 @@ exports.resolvePages = () => { require.resolve('../../../.next/server/pages/getStaticProps/with-revalidate.js') require.resolve('../../../.next/server/pages/getStaticProps/withFallback/[...slug].js') require.resolve('../../../.next/server/pages/getStaticProps/withFallback/[id].js') + require.resolve('../../../.next/server/pages/getStaticProps/withFallbackAndMiddleware/[...slug].js') + require.resolve('../../../.next/server/pages/getStaticProps/withFallbackAndMiddleware/[id].js') + require.resolve('../../../.next/server/pages/getStaticProps/withFallbackAndMiddleware/_middleware.js') require.resolve('../../../.next/server/pages/getStaticProps/withFallbackBlocking/[id].js') require.resolve('../../../.next/server/pages/getStaticProps/withRevalidate/[id].js') require.resolve('../../../.next/server/pages/getStaticProps/withRevalidate/withFallback/[id].js') @@ -93,6 +99,9 @@ exports.resolvePages = () => { require.resolve('../../../web/.next/server/pages/getStaticProps/with-revalidate.js') require.resolve('../../../web/.next/server/pages/getStaticProps/withFallback/[...slug].js') require.resolve('../../../web/.next/server/pages/getStaticProps/withFallback/[id].js') + require.resolve('../../../web/.next/server/pages/getStaticProps/withFallbackAndMiddleware/[...slug].js') + require.resolve('../../../web/.next/server/pages/getStaticProps/withFallbackAndMiddleware/[id].js') + require.resolve('../../../web/.next/server/pages/getStaticProps/withFallbackAndMiddleware/_middleware.js') require.resolve('../../../web/.next/server/pages/getStaticProps/withFallbackBlocking/[id].js') require.resolve('../../../web/.next/server/pages/getStaticProps/withRevalidate/[id].js') require.resolve('../../../web/.next/server/pages/getStaticProps/withRevalidate/withFallback/[id].js') @@ -128,6 +137,9 @@ exports.resolvePages = () => { require.resolve('../../../web/.next/server/pages/getStaticProps/with-revalidate.js') require.resolve('../../../web/.next/server/pages/getStaticProps/withFallback/[...slug].js') require.resolve('../../../web/.next/server/pages/getStaticProps/withFallback/[id].js') + require.resolve('../../../web/.next/server/pages/getStaticProps/withFallbackAndMiddleware/[...slug].js') + require.resolve('../../../web/.next/server/pages/getStaticProps/withFallbackAndMiddleware/[id].js') + require.resolve('../../../web/.next/server/pages/getStaticProps/withFallbackAndMiddleware/_middleware.js') require.resolve('../../../web/.next/server/pages/getStaticProps/withFallbackBlocking/[id].js') require.resolve('../../../web/.next/server/pages/getStaticProps/withRevalidate/[id].js') require.resolve('../../../web/.next/server/pages/getStaticProps/withRevalidate/withFallback/[id].js') @@ -441,6 +453,126 @@ Array [ "status": 200, "to": "/.netlify/functions/___netlify-handler", }, + Object { + "from": "/getStaticProps/withFallbackAndMiddleware", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/getStaticProps/withFallbackAndMiddleware/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/_next/data/build-id/getStaticProps/withFallbackAndMiddleware/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/en/getStaticProps/withFallbackAndMiddleware", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/en/getStaticProps/withFallbackAndMiddleware/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/_next/data/build-id/en/getStaticProps/withFallbackAndMiddleware/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/es/getStaticProps/withFallbackAndMiddleware", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/es/getStaticProps/withFallbackAndMiddleware/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/_next/data/build-id/es/getStaticProps/withFallbackAndMiddleware/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/fr/getStaticProps/withFallbackAndMiddleware", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/fr/getStaticProps/withFallbackAndMiddleware/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/_next/data/build-id/fr/getStaticProps/withFallbackAndMiddleware/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/middle", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/middle/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/_next/data/build-id/middle/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/en/middle", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/en/middle/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/_next/data/build-id/en/middle/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/es/middle", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/es/middle/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/_next/data/build-id/es/middle/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/fr/middle", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/fr/middle/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, + Object { + "from": "/_next/data/build-id/fr/middle/*", + "status": 200, + "to": "/.netlify/functions/___netlify-handler", + }, Object { "force": true, "from": "/_next/data/build-id/en/getStaticProps/with-revalidate.json", @@ -1173,6 +1305,78 @@ Array [ "status": 200, "to": "/.netlify/builders/___netlify-odb-handler", }, + Object { + "force": false, + "from": "/_next/data/build-id/en/getStaticProps/withFallbackAndMiddleware/:id.json", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/getStaticProps/withFallbackAndMiddleware/:id", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/_next/data/build-id/es/getStaticProps/withFallbackAndMiddleware/:id.json", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/es/getStaticProps/withFallbackAndMiddleware/:id", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/_next/data/build-id/fr/getStaticProps/withFallbackAndMiddleware/:id.json", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/fr/getStaticProps/withFallbackAndMiddleware/:id", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/_next/data/build-id/en/getStaticProps/withFallbackAndMiddleware/:slug/*", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/getStaticProps/withFallbackAndMiddleware/:slug/*", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/_next/data/build-id/es/getStaticProps/withFallbackAndMiddleware/:slug/*", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/es/getStaticProps/withFallbackAndMiddleware/:slug/*", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/_next/data/build-id/fr/getStaticProps/withFallbackAndMiddleware/:slug/*", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, + Object { + "force": false, + "from": "/fr/getStaticProps/withFallbackAndMiddleware/:slug/*", + "status": 200, + "to": "/.netlify/builders/___netlify-odb-handler", + }, Object { "force": false, "from": "/_next/data/build-id/en/getStaticProps/withFallbackBlocking/:id.json", From cb061ca44316a3597fb5b7f813e166cfeb67c010 Mon Sep 17 00:00:00 2001 From: Matt Kane <m@mk.gg> Date: Fri, 28 Jan 2022 13:22:58 +0000 Subject: [PATCH 2/4] chore: fix duplicate rules --- src/helpers/redirects.ts | 37 +++++++------- src/helpers/utils.ts | 6 +-- test/__snapshots__/index.js.snap | 82 -------------------------------- 3 files changed, 22 insertions(+), 103 deletions(-) diff --git a/src/helpers/redirects.ts b/src/helpers/redirects.ts index 3456f3bdf4..dc937330b0 100644 --- a/src/helpers/redirects.ts +++ b/src/helpers/redirects.ts @@ -122,16 +122,20 @@ export const generateRedirects = async ({ // This generates a rewrite for every middleware in every locale, both with and without a splat netlifyConfig.redirects.push( ...middleware - .map((route) => [ - handlerRewrite(`${route}`), - handlerRewrite(`${route}/*`), - handlerRewrite(routeToDataRoute(`${route}/*`, buildId)), - ...(i18n?.locales?.map((locale) => [ - handlerRewrite(`/${locale}${route}`), - handlerRewrite(`/${locale}${route}/*`), - handlerRewrite(routeToDataRoute(`${route}/*`, buildId, locale)), - ]) ?? []), - ]) + .map((route) => { + const unlocalized = [handlerRewrite(`${route}`), handlerRewrite(`${route}/*`)] + if (i18n?.locales?.length > 0) { + const localized = i18n?.locales?.map((locale) => [ + handlerRewrite(`/${locale}${route}`), + handlerRewrite(`/${locale}${route}/*`), + handlerRewrite(`/_next/data/${buildId}/${locale}${route}/*`), + ]) + // With i18n, all data routes are prefixed with the locale, but the HTML also has the unprefixed default + return [...unlocalized, ...localized] + } + return [...unlocalized, handlerRewrite(`/_next/data/${buildId}${route}/*`)] + }) + // Flatten the array of arrays. Can't use flatMap as it might be 2 levels deep .flat(2), ) @@ -166,10 +170,10 @@ export const generateRedirects = async ({ force: true, }), ) + } else if (matchesMiddleware(middleware, route)) { + // Routes that match middleware can't use the ODB + routesThatMatchMiddleware.add(route) } else { - if (matchesMiddleware(middleware, route)) { - routesThatMatchMiddleware.add(route) - } // ISR routes use the ODB handler netlifyConfig.redirects.push( // No i18n, because the route is already localized @@ -195,10 +199,11 @@ export const generateRedirects = async ({ if (route.page in prerenderedDynamicRoutes) { if (matchesMiddleware(middleware, route.page)) { routesThatMatchMiddleware.add(route.page) + } else { + netlifyConfig.redirects.push( + ...redirectsForNextRoute({ buildId, route: route.page, basePath, to: ODB_FUNCTION_PATH, status: 200, i18n }), + ) } - netlifyConfig.redirects.push( - ...redirectsForNextRoute({ buildId, route: route.page, basePath, to: ODB_FUNCTION_PATH, status: 200, i18n }), - ) } else { // If the route isn't prerendered, it's SSR netlifyConfig.redirects.push( diff --git a/src/helpers/utils.ts b/src/helpers/utils.ts index 7d8df002e9..cb46b75ebe 100644 --- a/src/helpers/utils.ts +++ b/src/helpers/utils.ts @@ -45,12 +45,8 @@ export const netlifyRoutesForNextRouteWithData = ({ route, dataRoute }: { route: ...toNetlifyRoute(route), ] -export const endsWithDynamicSegment = (route: string) => /\/(\*|(:[a-z]+))$/.test(route) - export const routeToDataRoute = (route: string, buildId: string, locale?: string) => - `/_next/data/${buildId}${locale ? `/${locale}` : ''}${route === '/' ? '/index' : route}${ - endsWithDynamicSegment(route) ? '' : '.json' - }` + `/_next/data/${buildId}${locale ? `/${locale}` : ''}${route === '/' ? '/index' : route}.json` const netlifyRoutesForNextRoute = (route: string, buildId: string, i18n?: I18n): Array<string> => { if (!i18n?.locales?.length) { diff --git a/test/__snapshots__/index.js.snap b/test/__snapshots__/index.js.snap index 3f64b358f3..91e49adb40 100644 --- a/test/__snapshots__/index.js.snap +++ b/test/__snapshots__/index.js.snap @@ -463,11 +463,6 @@ Array [ "status": 200, "to": "/.netlify/functions/___netlify-handler", }, - Object { - "from": "/_next/data/build-id/getStaticProps/withFallbackAndMiddleware/*", - "status": 200, - "to": "/.netlify/functions/___netlify-handler", - }, Object { "from": "/en/getStaticProps/withFallbackAndMiddleware", "status": 200, @@ -523,11 +518,6 @@ Array [ "status": 200, "to": "/.netlify/functions/___netlify-handler", }, - Object { - "from": "/_next/data/build-id/middle/*", - "status": 200, - "to": "/.netlify/functions/___netlify-handler", - }, Object { "from": "/en/middle", "status": 200, @@ -1305,78 +1295,6 @@ Array [ "status": 200, "to": "/.netlify/builders/___netlify-odb-handler", }, - Object { - "force": false, - "from": "/_next/data/build-id/en/getStaticProps/withFallbackAndMiddleware/:id.json", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/getStaticProps/withFallbackAndMiddleware/:id", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/_next/data/build-id/es/getStaticProps/withFallbackAndMiddleware/:id.json", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/es/getStaticProps/withFallbackAndMiddleware/:id", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/_next/data/build-id/fr/getStaticProps/withFallbackAndMiddleware/:id.json", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/fr/getStaticProps/withFallbackAndMiddleware/:id", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/_next/data/build-id/en/getStaticProps/withFallbackAndMiddleware/:slug/*", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/getStaticProps/withFallbackAndMiddleware/:slug/*", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/_next/data/build-id/es/getStaticProps/withFallbackAndMiddleware/:slug/*", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/es/getStaticProps/withFallbackAndMiddleware/:slug/*", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/_next/data/build-id/fr/getStaticProps/withFallbackAndMiddleware/:slug/*", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, - Object { - "force": false, - "from": "/fr/getStaticProps/withFallbackAndMiddleware/:slug/*", - "status": 200, - "to": "/.netlify/builders/___netlify-odb-handler", - }, Object { "force": false, "from": "/_next/data/build-id/en/getStaticProps/withFallbackBlocking/:id.json", From ff9445a9c9cbe10c3f877909a4f68808a1463dcf Mon Sep 17 00:00:00 2001 From: Matt Kane <m@mk.gg> Date: Wed, 2 Feb 2022 10:48:16 +0000 Subject: [PATCH 3/4] chore: changes from review --- src/helpers/files.ts | 2 +- src/helpers/redirects.ts | 229 ++++++++++++++++++++++++++------------- 2 files changed, 153 insertions(+), 78 deletions(-) diff --git a/src/helpers/files.ts b/src/helpers/files.ts index 8b637ba656..90d0eaa79e 100644 --- a/src/helpers/files.ts +++ b/src/helpers/files.ts @@ -61,7 +61,7 @@ export const matchesRewrite = (file: string, rewrites: Rewrites): boolean => { export const getMiddleware = async (publish: string): Promise<Array<string>> => { const manifestPath = join(publish, 'server', 'middleware-manifest.json') if (existsSync(manifestPath)) { - const manifest = await readJson(manifestPath) + const manifest = await readJson(manifestPath, { throws: false }) return manifest?.sortedMiddleware ?? [] } return [] diff --git a/src/helpers/redirects.ts b/src/helpers/redirects.ts index dc937330b0..6192a6af30 100644 --- a/src/helpers/redirects.ts +++ b/src/helpers/redirects.ts @@ -1,9 +1,9 @@ /* eslint-disable max-lines */ -import { NetlifyConfig } from '@netlify/build' +import type { NetlifyConfig } from '@netlify/build' import { yellowBright } from 'chalk' import { readJSON } from 'fs-extra' -import { NextConfig } from 'next' -import { PrerenderManifest } from 'next/dist/build' +import type { NextConfig } from 'next' +import type { PrerenderManifest, SsgRoute } from 'next/dist/build' import { outdent } from 'outdent' import { join } from 'pathe' @@ -21,7 +21,7 @@ import { } from './utils' const matchesMiddleware = (middleware: Array<string>, route: string): boolean => - middleware?.some((middlewarePath) => route.startsWith(middlewarePath)) + middleware.some((middlewarePath) => route.startsWith(middlewarePath)) const generateLocaleRedirects = ({ i18n, @@ -71,61 +71,23 @@ export const generateStaticRedirects = ({ } } -// eslint-disable-next-line max-lines-per-function -export const generateRedirects = async ({ - netlifyConfig, - nextConfig: { i18n, basePath, trailingSlash, appDir }, - buildId, -}: { - netlifyConfig: NetlifyConfig - nextConfig: Pick<NextConfig, 'i18n' | 'basePath' | 'trailingSlash' | 'appDir'> - buildId: string -}) => { - const { dynamicRoutes: prerenderedDynamicRoutes, routes: prerenderedStaticRoutes }: PrerenderManifest = - await readJSON(join(netlifyConfig.build.publish, 'prerender-manifest.json')) - - const { dynamicRoutes, staticRoutes }: RoutesManifest = await readJSON( - join(netlifyConfig.build.publish, 'routes-manifest.json'), - ) - - netlifyConfig.redirects.push( - ...HIDDEN_PATHS.map((path) => ({ - from: `${basePath}${path}`, - to: '/404.html', - status: 404, - force: true, - })), - ) - - if (i18n && i18n.localeDetection !== false) { - netlifyConfig.redirects.push(...generateLocaleRedirects({ i18n, basePath, trailingSlash })) - } - - // This is only used in prod, so dev uses `next dev` directly - netlifyConfig.redirects.push( - // API routes always need to be served from the regular function - ...getApiRewrites(basePath), - // Preview mode gets forced to the function, to bypass pre-rendered pages, but static files need to be skipped - ...(await getPreviewRewrites({ basePath, appDir })), - ) - - const middleware = await getMiddleware(netlifyConfig.build.publish) - const routesThatMatchMiddleware = new Set<string>() - +/** + * Routes that match middleware need to always use the SSR function + * This generates a rewrite for every middleware in every locale, both with and without a splat + */ +const generateMiddlewareRewrites = ({ basePath, middleware, i18n, buildId }) => { const handlerRewrite = (from: string) => ({ from: `${basePath}${from}`, to: HANDLER_FUNCTION_PATH, status: 200, }) - // Routes that match middleware need to always use the SSR function - // This generates a rewrite for every middleware in every locale, both with and without a splat - netlifyConfig.redirects.push( - ...middleware + return ( + middleware .map((route) => { const unlocalized = [handlerRewrite(`${route}`), handlerRewrite(`${route}/*`)] if (i18n?.locales?.length > 0) { - const localized = i18n?.locales?.map((locale) => [ + const localized = i18n.locales.map((locale) => [ handlerRewrite(`/${locale}${route}`), handlerRewrite(`/${locale}${route}/*`), handlerRewrite(`/_next/data/${buildId}/${locale}${route}/*`), @@ -136,14 +98,26 @@ export const generateRedirects = async ({ return [...unlocalized, handlerRewrite(`/_next/data/${buildId}${route}/*`)] }) // Flatten the array of arrays. Can't use flatMap as it might be 2 levels deep - .flat(2), + .flat(2) ) +} - const staticRouteEntries = Object.entries(prerenderedStaticRoutes) - +const generateStaticIsrRewrites = ({ + staticRouteEntries, + basePath, + i18n, + buildId, + middleware, +}: { + staticRouteEntries: Array<[string, SsgRoute]> + basePath: string + i18n: NextConfig['i18n'] + buildId: string + middleware: Array<string> +}) => { + const staticIsrRoutesThatMatchMiddleware: Array<string> = [] const staticRoutePaths = new Set<string>() - - // First add all static ISR routes + const staticIsrRewrites: NetlifyConfig['redirects'] = [] staticRouteEntries.forEach(([route, { initialRevalidateSeconds }]) => { if (isApiRoute(route)) { return @@ -159,9 +133,9 @@ export const generateRedirects = async ({ route = route.slice(i18n.defaultLocale.length + 1) staticRoutePaths.add(route) if (matchesMiddleware(middleware, route)) { - routesThatMatchMiddleware.add(route) + staticIsrRoutesThatMatchMiddleware.push(route) } - netlifyConfig.redirects.push( + staticIsrRewrites.push( ...redirectsForNextRouteWithData({ route, dataRoute: routeToDataRoute(route, buildId, i18n.defaultLocale), @@ -172,45 +146,146 @@ export const generateRedirects = async ({ ) } else if (matchesMiddleware(middleware, route)) { // Routes that match middleware can't use the ODB - routesThatMatchMiddleware.add(route) + staticIsrRoutesThatMatchMiddleware.push(route) } else { // ISR routes use the ODB handler - netlifyConfig.redirects.push( + staticIsrRewrites.push( // No i18n, because the route is already localized ...redirectsForNextRoute({ route, basePath, to: ODB_FUNCTION_PATH, force: true, buildId, i18n: null }), ) } }) - // Add rewrites for all static SSR routes. This is Next 12+ - staticRoutes?.forEach((route) => { - if (staticRoutePaths.has(route.page) || isApiRoute(route.page)) { - // Prerendered static routes are either handled by the CDN or are ISR - return - } - netlifyConfig.redirects.push( - ...redirectsForNextRoute({ route: route.page, buildId, basePath, to: HANDLER_FUNCTION_PATH, i18n }), - ) - }) - // Add rewrites for all dynamic routes (both SSR and ISR) + + return { + staticRoutePaths, + staticIsrRoutesThatMatchMiddleware, + staticIsrRewrites, + } +} + +/** + * Generate rewrites for all dynamic routes + */ +const generateDynamicRewrites = ({ + dynamicRoutes, + prerenderedDynamicRoutes, + middleware, + basePath, + buildId, + i18n, +}: { + dynamicRoutes: RoutesManifest['dynamicRoutes'] + prerenderedDynamicRoutes: PrerenderManifest['dynamicRoutes'] + basePath: string + i18n: NextConfig['i18n'] + buildId: string + middleware: Array<string> +}) => { + const dynamicRewrites: NetlifyConfig['redirects'] = [] + const dynamicRoutesThatMatchMiddleware: Array<string> = [] dynamicRoutes.forEach((route) => { if (isApiRoute(route.page)) { return } if (route.page in prerenderedDynamicRoutes) { if (matchesMiddleware(middleware, route.page)) { - routesThatMatchMiddleware.add(route.page) + dynamicRoutesThatMatchMiddleware.push(route.page) } else { - netlifyConfig.redirects.push( + dynamicRewrites.push( ...redirectsForNextRoute({ buildId, route: route.page, basePath, to: ODB_FUNCTION_PATH, status: 200, i18n }), ) } } else { // If the route isn't prerendered, it's SSR - netlifyConfig.redirects.push( + dynamicRewrites.push( ...redirectsForNextRoute({ route: route.page, buildId, basePath, to: HANDLER_FUNCTION_PATH, i18n }), ) } }) + return { + dynamicRoutesThatMatchMiddleware, + dynamicRewrites, + } +} + +export const generateRedirects = async ({ + netlifyConfig, + nextConfig: { i18n, basePath, trailingSlash, appDir }, + buildId, +}: { + netlifyConfig: NetlifyConfig + nextConfig: Pick<NextConfig, 'i18n' | 'basePath' | 'trailingSlash' | 'appDir'> + buildId: string +}) => { + const { dynamicRoutes: prerenderedDynamicRoutes, routes: prerenderedStaticRoutes }: PrerenderManifest = + await readJSON(join(netlifyConfig.build.publish, 'prerender-manifest.json')) + + const { dynamicRoutes, staticRoutes }: RoutesManifest = await readJSON( + join(netlifyConfig.build.publish, 'routes-manifest.json'), + ) + + netlifyConfig.redirects.push( + ...HIDDEN_PATHS.map((path) => ({ + from: `${basePath}${path}`, + to: '/404.html', + status: 404, + force: true, + })), + ) + + if (i18n && i18n.localeDetection !== false) { + netlifyConfig.redirects.push(...generateLocaleRedirects({ i18n, basePath, trailingSlash })) + } + + // This is only used in prod, so dev uses `next dev` directly + netlifyConfig.redirects.push( + // API routes always need to be served from the regular function + ...getApiRewrites(basePath), + // Preview mode gets forced to the function, to bypass pre-rendered pages, but static files need to be skipped + ...(await getPreviewRewrites({ basePath, appDir })), + ) + + const middleware = await getMiddleware(netlifyConfig.build.publish) + + netlifyConfig.redirects.push(...generateMiddlewareRewrites({ basePath, i18n, middleware, buildId })) + + const staticRouteEntries = Object.entries(prerenderedStaticRoutes) + + const routesThatMatchMiddleware: Array<string> = [] + + const { staticRoutePaths, staticIsrRewrites, staticIsrRoutesThatMatchMiddleware } = generateStaticIsrRewrites({ + staticRouteEntries, + basePath, + i18n, + buildId, + middleware, + }) + + routesThatMatchMiddleware.push(...staticIsrRoutesThatMatchMiddleware) + + netlifyConfig.redirects.push(...staticIsrRewrites) + + // Add rewrites for all static SSR routes. This is Next 12+ + staticRoutes?.forEach((route) => { + if (staticRoutePaths.has(route.page) || isApiRoute(route.page)) { + // Prerendered static routes are either handled by the CDN or are ISR + return + } + netlifyConfig.redirects.push( + ...redirectsForNextRoute({ route: route.page, buildId, basePath, to: HANDLER_FUNCTION_PATH, i18n }), + ) + }) + // Add rewrites for all dynamic routes (both SSR and ISR) + const { dynamicRewrites, dynamicRoutesThatMatchMiddleware } = generateDynamicRewrites({ + dynamicRoutes, + prerenderedDynamicRoutes, + middleware, + basePath, + buildId, + i18n, + }) + netlifyConfig.redirects.push(...dynamicRewrites) + routesThatMatchMiddleware.push(...dynamicRoutesThatMatchMiddleware) // Final fallback netlifyConfig.redirects.push({ @@ -219,15 +294,15 @@ export const generateRedirects = async ({ status: 200, }) - const middlewareMatches = routesThatMatchMiddleware.size + const middlewareMatches = new Set(routesThatMatchMiddleware).size if (middlewareMatches > 0) { console.log( yellowBright(outdent` There ${ middlewareMatches === 1 - ? `is one statically-generated or ISR route` - : `are ${middlewareMatches} statically-generated or ISR routes` - } that match a middleware function, which means they will always be served from the SSR function and will not use ISR or be served from the CDN. + ? `is one statically-generated or ISR route that matches` + : `are ${middlewareMatches} statically-generated or ISR routes that match` + } a middleware function. Matched routes will always be served from the SSR function and will not use ISR or be served from the CDN. If this was not intended, ensure that your middleware only matches routes that you intend to use SSR. `), ) From 012612aa7b587126d22864d91770ad2912943054 Mon Sep 17 00:00:00 2001 From: Matt Kane <m@mk.gg> Date: Wed, 2 Feb 2022 11:07:13 +0000 Subject: [PATCH 4/4] chore: add return types --- src/helpers/redirects.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/helpers/redirects.ts b/src/helpers/redirects.ts index 6192a6af30..67090f1d3f 100644 --- a/src/helpers/redirects.ts +++ b/src/helpers/redirects.ts @@ -114,7 +114,11 @@ const generateStaticIsrRewrites = ({ i18n: NextConfig['i18n'] buildId: string middleware: Array<string> -}) => { +}): { + staticRoutePaths: Set<string> + staticIsrRoutesThatMatchMiddleware: Array<string> + staticIsrRewrites: NetlifyConfig['redirects'] +} => { const staticIsrRoutesThatMatchMiddleware: Array<string> = [] const staticRoutePaths = new Set<string>() const staticIsrRewrites: NetlifyConfig['redirects'] = [] @@ -180,7 +184,10 @@ const generateDynamicRewrites = ({ i18n: NextConfig['i18n'] buildId: string middleware: Array<string> -}) => { +}): { + dynamicRoutesThatMatchMiddleware: Array<string> + dynamicRewrites: NetlifyConfig['redirects'] +} => { const dynamicRewrites: NetlifyConfig['redirects'] = [] const dynamicRoutesThatMatchMiddleware: Array<string> = [] dynamicRoutes.forEach((route) => {