fix: don't move files to the CDN if they match redirect/rewrite rules (#832)

* fix: don't move files to the CDN if they match redirects or beforeFiles rewrites

* chore: add tests

Release-As: 4.0.0-beta.10

Author: ascorbic

demo/next.config.js

@@ -4,7 +4,7 @@ module.exports = {
   generateBuildId: () => 'build-id',
   i18n: {
     defaultLocale: 'en',
-    locales: ['en', 'es', 'fr']
+    locales: ['en', 'es', 'fr'],
   },
   async headers() {
     return [
@@ -14,7 +14,7 @@ module.exports = {
           {
             key: 'x-custom-header',
             value: 'my custom header value',
-          }
+          },
         ],
       },
     ]
@@ -29,8 +29,8 @@ module.exports = {
         {
           source: '/old/:path*',
           destination: '/:path*',
-        }
-      ]
+        },
+      ],
     }
   },
   // Redirects allow you to redirect an incoming request path to a different destination path.

demo/pages/old/image.js

@@ -0,0 +1,7 @@
+const Images = () => (
+  <div>
+    <p>If you can see this, rewrites aren&apos;t working</p>
+  </div>
+)
+
+export default Images

demo/pages/redirectme.js

@@ -0,0 +1,7 @@
+const Redirect = () => (
+  <div>
+    <p>If you can see this, redirects aren&apos;t working</p>
+  </div>
+)
+
+export default Redirect

src/helpers/files.js

@@ -1,4 +1,4 @@
-//  @ts-check
+/* eslint-disable max-lines */
 const { cpus } = require('os')
 
 const { yellowBright } = require('chalk')
@@ -28,10 +28,36 @@ const matchMiddleware = (middleware, filePath) =>
       filePath === middlewarePath || filePath === `${middlewarePath}.html` || filePath.startsWith(`${middlewarePath}/`),
   )
 
+const matchesRedirect = (file, redirects) => {
+  if (!Array.isArray(redirects)) {
+    return false
+  }
+  return redirects.some((redirect) => {
+    if (!redirect.regex || redirect.internal) {
+      return false
+    }
+    // Strips the extension from the file path
+    return new RegExp(redirect.regex).test(`/${file.slice(0, -5)}`)
+  })
+}
+
+const matchesRewrite = (file, rewrites) => {
+  if (Array.isArray(rewrites)) {
+    return matchesRedirect(file, rewrites)
+  }
+  if (!Array.isArray(rewrites?.beforeFiles)) {
+    return false
+  }
+  return matchesRedirect(file, rewrites.beforeFiles)
+}
+
+exports.matchesRedirect = matchesRedirect
+exports.matchesRewrite = matchesRewrite
 exports.matchMiddleware = matchMiddleware
 exports.stripLocale = stripLocale
 exports.isDynamicRoute = isDynamicRoute
 
+// eslint-disable-next-line max-lines-per-function
 exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => {
   console.log('Moving static page files to serve from CDN...')
   const outputDir = join(netlifyConfig.build.publish, target === 'server' ? 'server' : 'serverless')
@@ -48,6 +74,7 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => {
   }
 
   const prerenderManifest = await readJson(join(netlifyConfig.build.publish, 'prerender-manifest.json'))
+  const { redirects, rewrites } = await readJson(join(netlifyConfig.build.publish, 'routes-manifest.json'))
 
   const isrFiles = new Set()
 
@@ -79,6 +106,8 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => {
 
   const matchingMiddleware = new Set()
   const matchedPages = new Set()
+  const matchedRedirects = new Set()
+  const matchedRewrites = 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))
@@ -91,6 +120,14 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => {
     if (isDynamicRoute(filePath)) {
       return
     }
+    if (matchesRedirect(filePath, redirects)) {
+      matchedRedirects.add(filePath)
+      return
+    }
+    if (matchesRewrite(filePath, rewrites)) {
+      matchedRewrites.add(filePath)
+      return
+    }
     // Middleware matches against the unlocalised path
     const unlocalizedPath = stripLocale(rawPath, i18n?.locales)
     const middlewarePath = matchMiddleware(middleware, unlocalizedPath)
@@ -110,7 +147,8 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => {
       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.
+      } 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.
       `),
     )
 
@@ -119,6 +157,8 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => {
         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
@@ -128,6 +168,40 @@ exports.moveStaticPages = async ({ netlifyConfig, target, i18n }) => {
           The following files matched middleware and were not moved to the CDN:
 
           ${yellowBright([...matchedPages].map((mid) => `- ${mid}`).join('\n'))}
+
+          ────────────────────────────────────────────────────────────────
+        `,
+      )
+    }
+  }
+
+  if (matchedRedirects.size !== 0 || matchedRewrites.size !== 0) {
+    console.log(
+      yellowBright(outdent`
+        Skipped moving ${
+          matchedRedirects.size + matchedRewrites.size
+        } files because they match redirects or beforeFiles rewrites, so cannot be deployed to the CDN and will be served from the origin instead.
+      `),
+    )
+    if (matchedRedirects.size < 50 && matchedRedirects.size !== 0) {
+      console.log(
+        outdent`
+          The following files matched redirects and were not moved to the CDN:
+
+          ${yellowBright([...matchedRedirects].map((mid) => `- ${mid}`).join('\n'))}
+
+          ────────────────────────────────────────────────────────────────
+        `,
+      )
+    }
+    if (matchedRewrites.size < 50 && matchedRewrites.size !== 0) {
+      console.log(
+        outdent`
+          The following files matched beforeFiles rewrites and were not moved to the CDN:
+
+          ${yellowBright([...matchedRewrites].map((mid) => `- ${mid}`).join('\n'))}
+
+          ────────────────────────────────────────────────────────────────
         `,
       )
     }
@@ -151,3 +225,4 @@ exports.movePublicFiles = async ({ appDir, publish }) => {
     await copy(publicDir, `${publish}/`)
   }
 }
