Skip to content

Commit b1a8620

Browse files
orinokaiorinokai
authored
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
1 parent 233fc2f commit b1a8620

File tree

14 files changed

+141
-26
lines changed

14 files changed

+141
-26
lines changed

‎src/build/content/prerendered.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type {
1818
NetlifyIncrementalCacheValue,
1919
} from '../../shared/cache-types.cjs'
2020
import type { PluginContext } from '../plugin-context.js'
21-
import { verifyNoNetlifyForms } from '../verification.js'
21+
import { verifyNetlifyForms } from '../verification.js'
2222

2323
const tracer = wrapTracer(trace.getTracer('Next runtime'))
2424

@@ -172,7 +172,7 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise<void>
172172

173173
// Netlify Forms are not support and require a workaround
174174
if (value.kind === 'PAGE' || value.kind === 'APP_PAGE') {
175-
verifyNoNetlifyForms(ctx, value.html)
175+
verifyNetlifyForms(ctx, value.html)
176176
}
177177

178178
await writeCacheEntry(key, value, lastModified, ctx)

‎src/build/content/static.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import glob from 'fast-glob'
88

99
import { encodeBlobKey } from '../../shared/blobkey.js'
1010
import { PluginContext } from '../plugin-context.js'
11-
import { verifyNoNetlifyForms } from '../verification.js'
11+
import { verifyNetlifyForms } from '../verification.js'
1212

1313
const tracer = wrapTracer(trace.getTracer('Next runtime'))
1414

@@ -32,7 +32,7 @@ export const copyStaticContent = async (ctx: PluginContext): Promise<void> => {
3232
.filter((path) => !paths.includes(`${path.slice(0, -5)}.json`))
3333
.map(async (path): Promise<void> => {
3434
const html = await readFile(join(srcDir, path), 'utf-8')
35-
verifyNoNetlifyForms(ctx, html)
35+
verifyNetlifyForms(ctx, html)
3636
await writeFile(join(destDir, await encodeBlobKey(path)), html, 'utf-8')
3737
}),
3838
)

‎src/build/verification.ts

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
import { existsSync } from 'node:fs'
2+
import { readFile } from 'node:fs/promises'
23
import { join } from 'node:path'
34

5+
import { glob } from 'fast-glob'
46
import { satisfies } from 'semver'
57

68
import { ApiRouteType, getAPIRoutesConfigs } from './advanced-api-routes.js'
79
import type { PluginContext } from './plugin-context.js'
810

911
const SUPPORTED_NEXT_VERSIONS = '>=13.5.0'
1012

11-
const warnings = new Set<string>()
13+
const verifications = new Set<string>()
1214

1315
export function verifyPublishDir(ctx: PluginContext) {
1416
if (!existsSync(ctx.publishDir)) {
@@ -55,7 +57,7 @@ export function verifyPublishDir(ctx: PluginContext) {
5557
!satisfies(ctx.nextVersion, SUPPORTED_NEXT_VERSIONS, { includePrerelease: true })
5658
) {
5759
ctx.failBuild(
58-
`@netlify/plugin-next@5 requires Next.js version ${SUPPORTED_NEXT_VERSIONS}, but found ${ctx.nextVersion}. Please upgrade your project's Next.js version.`,
60+
`@netlify/plugin-nextjs@5 requires Next.js version ${SUPPORTED_NEXT_VERSIONS}, but found ${ctx.nextVersion}. Please upgrade your project's Next.js version.`,
5961
)
6062
}
6163
}
@@ -71,7 +73,7 @@ export function verifyPublishDir(ctx: PluginContext) {
7173
}
7274
}
7375

