Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't delete functions generated by other build plugins #2511

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 49 additions & 6 deletions src/build/functions/edge.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { cp, mkdir, readFile, rm, writeFile } from 'node:fs/promises'
import { dirname, join } from 'node:path'
import { cp, mkdir, readdir, readFile, rm, writeFile } from 'node:fs/promises'
import { dirname, join, parse as parsePath } from 'node:path'

import type { Manifest, ManifestFunction } from '@netlify/edge-functions'
import { glob } from 'fast-glob'
@@ -8,9 +8,21 @@ import { pathToRegexp } from 'path-to-regexp'

import { EDGE_HANDLER_NAME, PluginContext } from '../plugin-context.js'

type ManifestFunctionWithGenerator = ManifestFunction & { generator?: string }

const getEdgeManifestPath = (ctx: PluginContext) => join(ctx.edgeFunctionsDir, 'manifest.json')

const writeEdgeManifest = async (ctx: PluginContext, manifest: Manifest) => {
await mkdir(ctx.edgeFunctionsDir, { recursive: true })
await writeFile(join(ctx.edgeFunctionsDir, 'manifest.json'), JSON.stringify(manifest, null, 2))
await writeFile(getEdgeManifestPath(ctx), JSON.stringify(manifest, null, 2))
}

const readEdgeManifest = async (ctx: PluginContext) => {
try {
return JSON.parse(await readFile(getEdgeManifestPath(ctx), 'utf-8')) as Manifest
} catch {
return null
}
}

const copyRuntime = async (ctx: PluginContext, handlerDirectory: string): Promise<void> => {
@@ -145,7 +157,7 @@ const getHandlerName = ({ name }: Pick<NextDefinition, 'name'>): string =>
const buildHandlerDefinition = (
ctx: PluginContext,
{ name, matchers, page }: NextDefinition,
): Array<ManifestFunction> => {
): Array<ManifestFunctionWithGenerator> => {
const fun = getHandlerName({ name })
const funName = name.endsWith('middleware')
? 'Next.js Middleware Handler'
@@ -162,8 +174,39 @@ const buildHandlerDefinition = (
}))
}

const clearStaleEdgeHandlers = async (ctx: PluginContext) => {
const previousManifest = await readEdgeManifest(ctx)
if (!previousManifest) {
return []
}

const uniqueNextRuntimeFunctions = new Set<string>()
const nonNextRuntimeFunctions: ManifestFunctionWithGenerator[] = []

for (const fn of previousManifest.functions as ManifestFunctionWithGenerator[]) {
if (fn?.generator?.startsWith(ctx.pluginName)) {
uniqueNextRuntimeFunctions.add(fn.function)
} else {
nonNextRuntimeFunctions.push(fn)
}
}

if (uniqueNextRuntimeFunctions.size === 0) {
return nonNextRuntimeFunctions
}

for (const fileOrDir of await readdir(ctx.edgeFunctionsDir, { withFileTypes: true })) {
const nameWithoutExtension = parsePath(fileOrDir.name).name

if (uniqueNextRuntimeFunctions.has(nameWithoutExtension)) {
await rm(join(ctx.edgeFunctionsDir, fileOrDir.name), { recursive: true, force: true })
}
}
return nonNextRuntimeFunctions
}