+/* eslint-enable max-lines */

test/index.js

@@ -10,7 +10,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 { matchMiddleware, stripLocale, matchesRedirect, matchesRewrite } = require('../src/helpers/files')
 
 const FIXTURES_DIR = `${__dirname}/fixtures`
 const SAMPLE_PROJECT_DIR = `${__dirname}/../demo`
@@ -32,6 +32,45 @@ const utils = {
   },
 }
 
+const REDIRECTS = [
+  {
+    source: '/:file((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/]+\\.\\w+)/',
+    destination: '/:file',
+    locale: false,
+    internal: true,
+    statusCode: 308,
+    regex: '^(?:/((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/]+\\.\\w+))/$',
+  },
+  {
+    source: '/:notfile((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/\\.]+)',
+    destination: '/:notfile/',
+    locale: false,
+    internal: true,
+    statusCode: 308,
+    regex: '^(?:/((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/\\.]+))$',
+  },
+  {
+    source: '/en/redirectme',
+    destination: '/',
+    statusCode: 308,
+    regex: '^(?!/_next)/en/redirectme(?:/)?$',
+  },
+  {
+    source: '/:nextInternalLocale(en|es|fr)/redirectme',
+    destination: '/:nextInternalLocale/',
+    statusCode: 308,
+    regex: '^(?!/_next)(?:/(en|es|fr))/redirectme(?:/)?$',
+  },
+]
+
+const REWRITES = [
+  {
+    source: '/:nextInternalLocale(en|es|fr)/old/:path*',
+    destination: '/:nextInternalLocale/:path*',
+    regex: '^(?:/(en|es|fr))/old(?:/((?:[^/]+?)(?:/(?:[^/]+?))*))?(?:/)?$',
+  },
+]
+
 // Temporary switch cwd
 const changeCwd = function (cwd) {
   const originalCwd = process.cwd()
@@ -450,6 +489,46 @@ describe('utility functions', () => {
       expect(stripLocale(path, locales)).toEqual(path)
     }
   })
+
+  test('matchesRedirect correctly matches paths with locales', () => {
+    const paths = ['en/redirectme.html', 'en/redirectme.json', 'fr/redirectme.html', 'fr/redirectme.json']
+    paths.forEach((path) => {
+      expect(matchesRedirect(path, REDIRECTS)).toBeTruthy()
+    })
+  })
+
+  test("matchesRedirect doesn't match paths with invalid locales", () => {
+    const paths = ['dk/redirectme.html', 'dk/redirectme.json', 'gr/redirectme.html', 'gr/redirectme.json']
+    paths.forEach((path) => {
+      expect(matchesRedirect(path, REDIRECTS)).toBeFalsy()
+    })
+  })
+
+  test("matchesRedirect doesn't match internal redirects", () => {
+    const paths = ['en/notrailingslash']
+    paths.forEach((path) => {
+      expect(matchesRedirect(path, REDIRECTS)).toBeFalsy()
+    })
+  })
+
+  it('matchesRewrite matches array of rewrites', () => {
+    expect(matchesRewrite('en/old/page.html', REWRITES)).toBeTruthy()
+  })
+
+  it('matchesRewrite matches beforeFiles rewrites', () => {
+    expect(matchesRewrite('en/old/page.html', { beforeFiles: REWRITES })).toBeTruthy()
+  })
+
+  it("matchesRewrite doesn't match afterFiles rewrites", () => {
+    expect(matchesRewrite('en/old/page.html', { afterFiles: REWRITES })).toBeFalsy()
+  })
+
+  it('matchesRewrite matches various paths', () => {
+    const paths = ['en/old/page.html', 'fr/old/page.html', 'en/old/deep/page.html', 'en/old.html']
+    paths.forEach((path) => {
+      expect(matchesRewrite(path, REWRITES)).toBeTruthy()
+    })
+  })
 })
 
 describe('function helpers', () => {