Skip to content

Commit

Permalink
fix: change the order of how edge functions are written to the manife…
Browse files Browse the repository at this point in the history
…st (#357)
  • Loading branch information
danez committed Apr 5, 2023
1 parent 2eee603 commit 59d1c8c
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 28 deletions.
71 changes: 62 additions & 9 deletions node/__snapshots__/declaration.test.ts.snap
Original file line number Diff line number Diff line change
@@ -1,37 +1,90 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`Deploy config takes precedence over user config 1`] = `
exports[`Ensure the order of edge functions with FF 1`] = `
[
{
"function": "user-a",
"function": "framework-manifest-a",
"path": "/path1",
},
{
"function": "user-b",
"function": "framework-manifest-c",
"path": "/path3",
},
{
"function": "framework-manifest-b",
"path": "/path2",
},
{
"function": "framework-isc-c",
"path": "/path1",
},
{
"function": "framework-isc-c",
"path": "/path2",
},
{
"function": "user-toml-a",
"path": "/path1",
},
{
"function": "user-toml-c",
"path": "/path3",
},
{
"function": "user-toml-b",
"path": "/path2",
},
{
"function": "user-isc-c",
"path": "/path1",
},
{
"function": "user-isc-c",
"path": "/path2",
},
]
`;

exports[`Ensure the order of edge functions without FF 1`] = `
[
{
"function": "user-toml-a",
"path": "/path1",
},
{
"function": "user-toml-c",
"path": "/path3",
},
{
"function": "user-toml-b",
"path": "/path2",
},
{
"function": "framework-a",
"function": "framework-manifest-a",
"path": "/path1",
},
{
"function": "framework-b",
"function": "framework-manifest-c",
"path": "/path3",
},
{
"function": "framework-manifest-b",
"path": "/path2",
},
{
"function": "framework-c",
"function": "framework-isc-c",
"path": "/path1",
},
{
"function": "framework-c",
"function": "framework-isc-c",
"path": "/path2",
},
{
"function": "user-c",
"function": "user-isc-c",
"path": "/path1",
},
{
"function": "user-c",
"function": "user-isc-c",
"path": "/path2",
},
]
Expand Down
1 change: 1 addition & 0 deletions node/bundler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ const bundle = async (
userFunctionsWithConfig,
internalFunctionsWithConfig,
deployConfig.declarations,
featureFlags,
)