74-
export async function verifyNoAdvancedAPIRoutes(ctx: PluginContext) {
76+
export async function verifyAdvancedAPIRoutes(ctx: PluginContext) {
7577
const apiRoutesConfigs = await getAPIRoutesConfigs(ctx)
7678

7779
const unsupportedAPIRoutes = apiRoutesConfigs.filter((apiRouteConfig) => {
@@ -83,16 +85,41 @@ export async function verifyNoAdvancedAPIRoutes(ctx: PluginContext) {
8385

8486
if (unsupportedAPIRoutes.length !== 0) {
8587
ctx.failBuild(
86-
`@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.`,
88+
`@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.`,
8789
)
8890
}
8991
}
9092

91-
export function verifyNoNetlifyForms(ctx: PluginContext, html: string) {
92-
if (!warnings.has('netlifyForms') && /<form[^>]*?\s(netlify|data-netlify)[=>\s]/.test(html)) {
93+
const formDetectionRegex = /<form[^>]*?\s(netlify|data-netlify)[=>\s]/
94+
95+
export async function verifyNetlifyFormsWorkaround(ctx: PluginContext) {
96+
const srcDir = ctx.resolveFromSiteDir('public')
97+
const paths = await glob('**/*.html', {
98+
cwd: srcDir,
99+
dot: true,
100+
})
101+
try {
102+
for (const path of paths) {
103+
const html = await readFile(join(srcDir, path), 'utf-8')
104+
if (formDetectionRegex.test(html)) {
105+
verifications.add('netlifyFormsWorkaround')
106+
return
107+
}
108+
}
109+
} catch (error) {
110+
ctx.failBuild('Failed verifying public files', error)
111+
}
112+
}
113+
114+
export function verifyNetlifyForms(ctx: PluginContext, html: string) {
115+
if (
116+
!verifications.has('netlifyForms') &&
117+
!verifications.has('netlifyFormsWorkaround') &&
118+
formDetectionRegex.test(html)
119+
) {
93120
console.warn(
94-
'@netlify/plugin-next@5 does not support Netlify Forms. Refer to https://ntl.fyi/next-runtime-forms-migration for migration example.',
121+
'@netlify/plugin-nextjs@5 requires migration steps to support Netlify Forms. Refer to https://ntl.fyi/next-runtime-forms-migration for migration example.',
95122
)
96-
warnings.add('netlifyForms')
123+
verifications.add('netlifyForms')
97124
}
98125
}

‎src/index.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ import { createEdgeHandlers } from './build/functions/edge.js'
1717
import { createServerHandler } from './build/functions/server.js'
1818
import { setImageConfig } from './build/image-cdn.js'
1919
import { PluginContext } from './build/plugin-context.js'
20-
import { verifyNoAdvancedAPIRoutes, verifyPublishDir } from './build/verification.js'
20+
import {
21+
verifyAdvancedAPIRoutes,
22+
verifyNetlifyFormsWorkaround,
23+
verifyPublishDir,
24+
} from './build/verification.js'
2125

2226
const tracer = wrapTracer(trace.getTracer('Next.js runtime'))
2327

@@ -58,7 +62,8 @@ export const onBuild = async (options: NetlifyPluginOptions) => {
5862
return copyStaticExport(ctx)
5963
}
6064

61-
await verifyNoAdvancedAPIRoutes(ctx)
65+
await verifyAdvancedAPIRoutes(ctx)
66+
await verifyNetlifyFormsWorkaround(ctx)
6267

6368
await Promise.all([
6469
copyStaticAssets(ctx),

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export const metadata = {
2+
title: 'Netlify Forms',
3+
description: 'Test for verifying Netlify Forms',
4+
}
5+
6+
export default function RootLayout({ children }) {
7+
return (
8+
<html lang="en">
9+
<body>{children}</body>
10+
</html>
11+
)
12+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function Page() {
2+
return (
3+
<form data-netlify="true">
4+
<button type="submit">Send</button>
5+
</form>
6+
)
7+
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/** @type {import('next').NextConfig} */
2+
const nextConfig = {
3+
output: 'standalone',
4+
eslint: {
5+
ignoreDuringBuilds: true,
6+
},
7+
generateBuildId: () => 'build-id',
8+
}
9+
10+
module.exports = nextConfig

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"name": "netlify-forms",
3+
"version": "0.1.0",
4+
"private": true,
5+
"scripts": {
6+
"postinstall": "next build",
7+
"dev": "next dev",
8+
"build": "next build"
9+
},
10+
"dependencies": {
11+
"@netlify/functions": "^2.7.0",
12+
"next": "latest",
13+
"react": "18.2.0",
14+
"react-dom": "18.2.0"
15+
},
16+
"devDependencies": {
17+
"@types/react": "18.2.75"
18+
}
19+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<form name="contact" netlify></form>

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"compilerOptions": {
3+
"lib": ["dom", "dom.iterable", "esnext"],
4+
"allowJs": true,
5+
"skipLibCheck": true,
6+
"strict": false,
7+
"noEmit": true,
8+
"incremental": true,
9+
"esModuleInterop": true,
10+
"module": "esnext",
11+
"moduleResolution": "node",
12+
"resolveJsonModule": true,
13+
"isolatedModules": true,
14+
"jsx": "preserve",
15+
"plugins": [
16+
{
17+
"name": "next"
18+
}
19+
]
20+
},
21+
"include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"],
22+
"exclude": ["node_modules"]
23+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<form></form>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ it<FixtureTestContext>('test', async (ctx) => {
2626
const runPluginPromise = runPlugin(ctx)
2727

2828
await expect(runPluginPromise).rejects.toThrow(
29-
'@netlify/plugin-next@5 does not support advanced API routes. The following API routes should be migrated to Netlify background or scheduled functions:',
29+
'@netlify/plugin-nextjs@5 does not support advanced API routes. The following API routes should be migrated to Netlify background or scheduled functions:',
3030
)
3131

3232
// list API routes to migrate

‎tests/integration/netlify-forms.test.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,29 @@ beforeEach<FixtureTestContext>(async (ctx) => {
1414
vi.stubEnv('SITE_ID', ctx.siteID)
1515
vi.stubEnv('DEPLOY_ID', ctx.deployID)
1616
vi.stubEnv('NETLIFY_PURGE_API_TOKEN', 'fake-token')
17-
// hide debug logs in tests
18-
// vi.spyOn(console, 'debug').mockImplementation(() => {})
17+
vi.resetModules()
1918

2019
await startMockBlobStore(ctx)
2120
})
2221

23-
// test skipped until we actually start failing builds - right now we are just showing a warning
24-
it.skip<FixtureTestContext>('should fail build when netlify forms are used', async (ctx) => {
22+
it<FixtureTestContext>('should warn when netlify forms are used', async (ctx) => {
23+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
24+
2525
await createFixture('netlify-forms', ctx)
2626

27-
const runPluginPromise = runPlugin(ctx)
27+
const runPluginPromise = await runPlugin(ctx)
2828

29-
await expect(runPluginPromise).rejects.toThrow(
30-
'@netlify/plugin-next@5 does not support Netlify Forms',
29+
expect(warn).toBeCalledWith(
30+
'@netlify/plugin-nextjs@5 requires migration steps to support Netlify Forms. Refer to https://ntl.fyi/next-runtime-forms-migration for migration example.',
3131
)
3232
})
33+
34+
it<FixtureTestContext>('should not warn when netlify forms are used with workaround', async (ctx) => {
35+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
36+
37+
await createFixture('netlify-forms-workaround', ctx)
38+
39+
const runPluginPromise = await runPlugin(ctx)
40+
41+
expect(warn).not.toBeCalled()
42+
})

‎tests/smoke/deploy.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { expect, test, describe, afterEach } from 'vitest'
1+
import { afterEach, describe, expect, test } from 'vitest'
22
import { Fixture, fixtureFactories } from '../utils/create-e2e-fixture'
33

44
const usedFixtures = new Set<Fixture>()
@@ -66,7 +66,7 @@ describe('version check', () => {
6666
async () => {
6767
await expect(selfCleaningFixtureFactories.next12_1_0()).rejects.toThrow(
6868
new RegExp(
69-
`@netlify/plugin-next@5 requires Next.js version >=13.5.0, but found 12.1.0. Please upgrade your project's Next.js version.`,
69+
`@netlify/plugin-nextjs@5 requires Next.js version >=13.5.0, but found 12.1.0. Please upgrade your project's Next.js version.`,
7070
),
7171
)
7272
},
@@ -83,7 +83,7 @@ describe('version check', () => {
8383
selfCleaningFixtureFactories.yarnMonorepoMultipleNextVersionsSiteIncompatible(),
8484
).rejects.toThrow(
8585
new RegExp(
86-
`@netlify/plugin-next@5 requires Next.js version >=13.5.0, but found 13.4.1. Please upgrade your project's Next.js version.`,
86+
`@netlify/plugin-nextjs@5 requires Next.js version >=13.5.0, but found 13.4.1. Please upgrade your project's Next.js version.`,
8787
),
8888
)
8989
},
@@ -101,7 +101,7 @@ describe('version check', () => {
101101
fixtureFactories.npmNestedSiteMultipleNextVersionsIncompatible(),
102102
).rejects.toThrow(
103103
new RegExp(
104-
`@netlify/plugin-next@5 requires Next.js version >=13.5.0, but found 13.4.1. Please upgrade your project's Next.js version.`,
104+
`@netlify/plugin-nextjs@5 requires Next.js version >=13.5.0, but found 13.4.1. Please upgrade your project's Next.js version.`,
105105
),
106106
)
107107
},

0 commit comments

Comments
 (0)