chore: changes from review

Author: ascorbic

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 []

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.
       `),
     )