feat: check for netlify forms workaround (#2523)

* feat: check for forms workaround before checking for forms

* test: add forms workaround test

* fix: ensure forms test is actually testing!

* test: add negative match case

* chore: console log for windows test

* feat: update forms warning

* fix: ensure forms workaround globbing works in windows

Author: orinokai

src/build/content/prerendered.ts

@@ -18,7 +18,7 @@ import type {
   NetlifyIncrementalCacheValue,
 } from '../../shared/cache-types.cjs'
 import type { PluginContext } from '../plugin-context.js'
-import { verifyNoNetlifyForms } from '../verification.js'
+import { verifyNetlifyForms } from '../verification.js'
 
 const tracer = wrapTracer(trace.getTracer('Next runtime'))
 
@@ -172,7 +172,7 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise<void>
 
               // Netlify Forms are not support and require a workaround
               if (value.kind === 'PAGE' || value.kind === 'APP_PAGE') {
-                verifyNoNetlifyForms(ctx, value.html)
+                verifyNetlifyForms(ctx, value.html)
               }
 
               await writeCacheEntry(key, value, lastModified, ctx)

src/build/content/static.ts

@@ -8,7 +8,7 @@ import glob from 'fast-glob'
 
 import { encodeBlobKey } from '../../shared/blobkey.js'
 import { PluginContext } from '../plugin-context.js'
-import { verifyNoNetlifyForms } from '../verification.js'
+import { verifyNetlifyForms } from '../verification.js'
 
 const tracer = wrapTracer(trace.getTracer('Next runtime'))
 
@@ -32,7 +32,7 @@ export const copyStaticContent = async (ctx: PluginContext): Promise<void> => {
           .filter((path) => !paths.includes(`${path.slice(0, -5)}.json`))
           .map(async (path): Promise<void> => {
             const html = await readFile(join(srcDir, path), 'utf-8')
-            verifyNoNetlifyForms(ctx, html)
+            verifyNetlifyForms(ctx, html)
             await writeFile(join(destDir, await encodeBlobKey(path)), html, 'utf-8')
           }),
       )

src/build/verification.ts

@@ -1,14 +1,16 @@
 import { existsSync } from 'node:fs'
+import { readFile } from 'node:fs/promises'
 import { join } from 'node:path'
 
+import { glob } from 'fast-glob'
 import { satisfies } from 'semver'
 
 import { ApiRouteType, getAPIRoutesConfigs } from './advanced-api-routes.js'
 import type { PluginContext } from './plugin-context.js'
 
 const SUPPORTED_NEXT_VERSIONS = '>=13.5.0'
 
-const warnings = new Set<string>()
+const verifications = new Set<string>()
 
 export function verifyPublishDir(ctx: PluginContext) {
   if (!existsSync(ctx.publishDir)) {
@@ -55,7 +57,7 @@ export function verifyPublishDir(ctx: PluginContext) {
       !satisfies(ctx.nextVersion, SUPPORTED_NEXT_VERSIONS, { includePrerelease: true })
     ) {
       ctx.failBuild(
-        `@netlify/plugin-next@5 requires Next.js version ${SUPPORTED_NEXT_VERSIONS}, but found ${ctx.nextVersion}. Please upgrade your project's Next.js version.`,
+        `@netlify/plugin-nextjs@5 requires Next.js version ${SUPPORTED_NEXT_VERSIONS}, but found ${ctx.nextVersion}. Please upgrade your project's Next.js version.`,
       )
     }
   }
@@ -71,7 +73,7 @@ export function verifyPublishDir(ctx: PluginContext) {
   }
 }
 
-export async function verifyNoAdvancedAPIRoutes(ctx: PluginContext) {
+export async function verifyAdvancedAPIRoutes(ctx: PluginContext) {
   const apiRoutesConfigs = await getAPIRoutesConfigs(ctx)
 
   const unsupportedAPIRoutes = apiRoutesConfigs.filter((apiRouteConfig) => {
@@ -83,16 +85,41 @@ export async function verifyNoAdvancedAPIRoutes(ctx: PluginContext) {
 
   if (unsupportedAPIRoutes.length !== 0) {
     ctx.failBuild(
-      `@netlify/plugin-next@5 does not support advanced API routes. The following API routes should be migrated to Netlify background or scheduled functions:\n${unsupportedAPIRoutes.map((apiRouteConfig) => ` - ${apiRouteConfig.apiRoute} (type: "${apiRouteConfig.config.type}")`).join('\n')}\n\nRefer to https://ntl.fyi/next-scheduled-bg-function-migration as migration example.`,
+      `@netlify/plugin-nextjs@5 does not support advanced API routes. The following API routes should be migrated to Netlify background or scheduled functions:\n${unsupportedAPIRoutes.map((apiRouteConfig) => ` - ${apiRouteConfig.apiRoute} (type: "${apiRouteConfig.config.type}")`).join('\n')}\n\nRefer to https://ntl.fyi/next-scheduled-bg-function-migration as migration example.`,
     )
   }
 }
 
-export function verifyNoNetlifyForms(ctx: PluginContext, html: string) {
-  if (!warnings.has('netlifyForms') && /<form[^>]*?\s(netlify|data-netlify)[=>\s]/.test(html)) {
+const formDetectionRegex = /<form[^>]*?\s(netlify|data-netlify)[=>\s]/
+
+export async function verifyNetlifyFormsWorkaround(ctx: PluginContext) {
+  const srcDir = ctx.resolveFromSiteDir('public')
+  const paths = await glob('**/*.html', {
+    cwd: srcDir,
+    dot: true,
+  })
+  try {
+    for (const path of paths) {
+      const html = await readFile(join(srcDir, path), 'utf-8')
+      if (formDetectionRegex.test(html)) {
+        verifications.add('netlifyFormsWorkaround')
+        return
+      }
+    }
+  } catch (error) {
+    ctx.failBuild('Failed verifying public files', error)
+  }
+}
+
+export function verifyNetlifyForms(ctx: PluginContext, html: string) {
+  if (
+    !verifications.has('netlifyForms') &&
+    !verifications.has('netlifyFormsWorkaround') &&
+    formDetectionRegex.test(html)
+  ) {
     console.warn(
-      '@netlify/plugin-next@5 does not support Netlify Forms. Refer to https://ntl.fyi/next-runtime-forms-migration for migration example.',
+      '@netlify/plugin-nextjs@5 requires migration steps to support Netlify Forms. Refer to https://ntl.fyi/next-runtime-forms-migration for migration example.',
     )
-    warnings.add('netlifyForms')
+    verifications.add('netlifyForms')
   }
 }

src/index.ts

@@ -17,7 +17,11 @@ import { createEdgeHandlers } from './build/functions/edge.js'
 import { createServerHandler } from './build/functions/server.js'
 import { setImageConfig } from './build/image-cdn.js'
 import { PluginContext } from './build/plugin-context.js'
-import { verifyNoAdvancedAPIRoutes, verifyPublishDir } from './build/verification.js'
+import {
+  verifyAdvancedAPIRoutes,
+  verifyNetlifyFormsWorkaround,
+  verifyPublishDir,
+} from './build/verification.js'
 
 const tracer = wrapTracer(trace.getTracer('Next.js runtime'))
 
@@ -58,7 +62,8 @@ export const onBuild = async (options: NetlifyPluginOptions) => {
       return copyStaticExport(ctx)
     }
 
-    await verifyNoAdvancedAPIRoutes(ctx)
+    await verifyAdvancedAPIRoutes(ctx)
+    await verifyNetlifyFormsWorkaround(ctx)
 
     await Promise.all([
       copyStaticAssets(ctx),

tests/fixtures/netlify-forms-workaround/app/layout.js

@@ -0,0 +1,12 @@
+export const metadata = {
+  title: 'Netlify Forms',
+  description: 'Test for verifying Netlify Forms',
+}
+
+export default function RootLayout({ children }) {
+  return (
+    <html lang="en">
+      <body>{children}</body>
+    </html>
+  )
+}

tests/fixtures/netlify-forms-workaround/app/page.js

@@ -0,0 +1,7 @@
+export default function Page() {
+  return (
+    <form data-netlify="true">
+      <button type="submit">Send</button>
+    </form>
+  )
+}

tests/fixtures/netlify-forms-workaround/next.config.js

@@ -0,0 +1,10 @@
+/** @type {import('next').NextConfig} */
+const nextConfig = {
+  output: 'standalone',
+  eslint: {
+    ignoreDuringBuilds: true,
+  },
+  generateBuildId: () => 'build-id',
+}
+
+module.exports = nextConfig

tests/fixtures/netlify-forms-workaround/package.json

@@ -0,0 +1,19 @@
+{
+  "name": "netlify-forms",
+  "version": "0.1.0",
+  "private": true,
+  "scripts": {
+    "postinstall": "next build",
+    "dev": "next dev",
+    "build": "next build"
+  },
+  "dependencies": {
+    "@netlify/functions": "^2.7.0",
+    "next": "latest",
+    "react": "18.2.0",
+    "react-dom": "18.2.0"
+  },
+  "devDependencies": {
+    "@types/react": "18.2.75"
+  }
+}

tests/fixtures/netlify-forms-workaround/public/form.html

@@ -0,0 +1 @@
+<form name="contact" netlify></form>

tests/fixtures/netlify-forms-workaround/tsconfig.json

@@ -0,0 +1,23 @@
+{
+  "compilerOptions": {
+    "lib": ["dom", "dom.iterable", "esnext"],
+    "allowJs": true,
+    "skipLibCheck": true,
+    "strict": false,
+    "noEmit": true,
+    "incremental": true,
+    "esModuleInterop": true,
+    "module": "esnext",
+    "moduleResolution": "node",
+    "resolveJsonModule": true,
+    "isolatedModules": true,
+    "jsx": "preserve",
+    "plugins": [
+      {
+        "name": "next"
+      }
+    ]
+  },
+  "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"],
+  "exclude": ["node_modules"]
+}

tests/fixtures/netlify-forms/public/form.html

@@ -0,0 +1 @@
+<form></form>

tests/integration/advanced-api-routes.test.ts

@@ -26,7 +26,7 @@ it<FixtureTestContext>('test', async (ctx) => {
   const runPluginPromise = runPlugin(ctx)
 
   await expect(runPluginPromise).rejects.toThrow(
-    '@netlify/plugin-next@5 does not support advanced API routes. The following API routes should be migrated to Netlify background or scheduled functions:',
+    '@netlify/plugin-nextjs@5 does not support advanced API routes. The following API routes should be migrated to Netlify background or scheduled functions:',
   )
 
   // list API routes to migrate

tests/integration/netlify-forms.test.ts

@@ -14,19 +14,29 @@ beforeEach<FixtureTestContext>(async (ctx) => {
   vi.stubEnv('SITE_ID', ctx.siteID)
   vi.stubEnv('DEPLOY_ID', ctx.deployID)
   vi.stubEnv('NETLIFY_PURGE_API_TOKEN', 'fake-token')
-  // hide debug logs in tests
-  // vi.spyOn(console, 'debug').mockImplementation(() => {})
+  vi.resetModules()
 
   await startMockBlobStore(ctx)
 })
 
-// test skipped until we actually start failing builds - right now we are just showing a warning
-it.skip<FixtureTestContext>('should fail build when netlify forms are used', async (ctx) => {
+it<FixtureTestContext>('should warn when netlify forms are used', async (ctx) => {
+  const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+
   await createFixture('netlify-forms', ctx)
 
-  const runPluginPromise = runPlugin(ctx)
+  const runPluginPromise = await runPlugin(ctx)
 
-  await expect(runPluginPromise).rejects.toThrow(
-    '@netlify/plugin-next@5 does not support Netlify Forms',
+  expect(warn).toBeCalledWith(
+    '@netlify/plugin-nextjs@5 requires migration steps to support Netlify Forms. Refer to https://ntl.fyi/next-runtime-forms-migration for migration example.',
   )
 })
+
+it<FixtureTestContext>('should not warn when netlify forms are used with workaround', async (ctx) => {
+  const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+
+  await createFixture('netlify-forms-workaround', ctx)
+
+  const runPluginPromise = await runPlugin(ctx)
+
+  expect(warn).not.toBeCalled()
+})

tests/smoke/deploy.test.ts

@@ -1,4 +1,4 @@
-import { expect, test, describe, afterEach } from 'vitest'
+import { afterEach, describe, expect, test } from 'vitest'
 import { Fixture, fixtureFactories } from '../utils/create-e2e-fixture'
 
 const usedFixtures = new Set<Fixture>()
@@ -66,7 +66,7 @@ describe('version check', () => {
     async () => {
       await expect(selfCleaningFixtureFactories.next12_1_0()).rejects.toThrow(
         new RegExp(
-          `@netlify/plugin-next@5 requires Next.js version >=13.5.0, but found 12.1.0. Please upgrade your project's Next.js version.`,
+          `@netlify/plugin-nextjs@5 requires Next.js version >=13.5.0, but found 12.1.0. Please upgrade your project's Next.js version.`,
         ),
       )
     },
@@ -83,7 +83,7 @@ describe('version check', () => {
         selfCleaningFixtureFactories.yarnMonorepoMultipleNextVersionsSiteIncompatible(),
       ).rejects.toThrow(
         new RegExp(
-          `@netlify/plugin-next@5 requires Next.js version >=13.5.0, but found 13.4.1. Please upgrade your project's Next.js version.`,
+          `@netlify/plugin-nextjs@5 requires Next.js version >=13.5.0, but found 13.4.1. Please upgrade your project's Next.js version.`,
         ),
       )
     },
@@ -101,7 +101,7 @@ describe('version check', () => {
         fixtureFactories.npmNestedSiteMultipleNextVersionsIncompatible(),
       ).rejects.toThrow(
         new RegExp(
-          `@netlify/plugin-next@5 requires Next.js version >=13.5.0, but found 13.4.1. Please upgrade your project's Next.js version.`,
+          `@netlify/plugin-nextjs@5 requires Next.js version >=13.5.0, but found 13.4.1. Please upgrade your project's Next.js version.`,
         ),
       )
     },