From bbccdf791b890ede017b346d6a8e97aea182564d Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Wed, 26 Jun 2024 13:03:20 +0100 Subject: [PATCH 1/8] feat: verify no netlify forms --- src/build/content/prerendered.ts | 6 ++++++ src/build/verification.ts | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/src/build/content/prerendered.ts b/src/build/content/prerendered.ts index 727044b69c..872413b11f 100644 --- a/src/build/content/prerendered.ts +++ b/src/build/content/prerendered.ts @@ -18,6 +18,7 @@ import type { NetlifyIncrementalCacheValue, } from '../../shared/cache-types.cjs' import type { PluginContext } from '../plugin-context.js' +import { verifyNoNetlifyForms } from '../verification.js' const tracer = wrapTracer(trace.getTracer('Next runtime')) @@ -169,6 +170,11 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise throw new Error(`Unrecognized content: ${route}`) } + // Netlify Forms are not support and require a workaround + if (value.kind === 'PAGE') { + verifyNoNetlifyForms(ctx, value.html) + } + await writeCacheEntry(key, value, lastModified, ctx) }), ), diff --git a/src/build/verification.ts b/src/build/verification.ts index 6589cb67c1..7dd2d42934 100644 --- a/src/build/verification.ts +++ b/src/build/verification.ts @@ -85,3 +85,11 @@ export async function verifyNoAdvancedAPIRoutes(ctx: PluginContext) { ) } } + +export function verifyNoNetlifyForms(ctx: PluginContext, html: string) { + if (/]*?\s(netlify|data-netlify)[=>\s]/.test(html)) { + ctx.failBuild( + `@netlify/plugin-next@5 does not support Netlify Forms. Refer to https://ntl.fyi/next-runtime-forms-migration for migration example.`, + ) + } +} From 39511ed952d7a9f720bdb140fd32c15a353451ba Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Wed, 26 Jun 2024 13:03:54 +0100 Subject: [PATCH 2/8] test: ensure build is failed when netlify forms detected --- tests/fixtures/netlify-forms/app/layout.js | 12 +++++++ tests/fixtures/netlify-forms/app/page.js | 7 ++++ tests/fixtures/netlify-forms/next-env.d.ts | 5 +++ tests/fixtures/netlify-forms/next.config.js | 10 ++++++ tests/fixtures/netlify-forms/package.json | 19 +++++++++++ tests/fixtures/netlify-forms/tsconfig.json | 34 +++++++++++++++++++ tests/integration/advanced-api-routes.test.ts | 5 +-- tests/integration/netlify-forms.test.ts | 31 +++++++++++++++++ 8 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/netlify-forms/app/layout.js create mode 100644 tests/fixtures/netlify-forms/app/page.js create mode 100644 tests/fixtures/netlify-forms/next-env.d.ts create mode 100644 tests/fixtures/netlify-forms/next.config.js create mode 100644 tests/fixtures/netlify-forms/package.json create mode 100644 tests/fixtures/netlify-forms/tsconfig.json create mode 100644 tests/integration/netlify-forms.test.ts diff --git a/tests/fixtures/netlify-forms/app/layout.js b/tests/fixtures/netlify-forms/app/layout.js new file mode 100644 index 0000000000..60c789ba7a --- /dev/null +++ b/tests/fixtures/netlify-forms/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/app/page.js b/tests/fixtures/netlify-forms/app/page.js new file mode 100644 index 0000000000..591550885b --- /dev/null +++ b/tests/fixtures/netlify-forms/app/page.js @@ -0,0 +1,7 @@ +export default function Page() { + return ( +
+ +
+ ) +} diff --git a/tests/fixtures/netlify-forms/next-env.d.ts b/tests/fixtures/netlify-forms/next-env.d.ts new file mode 100644 index 0000000000..4f11a03dc6 --- /dev/null +++ b/tests/fixtures/netlify-forms/next-env.d.ts @@ -0,0 +1,5 @@ +/// +/// + +// NOTE: This file should not be edited +// see https://nextjs.org/docs/basic-features/typescript for more information. diff --git a/tests/fixtures/netlify-forms/next.config.js b/tests/fixtures/netlify-forms/next.config.js new file mode 100644 index 0000000000..6346ab0742 --- /dev/null +++ b/tests/fixtures/netlify-forms/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/package.json b/tests/fixtures/netlify-forms/package.json new file mode 100644 index 0000000000..a95e5e097d --- /dev/null +++ b/tests/fixtures/netlify-forms/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/tsconfig.json b/tests/fixtures/netlify-forms/tsconfig.json new file mode 100644 index 0000000000..1be3b6a618 --- /dev/null +++ b/tests/fixtures/netlify-forms/tsconfig.json @@ -0,0 +1,34 @@ +{ + "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/integration/advanced-api-routes.test.ts b/tests/integration/advanced-api-routes.test.ts index 7eb66199b0..41ad420936 100644 --- a/tests/integration/advanced-api-routes.test.ts +++ b/tests/integration/advanced-api-routes.test.ts @@ -1,7 +1,8 @@ import { getLogger } from 'lambda-local' import { v4 } from 'uuid' -import { beforeEach, vi, it, expect } from 'vitest' -import { createFixture, runPlugin, type FixtureTestContext } from '../utils/fixture.js' +import { beforeEach, expect, it, vi } from 'vitest' +import { type FixtureTestContext } from '../utils/contexts.js' +import { createFixture, runPlugin } from '../utils/fixture.js' import { generateRandomObjectID, startMockBlobStore } from '../utils/helpers.js' getLogger().level = 'alert' diff --git a/tests/integration/netlify-forms.test.ts b/tests/integration/netlify-forms.test.ts new file mode 100644 index 0000000000..daca7c624e --- /dev/null +++ b/tests/integration/netlify-forms.test.ts @@ -0,0 +1,31 @@ +import { getLogger } from 'lambda-local' +import { v4 } from 'uuid' +import { beforeEach, expect, it, vi } from 'vitest' +import { type FixtureTestContext } from '../utils/contexts.js' +import { createFixture, runPlugin } from '../utils/fixture.js' +import { generateRandomObjectID, startMockBlobStore } from '../utils/helpers.js' + +getLogger().level = 'alert' + +beforeEach(async (ctx) => { + // set for each test a new deployID and siteID + ctx.deployID = generateRandomObjectID() + ctx.siteID = v4() + 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(() => {}) + + await startMockBlobStore(ctx) +}) + +it('test', async (ctx) => { + await createFixture('netlify-forms', ctx) + + const runPluginPromise = runPlugin(ctx) + + await expect(runPluginPromise).rejects.toThrow( + '@netlify/plugin-next@5 does not support Netlify Forms', + ) +}) From 0ab3d7d5eea8eb65ac94e44f72b231a07cf4e574 Mon Sep 17 00:00:00 2001 From: orinokai Date: Wed, 26 Jun 2024 12:12:28 +0000 Subject: [PATCH 3/8] chore: format with prettier --- tests/fixtures/netlify-forms/tsconfig.json | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/tests/fixtures/netlify-forms/tsconfig.json b/tests/fixtures/netlify-forms/tsconfig.json index 1be3b6a618..482806989f 100644 --- a/tests/fixtures/netlify-forms/tsconfig.json +++ b/tests/fixtures/netlify-forms/tsconfig.json @@ -1,10 +1,6 @@ { "compilerOptions": { - "lib": [ - "dom", - "dom.iterable", - "esnext" - ], + "lib": ["dom", "dom.iterable", "esnext"], "allowJs": true, "skipLibCheck": true, "strict": false, @@ -22,13 +18,6 @@ } ] }, - "include": [ - "next-env.d.ts", - "**/*.ts", - "**/*.tsx", - ".next/types/**/*.ts" - ], - "exclude": [ - "node_modules" - ] + "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"], + "exclude": ["node_modules"] } From 0d665332d2a631de5fe58279a96ecc0ca66a4248 Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Thu, 27 Jun 2024 14:19:31 +0100 Subject: [PATCH 4/8] feat: make verification passive by outputing warning --- src/build/verification.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/build/verification.ts b/src/build/verification.ts index 7dd2d42934..c6bcf929ef 100644 --- a/src/build/verification.ts +++ b/src/build/verification.ts @@ -8,6 +8,8 @@ import type { PluginContext } from './plugin-context.js' const SUPPORTED_NEXT_VERSIONS = '>=13.5.0' +const warnings = new Set() + export function verifyPublishDir(ctx: PluginContext) { if (!existsSync(ctx.publishDir)) { ctx.failBuild( @@ -87,9 +89,10 @@ export async function verifyNoAdvancedAPIRoutes(ctx: PluginContext) { } export function verifyNoNetlifyForms(ctx: PluginContext, html: string) { - if (/]*?\s(netlify|data-netlify)[=>\s]/.test(html)) { - ctx.failBuild( - `@netlify/plugin-next@5 does not support Netlify Forms. Refer to https://ntl.fyi/next-runtime-forms-migration for migration example.`, + if (!warnings.has('netlifyForms') && /]*?\s(netlify|data-netlify)[=>\s]/.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.', ) + warnings.add('netlifyForms') } } From 9a500d27a22d89fe5afee901b6e4f9fd529a176e Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Thu, 27 Jun 2024 14:19:53 +0100 Subject: [PATCH 5/8] feat: include static content for forms verification --- src/build/content/static.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/build/content/static.ts b/src/build/content/static.ts index 7733c318ef..d5b418b438 100644 --- a/src/build/content/static.ts +++ b/src/build/content/static.ts @@ -1,5 +1,5 @@ import { existsSync } from 'node:fs' -import { cp, mkdir, rename, rm } from 'node:fs/promises' +import { cp, mkdir, readFile, rename, rm, writeFile } from 'node:fs/promises' import { basename, join } from 'node:path' import { trace } from '@opentelemetry/api' @@ -8,6 +8,7 @@ import glob from 'fast-glob' import { encodeBlobKey } from '../../shared/blobkey.js' import { PluginContext } from '../plugin-context.js' +import { verifyNoNetlifyForms } from '../verification.js' const tracer = wrapTracer(trace.getTracer('Next runtime')) @@ -25,14 +26,14 @@ export const copyStaticContent = async (ctx: PluginContext): Promise => { }) try { + await mkdir(destDir, { recursive: true }) await Promise.all( paths .filter((path) => !paths.includes(`${path.slice(0, -5)}.json`)) .map(async (path): Promise => { - await cp(join(srcDir, path), join(destDir, await encodeBlobKey(path)), { - recursive: true, - force: true, - }) + const html = await readFile(join(srcDir, path), 'utf-8') + verifyNoNetlifyForms(ctx, html) + await writeFile(join(destDir, await encodeBlobKey(path)), html, 'utf-8') }), ) } catch (error) { From ccd1351d1282ce89041c3f91f0334af78fdf7e78 Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Thu, 27 Jun 2024 14:20:17 +0100 Subject: [PATCH 6/8] test: skip forms verification test until we are failing the build --- tests/integration/netlify-forms.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/netlify-forms.test.ts b/tests/integration/netlify-forms.test.ts index daca7c624e..e134e5cb0d 100644 --- a/tests/integration/netlify-forms.test.ts +++ b/tests/integration/netlify-forms.test.ts @@ -20,7 +20,7 @@ beforeEach(async (ctx) => { await startMockBlobStore(ctx) }) -it('test', async (ctx) => { +it.skip('test', async (ctx) => { await createFixture('netlify-forms', ctx) const runPluginPromise = runPlugin(ctx) From faed5fafe6b564fdcf97300e97e971b4d1a59bad Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Thu, 27 Jun 2024 14:34:55 +0100 Subject: [PATCH 7/8] feat: also verify no netlify forms for APP_PAGE --- src/build/content/prerendered.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/build/content/prerendered.ts b/src/build/content/prerendered.ts index 872413b11f..61027a8f5a 100644 --- a/src/build/content/prerendered.ts +++ b/src/build/content/prerendered.ts @@ -171,7 +171,7 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise } // Netlify Forms are not support and require a workaround - if (value.kind === 'PAGE') { + if (value.kind === 'PAGE' || value.kind === 'APP_PAGE') { verifyNoNetlifyForms(ctx, value.html) } From eee0e833a71e33f5e56c174c128846ad2b1af124 Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Thu, 27 Jun 2024 15:54:49 +0100 Subject: [PATCH 8/8] Update tests/integration/netlify-forms.test.ts Co-authored-by: Michal Piechowiak --- tests/integration/netlify-forms.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/netlify-forms.test.ts b/tests/integration/netlify-forms.test.ts index e134e5cb0d..02073f955a 100644 --- a/tests/integration/netlify-forms.test.ts +++ b/tests/integration/netlify-forms.test.ts @@ -20,7 +20,8 @@ beforeEach(async (ctx) => { await startMockBlobStore(ctx) }) -it.skip('test', async (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) => { await createFixture('netlify-forms', ctx) const runPluginPromise = runPlugin(ctx)