export const createEdgeHandlers = async (ctx: PluginContext) => {
await rm(ctx.edgeFunctionsDir, { recursive: true, force: true })
const nonNextRuntimeFunctions = await clearStaleEdgeHandlers(ctx)

const nextManifest = await ctx.getMiddlewareManifest()
const nextDefinitions = [
@@ -175,7 +218,7 @@ export const createEdgeHandlers = async (ctx: PluginContext) => {
const netlifyDefinitions = nextDefinitions.flatMap((def) => buildHandlerDefinition(ctx, def))
const netlifyManifest: Manifest = {
version: 1,
functions: netlifyDefinitions,
functions: [...nonNextRuntimeFunctions, ...netlifyDefinitions],
}
await writeEdgeManifest(ctx, netlifyManifest)
}
46 changes: 43 additions & 3 deletions src/build/functions/server.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { cp, mkdir, readFile, rm, writeFile } from 'node:fs/promises'
import { join, relative } from 'node:path'
import { cp, mkdir, readdir, readFile, rm, writeFile } from 'node:fs/promises'
import { join, parse as parsePath, relative } from 'node:path'
import { join as posixJoin } from 'node:path/posix'

import { trace } from '@opentelemetry/api'
@@ -127,12 +127,52 @@ const writeHandlerFile = async (ctx: PluginContext) => {
await writeFile(join(ctx.serverHandlerRootDir, `${SERVER_HANDLER_NAME}.mjs`), handler)
}

const clearStaleServerHandlers = async (ctx: PluginContext) => {
const potentialServerlessFunctionConfigFiles = await glob('**/*.json', {
deep: 2,
cwd: ctx.serverFunctionsDir,
})

const toRemove = new Set<string>()

for (const potentialServerlessFunctionConfigFile of potentialServerlessFunctionConfigFiles) {
try {
const functionConfig = JSON.parse(
await readFile(
join(ctx.serverFunctionsDir, potentialServerlessFunctionConfigFile),
'utf-8',
),
)

if (functionConfig?.config?.generator?.startsWith(ctx.pluginName)) {
const parsedPath = parsePath(potentialServerlessFunctionConfigFile)

toRemove.add(parsedPath.dir || parsedPath.name)
}
} catch {
// this might be malformatted json or json that doesn't represent function configuration
// so we just skip it in case of errors
}
}

if (toRemove.size === 0) {
return
}

for (const fileOrDir of await readdir(ctx.serverFunctionsDir, { withFileTypes: true })) {
const nameWithoutExtension = parsePath(fileOrDir.name).name

if (toRemove.has(nameWithoutExtension)) {
await rm(join(ctx.serverFunctionsDir, fileOrDir.name), { recursive: true, force: true })
}
}
}
/**
* Create a Netlify function to run the Next.js server
*/
export const createServerHandler = async (ctx: PluginContext) => {
await tracer.withActiveSpan('createServerHandler', async () => {
await rm(ctx.serverFunctionsDir, { recursive: true, force: true })
await clearStaleServerHandlers(ctx)
await mkdir(join(ctx.serverHandlerDir, '.netlify'), { recursive: true })

await copyNextServerCode(ctx)
56 changes: 56 additions & 0 deletions tests/e2e/with-integrations.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { expect } from '@playwright/test'
import { test } from '../utils/playwright-helpers.js'

test('Renders the Home page correctly', async ({ page, withIntegrations }) => {
await page.goto(withIntegrations.url)

expect(page.locator('body')).toHaveText('Hello World')
})

test.describe('Should clear stale functions produced by previous builds by @netlify/plugin-nextjs', () => {
test('Serverless functions', async ({ page, withIntegrations }) => {
const response1 = await page.goto(new URL('/test/serverless/v4', withIntegrations.url).href)
expect(response1?.status()).toBe(404)

const response2 = await page.goto(new URL('/test/serverless/v5', withIntegrations.url).href)
expect(response2?.status()).toBe(404)
})

test('Edge functions', async ({ page, withIntegrations }) => {
const response1 = await page.goto(new URL('/test/edge/v4', withIntegrations.url).href)
expect(response1?.status()).toBe(404)

const response2 = await page.goto(new URL('/test/edge/v5', withIntegrations.url).href)
expect(response2?.status()).toBe(404)
})
})

test.describe('Should keep functions produced by other build plugins', () => {
test('Serverless functions', async ({ page, withIntegrations }) => {
const response1 = await page.goto(
new URL('/test/serverless/integration-with-json-config', withIntegrations.url).href,
)
expect(response1?.status()).toBe(200)
expect(await response1?.text()).toBe('Hello from /test/serverless/integration-with-json-config')

const response2 = await page.goto(
new URL('/test/serverless/integration-with-json-config', withIntegrations.url).href,
)
expect(response2?.status()).toBe(200)
expect(await response2?.text()).toBe('Hello from /test/serverless/integration-with-json-config')
})

test('Edge functions', async ({ page, withIntegrations }) => {
const response1 = await page.goto(
new URL('/test/edge/integration-in-manifest', withIntegrations.url).href,
)
expect(response1?.status()).toBe(200)
expect(await response1?.text()).toBe('Hello from /test/edge/integration-in-manifest')

const response2 = await page.goto(
new URL('/test/edge/integration-not-in-manifest', withIntegrations.url).href,
)
expect(response2?.status()).toBe(200)
expect(await response2?.text()).toBe('Hello from /test/edge/integration-not-in-manifest')
})
})
2 changes: 2 additions & 0 deletions tests/fixtures/with-integrations/netlify.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "/plugins/create-other-functions"
8 changes: 8 additions & 0 deletions tests/fixtures/with-integrations/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/** @type {import('next').NextConfig} */
const nextConfig = {
eslint: {
ignoreDuringBuilds: true,
},
}

module.exports = nextConfig
13 changes: 13 additions & 0 deletions tests/fixtures/with-integrations/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "with-integrations",
"version": "0.1.0",
"private": true,
"scripts": {
"build": "next build"
},
"dependencies": {
"next": "latest",
"react": "18.2.0",
"react-dom": "18.2.0"
}
}
7 changes: 7 additions & 0 deletions tests/fixtures/with-integrations/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Home() {
return (
<main>
<h1>Hello World</h1>
</main>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function handler() {
return new Response('Hello from /test/edge/integration-in-manifest', { status: 200 })
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function handler() {
return new Response('Hello from /test/edge/integration-not-in-manifest', { status: 200 })
}

export const config = {
path: '/test/edge/integration-not-in-manifest',
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"functions": [
{
"function": "next-runtime-v4",
"name": "next-runtime-v4",
"path": "/test/edge/v4",
"generator": "@netlify/plugin-nextjs@4.41.3"
},
{
"function": "next-runtime-v5",
"name": "next-runtime-v5",
"path": "/test/edge/v5",
"generator": "@netlify/plugin-nextjs@5.3.3"
},
{
"function": "integration-in-manifest",
"name": "integration-in-manifest",
"path": "/test/edge/integration-in-manifest",
"generator": "@netlify/some-integration@1.0.0"
}
],
"layers": [],
"version": 1
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default function handler() {
return new Response('Hello from edge functions generated by @netlify/plugin-nextjs@4', {
status: 200,
})
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default function handler() {
return new Response('Hello from edge functions generated by @netlify/plugin-nextjs@5', {
status: 200,
})
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function handler() {
return new Response('Hello from /test/serverless/integration-with-json-config', { status: 200 })
}

export const config = {
path: '/test/serverless/integration-with-json-config',
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"config": {
"name": "Some integration",
"generator": "@netlify/some-integration@1.0.0"
},
"version": 1
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function handler() {
return new Response('Hello from integration generated serverless function', { status: 200 })
}

export const config = {
path: '/test/serverless/integration-with-json-config',
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"config": {
"generator": "@netlify/plugin-nextjs@4.41.3"
},
"version": 1
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default function handler() {
return new Response('Hello from edge functions generated by @netlify/plugin-nextjs@5', {
status: 200,
})
}

export const config = {
path: '/test/serverless/v4',
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"config": {
"generator": "@netlify/plugin-nextjs@5.3.3"
},
"version": 1
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default function handler() {
return new Response('Hello from edge functions generated by @netlify/plugin-nextjs@5', {
status: 200,
})
}

export const config = {
path: '/test/serverless/v5',
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const { cp } = require('node:fs/promises')
const { join } = require('node:path')

exports.onPreBuild = async function onPreBuild({
constants: { INTERNAL_FUNCTIONS_SRC, INTERNAL_EDGE_FUNCTIONS_SRC },
}) {
// copying functions:
// - mocked functions to represent stale function produced by @netlify/plugin-nextjs (specified by `generator`) for v4 and v5 of runtime
// - mocked functions to represent functions produced by other build plugins (either specified by `generator` or missing `generator` metadata)
await Promise.all([
cp(join(__dirname, 'edge-functions'), INTERNAL_EDGE_FUNCTIONS_SRC, { recursive: true }),
cp(join(__dirname, 'functions-internal'), INTERNAL_FUNCTIONS_SRC, { recursive: true }),
])
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
name: 'simulate-integration'
1 change: 1 addition & 0 deletions tests/utils/create-e2e-fixture.ts
Original file line number Diff line number Diff line change
@@ -353,6 +353,7 @@ export const fixtureFactories = {
createE2EFixture('cli-before-regional-blobs-support', {
expectedCliVersion: '17.21.1',
}),
withIntegrations: () => createE2EFixture('with-integrations'),
yarnMonorepoWithPnpmLinker: () =>
createE2EFixture('yarn-monorepo-with-pnpm-linker', {
packageManger: 'berry',