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

feat: support in-source configuration in edge functions #5197

Merged
merged 2 commits into from
Oct 31, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions src/lib/build.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const getBuildOptions = ({ cachedConfig, options: { context, cwd, debug, dry, js
offline,
cwd,
featureFlags: {
edge_functions_config_export: true,
functionsBundlingManifest: true,
edge_functions_produce_eszip: true,
project_deploy_configuration_api_use_per_function_configuration_files: true,
Expand Down
86 changes: 61 additions & 25 deletions src/lib/edge-functions/registry.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,12 @@ class EdgeFunctionsRegistry {
/**
* @type {EdgeFunctionDeclaration[]}
*/
this.declarations = this.getDeclarations(config)
this.declarationsFromConfig = this.getDeclarationsFromConfig(config)

/**
* @type {EdgeFunctionDeclaration[]}
*/
this.declarationsFromSource = []

/**
* @type {Record<string, string>}
Expand All @@ -101,6 +106,11 @@ class EdgeFunctionsRegistry {
*/
this.functionPaths = new Map()

/**
* @type {EdgeFunction[]}
*/
this.functions = []

/**
* @type {Promise<EdgeFunction[]>}
*/
Expand All @@ -114,13 +124,16 @@ class EdgeFunctionsRegistry {
*/
async build(functions) {
try {
const { graph, success } = await this.runIsolate(functions, this.env)
const { functionsConfig, graph, success } = await this.runIsolate(functions, this.env, {
getFunctionsConfig: true,
})

if (!success) {
throw new Error('Build error')
}

this.buildError = null
this.declarationsFromSource = functions.map((func, index) => ({ function: func.name, ...functionsConfig[index] }))

this.processGraph(graph)
} catch (error) {
Expand All @@ -141,7 +154,7 @@ class EdgeFunctionsRegistry {
return
}

const hasDeclaration = this.declarations.some((declaration) => declaration.function === func.name)
const hasDeclaration = this.declarationsFromConfig.some((declaration) => declaration.function === func.name)

// We only load the function if there's a config declaration for it.
return hasDeclaration
Expand Down Expand Up @@ -175,7 +188,7 @@ class EdgeFunctionsRegistry {
}
}

getDeclarations(config) {
getDeclarationsFromConfig(config) {
const { edge_functions: userFunctions = [] } = config

// The order is important, since we want to run user-defined functions
Expand Down Expand Up @@ -206,10 +219,6 @@ class EdgeFunctionsRegistry {
return env
}

getManifest() {
return this.bundler.generateManifest({ declarations: this.declarations, functions: this.functions })
}

async handleFileChange(path) {
const matchingFunctions = new Set(
[this.functionPaths.get(path), ...(this.dependencyPaths.get(path) || [])].filter(Boolean),
Expand Down Expand Up @@ -271,39 +280,66 @@ class EdgeFunctionsRegistry {
* @param {string} urlPath
*/
async matchURLPath(urlPath) {
const declarations = this.mergeDeclarations()
const manifest = await this.bundler.generateManifest({
declarations,
functions: this.functions,
})
const routes = manifest.routes.map((route) => ({
...route,
pattern: new RegExp(route.pattern),
}))
const functionNames = routes.filter(({ pattern }) => pattern.test(urlPath)).map((route) => route.function)
const orphanedDeclarations = await this.matchURLPathAgainstOrphanedDeclarations(urlPath)

return { functionNames, orphanedDeclarations }
}

async matchURLPathAgainstOrphanedDeclarations(urlPath) {
// `generateManifest` will only include functions for which there is both a
// function file and a config declaration, but we want to catch cases where
// a config declaration exists without a matching function file. To do that
// we compute a list of functions from the declarations (the `path` doesn't
// really matter) and later on match the resulting routes against the list
// of functions we have in the registry. Any functions found in the former
// but not the latter are treated as orphaned declarations.
const functions = this.declarations.map((declaration) => ({ name: declaration.function, path: '' }))
// really matter).
const functions = this.declarationsFromConfig.map((declaration) => ({ name: declaration.function, path: '' }))
const manifest = await this.bundler.generateManifest({
declarations: this.declarations,
declarations: this.declarationsFromConfig,
functions,
})
const routes = manifest.routes.map((route) => ({
...route,
pattern: new RegExp(route.pattern),
}))
const orphanedDeclarations = new Set()
const functionNames = routes
.filter(({ pattern }) => pattern.test(urlPath))
.map((route) => {
const matchingFunction = this.functions.find(({ name }) => name === route.function)

if (matchingFunction === undefined) {
orphanedDeclarations.add(route.function)
.filter((route) => {
const hasFunctionFile = this.functions.some((func) => func.name === route.function)

return null
if (hasFunctionFile) {
return false
}

return matchingFunction.name
return route.pattern.test(urlPath)
})
.filter(Boolean)
.map((route) => route.function)

return { functionNames, orphanedDeclarations }
return functionNames
}

// Merges declarations coming from the config and from the function sources.
mergeDeclarations() {
const declarations = [...this.declarationsFromConfig]

this.declarationsFromSource.forEach((declarationFromSource) => {
const index = declarations.findIndex(({ function: func }) => func === declarationFromSource.function)

if (index === -1) {
declarations.push(declarationFromSource)
} else {
declarations[index] = { ...declarations[index], ...declarationFromSource }
}
})

return declarations
}

processGraph(graph) {
Expand Down Expand Up @@ -376,7 +412,7 @@ class EdgeFunctionsRegistry {
onChange: async () => {
const newConfig = await this.getUpdatedConfig()

this.declarations = this.getDeclarations(newConfig)
this.declarationsFromConfig = this.getDeclarationsFromConfig(newConfig)

await this.checkForAddedOrDeletedFunctions()
},
Expand Down
49 changes: 49 additions & 0 deletions tests/integration/100.command.dev.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,55 @@ test('should detect deleted edge functions', async (t) => {
})
})

test('should respect in-source configuration from edge functions', async (t) => {
await withSiteBuilder('site-with-edge-functions', async (builder) => {
const publicDir = 'public'
await builder
.withNetlifyToml({
config: {
build: {
publish: publicDir,
edge_functions: 'netlify/edge-functions',
},
},
})
.withEdgeFunction({
config: () => ({ path: '/hello-1' }),
handler: () => new Response('Hello world'),
name: 'hello',
})

await builder.buildAsync()

await withDevServer({ cwd: builder.directory }, async ({ port }) => {
const res1 = await got(`http://localhost:${port}/hello-1`, { throwHttpErrors: false })

t.is(res1.statusCode, 200)
t.is(res1.body, 'Hello world')

await builder
.withEdgeFunction({
config: () => ({ path: '/hello-2' }),
handler: () => new Response('Hello world'),
name: 'hello',
})
.buildAsync()

const DETECT_FILE_CHANGE_DELAY = 500
await pause(DETECT_FILE_CHANGE_DELAY)

const res2 = await got(`http://localhost:${port}/hello-1`, { throwHttpErrors: false })

t.is(res2.statusCode, 404)

const res3 = await got(`http://localhost:${port}/hello-2`, { throwHttpErrors: false })

t.is(res3.statusCode, 200)
t.is(res3.body, 'Hello world')
})
})
})

test('should have only allowed environment variables set', async (t) => {
const siteInfo = {
account_slug: 'test-account',
Expand Down
9 changes: 7 additions & 2 deletions tests/integration/utils/site-builder.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,16 @@ const createSiteBuilder = ({ siteName }) => {
})
return builder
},
withEdgeFunction: ({ handler, internal = false, name = 'function', pathPrefix = '' }) => {
withEdgeFunction: ({ config, handler, internal = false, name = 'function', pathPrefix = '' }) => {
const edgeFunctionsDirectory = internal ? '.netlify/edge-functions' : 'netlify/edge-functions'
const dest = path.join(directory, pathPrefix, edgeFunctionsDirectory, `${name}.js`)
tasks.push(async () => {
const content = typeof handler === 'string' ? handler : `export default ${handler.toString()}`
let content = typeof handler === 'string' ? handler : `export default ${handler.toString()}`

if (config) {
content += `;export const config = ${config.toString()}`
}

await ensureDir(path.dirname(dest))
await writeFile(dest, content)
})
Expand Down