const internalFunctionConfig = createFunctionConfig({
Expand Down
50 changes: 42 additions & 8 deletions node/declaration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,60 @@ import { Declaration, mergeDeclarations } from './declaration.js'

const deployConfigDeclarations: Declaration[] = []

test('Deploy config takes precedence over user config', () => {
test('Ensure the order of edge functions with FF', () => {
const deployConfigDeclarations: Declaration[] = [
{ function: 'framework-a', path: '/path1' },
{ function: 'framework-b', path: '/path2' },
{ function: 'framework-manifest-a', path: '/path1' },
{ function: 'framework-manifest-c', path: '/path3' },
{ function: 'framework-manifest-b', path: '/path2' },
]

const tomlConfig: Declaration[] = [
{ function: 'user-a', path: '/path1' },
{ function: 'user-b', path: '/path2' },
{ function: 'user-toml-a', path: '/path1' },
{ function: 'user-toml-c', path: '/path3' },
{ function: 'user-toml-b', path: '/path2' },
]

const userFuncConfig = {
'user-c': { path: ['/path1', '/path2'] },
'user-isc-c': { path: ['/path1', '/path2'] },
} as Record<string, FunctionConfig>

const internalFuncConfig = {
'framework-c': { path: ['/path1', '/path2'] },
'framework-isc-c': { path: ['/path1', '/path2'] },
} as Record<string, FunctionConfig>

expect(mergeDeclarations(tomlConfig, userFuncConfig, internalFuncConfig, deployConfigDeclarations)).toMatchSnapshot()
expect(
mergeDeclarations(tomlConfig, userFuncConfig, internalFuncConfig, deployConfigDeclarations, {
edge_functions_correct_order: true,
}),
).toMatchSnapshot()
})

test('Ensure the order of edge functions without FF', () => {
const deployConfigDeclarations: Declaration[] = [
{ function: 'framework-manifest-a', path: '/path1' },
{ function: 'framework-manifest-c', path: '/path3' },
{ function: 'framework-manifest-b', path: '/path2' },
]

const tomlConfig: Declaration[] = [
{ function: 'user-toml-a', path: '/path1' },
{ function: 'user-toml-c', path: '/path3' },
{ function: 'user-toml-b', path: '/path2' },
]

const userFuncConfig = {
'user-isc-c': { path: ['/path1', '/path2'] },
} as Record<string, FunctionConfig>

const internalFuncConfig = {
'framework-isc-c': { path: ['/path1', '/path2'] },
} as Record<string, FunctionConfig>

expect(
mergeDeclarations(tomlConfig, userFuncConfig, internalFuncConfig, deployConfigDeclarations, {
edge_functions_correct_order: false,
}),
).toMatchSnapshot()
})

test('In-source config takes precedence over netlify.toml config', () => {
Expand Down
68 changes: 57 additions & 11 deletions node/declaration.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import regexpAST from 'regexp-tree'

import { FunctionConfig, Path } from './config.js'
import { FeatureFlags } from './feature_flags.js'

interface BaseDeclaration {
cache?: string
Expand All @@ -27,16 +28,54 @@ export const mergeDeclarations = (
userFunctionsConfig: Record<string, FunctionConfig>,
internalFunctionsConfig: Record<string, FunctionConfig>,
deployConfigDeclarations: Declaration[],
featureFlags: FeatureFlags = {},
// eslint-disable-next-line max-params
) => {
const declarations: Declaration[] = []
const functionsVisited: Set<string> = new Set()

// We start by iterating over all the declarations in the TOML file and in
// the deploy configuration file. For any declaration for which we also have
// a function configuration object, we replace the path because that object
// takes precedence.
for (const declaration of [...tomlDeclarations, ...deployConfigDeclarations]) {
const config = userFunctionsConfig[declaration.function] || internalFunctionsConfig[declaration.function]
let declarations: Declaration[] = getDeclarationsFromInput(
deployConfigDeclarations,
internalFunctionsConfig,
functionsVisited,
)

// eslint-disable-next-line unicorn/prefer-ternary
if (featureFlags.edge_functions_correct_order) {
declarations = [
// INTEGRATIONS
// 1. Declarations from the integrations deploy config
...getDeclarationsFromInput(deployConfigDeclarations, internalFunctionsConfig, functionsVisited),
// 2. Declarations from the integrations ISC
...createDeclarationsFromFunctionConfigs(internalFunctionsConfig, functionsVisited),

// USER
// 3. Declarations from the users toml config
...getDeclarationsFromInput(tomlDeclarations, userFunctionsConfig, functionsVisited),
// 4. Declarations from the users ISC
...createDeclarationsFromFunctionConfigs(userFunctionsConfig, functionsVisited),
]
} else {
declarations = [
...getDeclarationsFromInput(tomlDeclarations, userFunctionsConfig, functionsVisited),
...getDeclarationsFromInput(deployConfigDeclarations, internalFunctionsConfig, functionsVisited),
...createDeclarationsFromFunctionConfigs(internalFunctionsConfig, functionsVisited),
...createDeclarationsFromFunctionConfigs(userFunctionsConfig, functionsVisited),
]
}

return declarations
}

const getDeclarationsFromInput = (
inputDeclarations: Declaration[],
functionConfigs: Record<string, FunctionConfig>,
functionsVisited: Set<string>,
): Declaration[] => {
const declarations: Declaration[] = []
// For any declaration for which we also have a function configuration object,
// we replace the path because that object takes precedence.
for (const declaration of inputDeclarations) {
const config = functionConfigs[declaration.function]

if (!config) {
// If no config is found, add the declaration as is.
Expand All @@ -59,10 +98,17 @@ export const mergeDeclarations = (
functionsVisited.add(declaration.function)
}

// Finally, we must create declarations for functions that are not declared
// in the TOML at all.
for (const name in { ...internalFunctionsConfig, ...userFunctionsConfig }) {
const { cache, path } = internalFunctionsConfig[name] || userFunctionsConfig[name]
return declarations
}

const createDeclarationsFromFunctionConfigs = (
functionConfigs: Record<string, FunctionConfig>,
functionsVisited: Set<string>,
): Declaration[] => {
const declarations: Declaration[] = []

for (const name in functionConfigs) {
const { cache, path } = functionConfigs[name]

// If we have a path specified, create a declaration for each path.
if (!functionsVisited.has(name) && path) {
Expand Down
1 change: 1 addition & 0 deletions node/feature_flags.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const defaultFlags = {
edge_functions_correct_order: false,
edge_functions_fail_unsupported_regex: false,
edge_functions_invalid_config_throw: false,
edge_functions_manifest_validate_slash: false,
Expand Down

0 comments on commit 59d1c8c

Please sign in to comment.