feat: don't move files to CDN if they match middleware (#812)

* feat: don't move files to CDN if they match middleware

* chore: lint

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

Author: ascorbic

src/helpers/files.js

@@ -1,8 +1,10 @@
 //  @ts-check
 const { cpus } = require('os')
 
+const { yellowBright } = require('chalk')
 const { existsSync, readJson, move, cpSync, copy, writeJson } = require('fs-extra')
 const globby = require('globby')
+const { outdent } = require('outdent')
 const pLimit = require('p-limit')
 const { join } = require('pathe')
 const slash = require('slash')
@@ -11,12 +13,41 @@ const TEST_ROUTE = /(|\/)\[[^/]+?](\/|\.html|$)/
 
 const isDynamicRoute = (route) => TEST_ROUTE.test(route)
 
-exports.moveStaticPages = async ({ netlifyConfig, target, i18n, failBuild }) => {
+const stripLocale = (rawPath, locales = []) => {
+  const [locale, ...segments] = rawPath.split('/')
+  if (locales.includes(locale)) {
+    return segments.join('/')
+  }
+  return rawPath
+}
+
+const matchMiddleware = (middleware, filePath) =>
+  middleware.includes('') ||
+  middleware?.find(
+    (middlewarePath) =>
+      filePath === middlewarePath || filePath === `${middlewarePath}.html` || filePath.startsWith(`${middlewarePath}/`),
+  )
+
+exports.matchMiddleware = matchMiddleware
+exports.stripLocale = stripLocale
+exports.isDynamicRoute = isDynamicRoute
+
+exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => {
   console.log('Moving static page files to serve from CDN...')
-  const root = join(netlifyConfig.build.publish, target === 'server' ? 'server' : 'serverless', 'pages')
+  const outputDir = join(netlifyConfig.build.publish, target === 'server' ? 'server' : 'serverless')
+  const root = join(outputDir, 'pages')
 
-  const files = []
+  // 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 files = []
   const moveFile = async (file) => {
     const source = join(root, file)
     files.push(file)
@@ -29,18 +60,58 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n, failBuild }) =>
     dot: true,
   })
 
+  const matchingMiddleware = new Set()
+  const matchedPages = new Set()
+
   // Limit concurrent file moves to number of cpus or 2 if there is only 1
   const limit = pLimit(Math.max(2, cpus().length))
   const promises = pages.map(async (rawPath) => {
     const filePath = slash(rawPath)
     if (isDynamicRoute(filePath)) {
       return
     }
+    // Middleware matches against the unlocalised path
+    const unlocalizedPath = stripLocale(rawPath, i18n?.locales)
+    const middlewarePath = matchMiddleware(middleware, unlocalizedPath)
+    // If a file matches middleware it can't be offloaded to the CDN, and needs to stay at the origin to be served by next/server
+    if (middlewarePath) {
+      matchingMiddleware.add(middlewarePath)
+      matchedPages.add(rawPath)
+      return
+    }
     return limit(moveFile, filePath)
   })
   await Promise.all(promises)
   console.log(`Moved ${files.length} files`)
 
+  if (matchedPages.size !== 0) {
+    console.log(
+      yellowBright(outdent`
+        Skipped moving ${matchedPages.size} ${
+        matchedPages.size === 1 ? 'file because it matches' : 'files because they match'
+      } middleware, so cannot be deployed to the CDN and will be served from the origin instead. This is fine, but we're letting you know because it may not be what you expect.
+      `),
+    )
+
+    console.log(
+      outdent`
+        The following middleware matched statically-rendered pages:
+
+        ${yellowBright([...matchingMiddleware].map((mid) => `- /${mid}/_middleware`).join('\n'))}
+      `,
+    )
+    // There could potentially be thousands of matching pages, so we don't want to spam the console with this
+    if (matchedPages.size < 50) {
+      console.log(
+        outdent`
+          The following files matched middleware and were not moved to the CDN:
+
+          ${yellowBright([...matchedPages].map((mid) => `- ${mid}`).join('\n'))}
+        `,
+      )
+    }
+  }
+
   // Write the manifest for use in the serverless functions
   await writeJson(join(netlifyConfig.build.publish, 'static-manifest.json'), files)
 

test/__snapshots__/index.js.snap

