chore: warn for problematic user rewrites (#1010)

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

Author: ascorbic

src/helpers/redirects.ts

@@ -42,6 +42,21 @@ const generateLocaleRedirects = ({
   return redirects
 }
 
+export const generateStaticRedirects = ({
+  netlifyConfig,
+  nextConfig: { i18n, basePath },
+}: {
+  netlifyConfig: NetlifyConfig
+  nextConfig: Pick<NextConfig, 'i18n' | 'basePath'>
+}) => {
+  // Static files are in `static`
+  netlifyConfig.redirects.push({ from: `${basePath}/_next/static/*`, to: `/static/:splat`, status: 200 })
+
+  if (i18n) {
+    netlifyConfig.redirects.push({ from: `${basePath}/:locale/_next/static/*`, to: `/static/:splat`, status: 200 })
+  }
+}
+
 export const generateRedirects = async ({
   netlifyConfig,
   nextConfig: { i18n, basePath, trailingSlash, appDir },
@@ -95,16 +110,10 @@ export const generateRedirects = async ({
     dataRedirects.push(...netlifyRoutesForNextRoute(dataRoute))
   })
 
-  if (i18n) {
-    netlifyConfig.redirects.push({ from: `${basePath}/:locale/_next/static/*`, to: `/static/:splat`, status: 200 })
-  }
-
   const publicFiles = await globby('**/*', { cwd: join(appDir, 'public') })
 
   // This is only used in prod, so dev uses `next dev` directly
   netlifyConfig.redirects.push(
-    // Static files are in `static`
-    { from: `${basePath}/_next/static/*`, to: `/static/:splat`, status: 200 },
     // API routes always need to be served from the regular function
     {
       from: `${basePath}/api`,

src/helpers/verification.ts

@@ -1,7 +1,8 @@
+/* eslint-disable max-lines */
 import { existsSync, promises } from 'fs'
 import path, { relative } from 'path'
 
-import { NetlifyPluginUtils } from '@netlify/build'
+import { NetlifyConfig, NetlifyPluginUtils } from '@netlify/build'
 import { yellowBright, greenBright, blueBright, redBright, reset } from 'chalk'
 import { async as StreamZip } from 'node-stream-zip'
 import { outdent } from 'outdent'
@@ -136,3 +137,55 @@ export const checkZipSize = async (file: string, maxSize: number = LAMBDA_MAX_SI
     greenBright`\n\nFor more information on fixing this, see ${blueBright`https://ntl.fyi/large-next-functions`}`,
   )
 }
+
+export const getProblematicUserRewrites = ({
+  redirects,
+  basePath,
+}: {
+  redirects: NetlifyConfig['redirects']
+  basePath: string
+}) => {
+  const userRewrites: NetlifyConfig['redirects'] = []
+  for (const redirect of redirects) {
+    // This is the first of the plugin-generated redirects so we can stop checking
+    if (redirect.from === `${basePath}/_next/static/*` && redirect.to === `/static/:splat` && redirect.status === 200) {
+      break
+    }
+    if (
+      // Redirects are fine
+      (redirect.status === 200 || redirect.status === 404) &&
+      // Rewriting to a function is also fine
+      !redirect.to.startsWith('/.netlify/') &&
+      // ...so is proxying
+      !redirect.to.startsWith('http')
+    ) {
+      userRewrites.push(redirect)
+    }
+  }
+  return userRewrites
+}
+
+export const warnForProblematicUserRewrites = ({
+  redirects,
+  basePath,
+}: {
+  redirects: NetlifyConfig['redirects']
+  basePath: string
+}) => {
+  const userRewrites = getProblematicUserRewrites({ redirects, basePath })
+  if (userRewrites.length === 0) {
+    return
+  }
+  console.log(
+    yellowBright(outdent`
+      You have the following Netlify rewrite${
+        userRewrites.length === 1 ? '' : 's'
+      } that might cause conflicts with the Next.js plugin:
+
+      ${reset(userRewrites.map(({ from, to, status }) => `- ${from} ${to} ${status}`).join('\n'))}
+
+      For more information, see https://ntl.fyi/next-rewrites
+    `),
+  )
+}
+/* eslint-enable max-lines */

src/index.ts

@@ -7,13 +7,14 @@ import { restoreCache, saveCache } from './helpers/cache'
 import { getNextConfig, configureHandlerFunctions } from './helpers/config'
 import { moveStaticPages, movePublicFiles, patchNextFiles, unpatchNextFiles } from './helpers/files'
 import { generateFunctions, setupImageFunction, generatePagesResolver } from './helpers/functions'
-import { generateRedirects } from './helpers/redirects'
+import { generateRedirects, generateStaticRedirects } from './helpers/redirects'
 import {
   verifyNetlifyBuildVersion,
   checkNextSiteHasBuilt,
   checkForRootPublish,
   checkZipSize,
   checkForOldFunctions,
+  warnForProblematicUserRewrites,
 } from './helpers/verification'
 
 const plugin: NetlifyPlugin = {
@@ -73,6 +74,11 @@ const plugin: NetlifyPlugin = {
       await moveStaticPages({ target, netlifyConfig, i18n })
     }
 
+    await generateStaticRedirects({
+      netlifyConfig,
+      nextConfig: { basePath, i18n },
+    })
+
     await setupImageFunction({ constants, imageconfig: images, netlifyConfig, basePath })
 
     await generateRedirects({
@@ -82,18 +88,22 @@ const plugin: NetlifyPlugin = {
   },
 
   async onPostBuild({
-    netlifyConfig,
+    netlifyConfig: {
+      build: { publish },
+      redirects,
+    },
     utils: {
       cache,
       functions,
       build: { failBuild },
     },
     constants: { FUNCTIONS_DIST },
   }) {
-    await saveCache({ cache, publish: netlifyConfig.build.publish })
+    await saveCache({ cache, publish })
     await checkForOldFunctions({ functions })
     await checkZipSize(join(FUNCTIONS_DIST, `${ODB_FUNCTION_NAME}.zip`))
-    const { basePath } = await getNextConfig({ publish: netlifyConfig.build.publish, failBuild })
+    const { basePath } = await getNextConfig({ publish, failBuild })
+    warnForProblematicUserRewrites({ basePath, redirects })
     await unpatchNextFiles(basePath)
   },
 }

test/index.js

@@ -1,4 +1,4 @@
-const { writeJSON, unlink, existsSync, readFileSync, copy, ensureDir, readJson } = require('fs-extra')
+const { writeJSON, unlink, existsSync, readFileSync, copy, ensureDir, readJson, writeFile } = require('fs-extra')
 const path = require('path')
 const process = require('process')
 const os = require('os')
@@ -12,6 +12,8 @@ const { HANDLER_FUNCTION_NAME, ODB_FUNCTION_NAME } = require('../src/constants')
 const { join } = require('pathe')
 const { matchMiddleware, stripLocale, matchesRedirect, matchesRewrite } = require('../src/helpers/files')
 const { dirname } = require('path')
+const { getProblematicUserRewrites } = require('../src/helpers/verification')
+const { outdent } = require('outdent')
 
 const FIXTURES_DIR = `${__dirname}/fixtures`
 const SAMPLE_PROJECT_DIR = `${__dirname}/../demos/default`
@@ -442,6 +444,33 @@ describe('onPostBuild', () => {
 
     console.log = oldLog
   })
+
+  test('finds problematic user rewrites', async () => {
+    await moveNextDist()
+    const rewrites = getProblematicUserRewrites({
+      redirects: [
+        { from: '/previous', to: '/rewrites-are-a-problem', status: 200 },
+        { from: '/api', to: '/.netlify/functions/are-ok', status: 200 },
+        { from: '/remote', to: 'http://example.com/proxying/is/ok', status: 200 },
+        { from: '/old', to: '/redirects-are-fine' },
+        { from: '/*', to: '/404-is-a-problem', status: 404 },
+        ...netlifyConfig.redirects,
+      ],
+      basePath: '',
+    })
+    expect(rewrites).toEqual([
+      {
+        from: '/previous',
+        status: 200,
+        to: '/rewrites-are-a-problem',
+      },
+      {
+        from: '/*',
+        status: 404,
+        to: '/404-is-a-problem',
+      },
+    ])
+  })
 })
 
 describe('utility functions', () => {