From b1a8620b20e9a407f1c5234b81a15311fbd13b21 Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Thu, 4 Jul 2024 17:39:52 +0100 Subject: [PATCH] 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 --- src/build/content/prerendered.ts | 4 +- src/build/content/static.ts | 4 +- src/build/verification.ts | 43 +++++++++++++++---- src/index.ts | 9 +++- .../netlify-forms-workaround/app/layout.js | 12 ++++++ .../netlify-forms-workaround/app/page.js | 7 +++ .../netlify-forms-workaround/next.config.js | 10 +++++ .../netlify-forms-workaround/package.json | 19 ++++++++ .../netlify-forms-workaround/public/form.html | 1 + .../netlify-forms-workaround/tsconfig.json | 23 ++++++++++ tests/fixtures/netlify-forms/public/form.html | 1 + tests/integration/advanced-api-routes.test.ts | 2 +- tests/integration/netlify-forms.test.ts | 24 ++++++++--- tests/smoke/deploy.test.ts | 8 ++-- 14 files changed, 141 insertions(+), 26 deletions(-) create mode 100644 tests/fixtures/netlify-forms-workaround/app/layout.js create mode 100644 tests/fixtures/netlify-forms-workaround/app/page.js create mode 100644 tests/fixtures/netlify-forms-workaround/next.config.js create mode 100644 tests/fixtures/netlify-forms-workaround/package.json create mode 100644 tests/fixtures/netlify-forms-workaround/public/form.html create mode 100644 tests/fixtures/netlify-forms-workaround/tsconfig.json create mode 100644 tests/fixtures/netlify-forms/public/form.html diff --git a/src/build/content/prerendered.ts b/src/build/content/prerendered.ts index 61027a8f5a..9f1ae2fe75 100644 --- a/src/build/content/prerendered.ts +++ b/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 // 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) diff --git a/src/build/content/static.ts b/src/build/content/static.ts index d5b418b438..4079695bd4 100644 --- a/src/build/content/static.ts +++ b/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 => { .filter((path) => !paths.includes(`${path.slice(0, -5)}.json`)) .map(async (path): Promise => { 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') }), ) diff --git a/src/build/verification.ts b/src/build/verification.ts index c6bcf929ef..a4089349d5 100644 --- a/src/build/verification.ts +++ b/src/build/verification.ts @@ -1,6 +1,8 @@ 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' @@ -8,7 +10,7 @@ import type { PluginContext } from './plugin-context.js' const SUPPORTED_NEXT_VERSIONS = '>=13.5.0' -const warnings = new Set() +const verifications = new Set() 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') && /]*?\s(netlify|data-netlify)[=>\s]/.test(html)) { +const formDetectionRegex = /]*?\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') } } diff --git a/src/index.ts b/src/index.ts index 729b264b43..5e62ab6b47 100644 --- a/src/index.ts +++ b/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), diff --git a/tests/fixtures/netlify-forms-workaround/app/layout.js b/tests/fixtures/netlify-forms-workaround/app/layout.js new file mode 100644 index 0000000000..60c789ba7a --- /dev/null +++ b/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 ( + + {children} + + ) +} diff --git a/tests/fixtures/netlify-forms-workaround/app/page.js b/tests/fixtures/netlify-forms-workaround/app/page.js new file mode 100644 index 0000000000..591550885b --- /dev/null +++ b/tests/fixtures/netlify-forms-workaround/app/page.js @@ -0,0 +1,7 @@ +export default function Page() { + return ( +
+ +
+ ) +} diff --git a/tests/fixtures/netlify-forms-workaround/next.config.js b/tests/fixtures/netlify-forms-workaround/next.config.js new file mode 100644 index 0000000000..6346ab0742 --- /dev/null +++ b/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 diff --git a/tests/fixtures/netlify-forms-workaround/package.json b/tests/fixtures/netlify-forms-workaround/package.json new file mode 100644 index 0000000000..a95e5e097d --- /dev/null +++ b/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" + } +} diff --git a/tests/fixtures/netlify-forms-workaround/public/form.html b/tests/fixtures/netlify-forms-workaround/public/form.html new file mode 100644 index 0000000000..a4af5bd458 --- /dev/null +++ b/tests/fixtures/netlify-forms-workaround/public/form.html @@ -0,0 +1 @@ +
diff --git a/tests/fixtures/netlify-forms-workaround/tsconfig.json b/tests/fixtures/netlify-forms-workaround/tsconfig.json new file mode 100644 index 0000000000..482806989f --- /dev/null +++ b/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"] +} diff --git a/tests/fixtures/netlify-forms/public/form.html b/tests/fixtures/netlify-forms/public/form.html new file mode 100644 index 0000000000..cddb6e3794 --- /dev/null +++ b/tests/fixtures/netlify-forms/public/form.html @@ -0,0 +1 @@ +
diff --git a/tests/integration/advanced-api-routes.test.ts b/tests/integration/advanced-api-routes.test.ts index 41ad420936..55412df622 100644 --- a/tests/integration/advanced-api-routes.test.ts +++ b/tests/integration/advanced-api-routes.test.ts @@ -26,7 +26,7 @@ it('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 diff --git a/tests/integration/netlify-forms.test.ts b/tests/integration/netlify-forms.test.ts index 02073f955a..728c377d74 100644 --- a/tests/integration/netlify-forms.test.ts +++ b/tests/integration/netlify-forms.test.ts @@ -14,19 +14,29 @@ beforeEach(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('should fail build when netlify forms are used', async (ctx) => { +it('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('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() +}) diff --git a/tests/smoke/deploy.test.ts b/tests/smoke/deploy.test.ts index 331470784c..c60a37b808 100644 --- a/tests/smoke/deploy.test.ts +++ b/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() @@ -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.`, ), ) },