@@ -99,7 +99,6 @@ Array [
   "en/getStaticProps/withRevalidate/withFallback/2.html",
   "en/getStaticProps/withRevalidate/withFallback/2.json",
   "en/image.html",
-  "en/middle.html",
   "en/previewTest.html",
   "en/previewTest.json",
   "en/static.html",
@@ -108,7 +107,6 @@ Array [
   "es/getStaticProps/with-revalidate.html",
   "es/getStaticProps/with-revalidate.json",
   "es/image.html",
-  "es/middle.html",
   "es/previewTest.html",
   "es/previewTest.json",
   "es/static.html",
@@ -117,7 +115,6 @@ Array [
   "fr/getStaticProps/with-revalidate.html",
   "fr/getStaticProps/with-revalidate.json",
   "fr/image.html",
-  "fr/middle.html",
   "fr/previewTest.html",
   "fr/previewTest.json",
   "fr/static.html",

test/index.js

@@ -9,6 +9,7 @@ const plugin = require('../src')
 
 const { HANDLER_FUNCTION_NAME, ODB_FUNCTION_NAME } = require('../src/constants')
 const { join } = require('pathe')
+const { matchMiddleware, stripLocale } = require('../src/helpers/files')
 
 const FIXTURES_DIR = `${__dirname}/fixtures`
 const SAMPLE_PROJECT_DIR = `${__dirname}/../demo`
@@ -208,8 +209,9 @@ describe('onBuild()', () => {
     await moveNextDist()
     process.env.EXPERIMENTAL_MOVE_STATIC_PAGES = 'true'
     await plugin.onBuild(defaultArgs)
-    expect(existsSync(path.resolve('.next/static-manifest.json'))).toBeTruthy()
-    const data = JSON.parse(readFileSync(path.resolve('.next/static-manifest.json'), 'utf8')).sort()
+    const manifestPath = path.resolve('.next/static-manifest.json')
+    expect(existsSync(manifestPath)).toBeTruthy()
+    const data = (await readJson(manifestPath)).sort()
     expect(data).toMatchSnapshot()
     delete process.env.EXPERIMENTAL_MOVE_STATIC_PAGES
   })
@@ -247,6 +249,17 @@ describe('onBuild()', () => {
     delete process.env.EXPERIMENTAL_MOVE_STATIC_PAGES
   })
 
+  test('skips static files that match middleware', async () => {
+    await moveNextDist()
+    process.env.EXPERIMENTAL_MOVE_STATIC_PAGES = 'true'
+    await plugin.onBuild(defaultArgs)
+
+    expect(existsSync(path.resolve(path.join('.next', 'en', 'middle.html')))).toBeFalsy()
+    expect(existsSync(path.resolve(path.join('.next', 'server', 'pages', 'en', 'middle.html')))).toBeTruthy()
+
+    delete process.env.EXPERIMENTAL_MOVE_STATIC_PAGES
+  })
+
   test('sets correct config', async () => {
     await moveNextDist()
 
@@ -355,3 +368,78 @@ describe('onPostBuild', () => {
     console.log = oldLog
   })
 })
+
+describe('utility functions', () => {
+  test('middleware tester matches correct paths', () => {
+    const middleware = ['middle', 'sub/directory']
+    const paths = [
+      'middle.html',
+      'middle',
+      'middle/',
+      'middle/ware',
+      'sub/directory',
+      'sub/directory.html',
+      'sub/directory/child',
+      'sub/directory/child.html',
+    ]
+    for (const path of paths) {
+      expect(matchMiddleware(middleware, path)).toBeTruthy()
+    }
+  })
+
+  test('middleware tester does not match incorrect paths', () => {
+    const middleware = ['middle', 'sub/directory']
+    const paths = [
+      'middl',
+      '',
+      'somethingelse',
+      'another.html',
+      'another/middle.html',
+      'sub/anotherdirectory.html',
+      'sub/directoryelse',
+      'sub/directoryelse.html',
+    ]
+    for (const path of paths) {
+      expect(matchMiddleware(middleware, path)).toBeFalsy()
+    }
+  })
+
+  test('middleware tester matches root middleware', () => {
+    const middleware = ['']
+    const paths = [
+      'middl',
+      '',
+      'somethingelse',
+      'another.html',
+      'another/middle.html',
+      'sub/anotherdirectory.html',
+      'sub/directoryelse',
+      'sub/directoryelse.html',
+    ]
+    for (const path of paths) {
+      expect(matchMiddleware(middleware, path)).toBeTruthy()
+    }
+  })
+
+  test('stripLocale correctly strips matching locales', () => {
+    const locales = ['en', 'fr', 'en-GB']
+    const paths = [
+      ['en/file.html', 'file.html'],
+      ['fr/file.html', 'file.html'],
+      ['en-GB/file.html', 'file.html'],
+      ['file.html', 'file.html'],
+    ]
+
+    for (const [path, expected] of paths) {
+      expect(stripLocale(path, locales)).toEqual(expected)
+    }
+  })
+
+  test('stripLocale does not touch non-matching matching locales', () => {
+    const locales = ['en', 'fr', 'en-GB']
+    const paths = ['de/file.html', 'enfile.html', 'en-US/file.html']
+    for (const path of paths) {
+      expect(stripLocale(path, locales)).toEqual(path)
+    }
+  })
+})