feat: split user and internal ISC function configs (#347)

* feat: split user and internal ISC function configs

* chore: run getFunctionConfig in parallel

* chore: comments

node/__snapshots__/declaration.test.ts.snap

@@ -0,0 +1,38 @@
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
+
+exports[`Deploy config takes precedence over user config 1`] = `
+[
+  {
+    "function": "user-a",
+    "path": "/path1",
+  },
+  {
+    "function": "user-b",
+    "path": "/path2",
+  },
+  {
+    "function": "framework-a",
+    "path": "/path1",
+  },
+  {
+    "function": "framework-b",
+    "path": "/path2",
+  },
+  {
+    "function": "framework-c",
+    "path": "/path1",
+  },
+  {
+    "function": "framework-c",
+    "path": "/path2",
+  },
+  {
+    "function": "user-c",
+    "path": "/path1",
+  },
+  {
+    "function": "user-c",
+    "path": "/path2",
+  },
+]
+`;

node/bundler.ts

@@ -1,5 +1,5 @@
 import { promises as fs } from 'fs'
-import { join, resolve } from 'path'
+import { join } from 'path'
 
 import commonPathPrefix from 'common-path-prefix'
 import isPathInside from 'is-path-inside'
@@ -9,7 +9,7 @@ import { importMapSpecifier } from '../shared/consts.js'
 
 import { DenoBridge, DenoOptions, OnAfterDownloadHook, OnBeforeDownloadHook } from './bridge.js'
 import type { Bundle } from './bundle.js'
-import { FunctionConfig, getFunctionConfig } from './config.js'
+import { getFunctionConfig } from './config.js'
 import { Declaration, mergeDeclarations } from './declaration.js'
 import { load as loadDeployConfig } from './deploy_config.js'
 import { EdgeFunction } from './edge_function.js'
@@ -62,7 +62,7 @@ const bundle = async (
     onAfterDownload,
     onBeforeDownload,
   }
-  const internalFunctionsPath = internalSrcFolder && resolve(internalSrcFolder)
+
   if (cacheDirectory !== undefined) {
     options.denoDir = join(cacheDirectory, 'deno_dir')
   }
@@ -84,11 +84,17 @@ const bundle = async (
   // Layers are marked as externals in the ESZIP, so that those specifiers are
   // not actually included in the bundle.
   const externals = deployConfig.layers.map((layer) => layer.name)
+
+  const userSourceDirectories = sourceDirectories.filter((dir) => dir !== internalSrcFolder)
+
   const importMap = new ImportMap()
 
   await importMap.addFiles([deployConfig?.importMap, ...importMapPaths], logger)
 
-  const functions = await findFunctions(sourceDirectories)
+  const userFunctions = userSourceDirectories.length === 0 ? [] : await findFunctions(userSourceDirectories)
+  const internalFunctions = internalSrcFolder ? await findFunctions([internalSrcFolder]) : []
+  const functions = [...internalFunctions, ...userFunctions]
+
   const functionBundle = await bundleESZIP({
     basePath,
     buildID,
@@ -107,26 +113,32 @@ const bundle = async (
   await createFinalBundles([functionBundle], distDirectory, buildID)
 
   // Retrieving a configuration object for each function.
-  const functionsConfig = await Promise.all(
-    functions.map((func) => getFunctionConfig(func, importMap, deno, logger, featureFlags)),
+  // Run `getFunctionConfig` in parallel as it is a non-trivial operation and spins up deno
+  const internalConfigPromises = internalFunctions.map(
+    async (func) => [func.name, await getFunctionConfig(func, importMap, deno, logger, featureFlags)] as const,
   )
 
-  // Creating a hash of function names to configuration objects.
-  const functionsWithConfig = functions.reduce(
-    (acc, func, index) => ({ ...acc, [func.name]: functionsConfig[index] }),
-    {} as Record<string, FunctionConfig>,
+  const userConfigPromises = userFunctions.map(
+    async (func) => [func.name, await getFunctionConfig(func, importMap, deno, logger, featureFlags)] as const,
   )
 
+  // Creating a hash of function names to configuration objects.
+  const internalFunctionsWithConfig = Object.fromEntries(await Promise.all(internalConfigPromises))
+  const userFunctionsWithConfig = Object.fromEntries(await Promise.all(userConfigPromises))
+
   // Creating a final declarations array by combining the TOML file with the
   // deploy configuration API and the in-source configuration.
-  const declarationsFromConfig = mergeDeclarations(tomlDeclarations, functionsWithConfig, deployConfig.declarations)
+  const declarationsFromConfig = mergeDeclarations(
+    tomlDeclarations,
+    userFunctionsWithConfig,
+    internalFunctionsWithConfig,
+    deployConfig.declarations,
+  )
 
   // If any declarations are autogenerated and are missing the generator field
   // add a default string.
-  const declarations = internalFunctionsPath
-    ? declarationsFromConfig.map((declaration) =>
-        addGeneratorFieldIfMissing(declaration, functions, internalFunctionsPath),
-      )
+  const declarations = internalSrcFolder
+    ? declarationsFromConfig.map((declaration) => addGeneratorFieldIfMissing(declaration, functions, internalSrcFolder))
     : declarationsFromConfig
 
   const manifest = await writeManifest({
@@ -135,7 +147,8 @@ const bundle = async (
     distDirectory,
     featureFlags,
     functions,
-    functionConfig: functionsWithConfig,
+    userFunctionConfig: userFunctionsWithConfig,
+    internalFunctionConfig: internalFunctionsWithConfig,
     importMap: importMapSpecifier,
     layers: deployConfig.layers,
   })

node/declaration.test.ts

@@ -3,16 +3,37 @@ import { test, expect } from 'vitest'
 import { FunctionConfig } from './config.js'
 import { Declaration, mergeDeclarations } from './declaration.js'
 
-// TODO: Add tests with the deploy config.
 const deployConfigDeclarations: Declaration[] = []
 
+test('Deploy config takes precedence over user config', () => {
+  const deployConfigDeclarations: Declaration[] = [
+    { function: 'framework-a', path: '/path1' },
+    { function: 'framework-b', path: '/path2' },
+  ]
+
+  const tomlConfig: Declaration[] = [
+    { function: 'user-a', path: '/path1' },
+    { function: 'user-b', path: '/path2' },
+  ]
+
+  const userFuncConfig = {
+    'user-c': { path: ['/path1', '/path2'] },
+  } as Record<string, FunctionConfig>
+
+  const internalFuncConfig = {
+    'framework-c': { path: ['/path1', '/path2'] },
+  } as Record<string, FunctionConfig>
+
+  expect(mergeDeclarations(tomlConfig, userFuncConfig, internalFuncConfig, deployConfigDeclarations)).toMatchSnapshot()
+})
+
 test('In-source config takes precedence over netlify.toml config', () => {
   const tomlConfig: Declaration[] = [
     { function: 'geolocation', path: '/geo', cache: 'off' },
     { function: 'json', path: '/json', cache: 'manual' },
   ]
 
-  const funcConfig = {
+  const userFuncConfig = {
     geolocation: { path: ['/geo-isc', '/*'], cache: 'manual' },
     json: { path: '/json', cache: 'off' },
   } as Record<string, FunctionConfig>
@@ -23,7 +44,7 @@ test('In-source config takes precedence over netlify.toml config', () => {
     { function: 'json', path: '/json', cache: 'off' },
   ]
 
-  const declarations = mergeDeclarations(tomlConfig, funcConfig, deployConfigDeclarations)
+  const declarations = mergeDeclarations(tomlConfig, userFuncConfig, {}, deployConfigDeclarations)
 
   expect(declarations).toEqual(expectedDeclarations)
 })
@@ -34,7 +55,7 @@ test("Declarations don't break if no in-source config is provided", () => {
     { function: 'json', path: '/json', cache: 'manual' },
   ]
 
-  const funcConfig = {
+  const userFuncConfig = {
     geolocation: { path: ['/geo-isc'], cache: 'manual' },
     json: {},
   } as Record<string, FunctionConfig>
@@ -44,7 +65,7 @@ test("Declarations don't break if no in-source config is provided", () => {
     { function: 'json', path: '/json', cache: 'manual' },
   ]
 
-  const declarations = mergeDeclarations(tomlConfig, funcConfig, deployConfigDeclarations)
+  const declarations = mergeDeclarations(tomlConfig, userFuncConfig, {}, deployConfigDeclarations)
 
   expect(declarations).toEqual(expectedDeclarations)
 })
@@ -68,10 +89,10 @@ test('In-source config works independent of the netlify.toml file if a path is d
 
   const expectedDeclarationsWithoutISCPath = [{ function: 'geolocation', path: '/geo', cache: 'off' }]
 
-  const declarationsWithISCPath = mergeDeclarations(tomlConfig, funcConfigWithPath, deployConfigDeclarations)
+  const declarationsWithISCPath = mergeDeclarations(tomlConfig, funcConfigWithPath, {}, deployConfigDeclarations)
   expect(declarationsWithISCPath).toEqual(expectedDeclarationsWithISCPath)
 
-  const declarationsWithoutISCPath = mergeDeclarations(tomlConfig, funcConfigWithoutPath, deployConfigDeclarations)
+  const declarationsWithoutISCPath = mergeDeclarations(tomlConfig, funcConfigWithoutPath, {}, deployConfigDeclarations)
   expect(declarationsWithoutISCPath).toEqual(expectedDeclarationsWithoutISCPath)
 })
 
@@ -84,7 +105,7 @@ test('In-source config works if only the cache config property is set', () => {
 
   const expectedDeclarations = [{ function: 'geolocation', path: '/geo', cache: 'manual' }]
 
-  expect(mergeDeclarations(tomlConfig, funcConfig, deployConfigDeclarations)).toEqual(expectedDeclarations)
+  expect(mergeDeclarations(tomlConfig, funcConfig, {}, deployConfigDeclarations)).toEqual(expectedDeclarations)
 })
 
 test("In-source config path property works if it's not an array", () => {
@@ -96,7 +117,7 @@ test("In-source config path property works if it's not an array", () => {
 
   const expectedDeclarations = [{ function: 'json', path: '/json', cache: 'manual' }]
 
-  expect(mergeDeclarations(tomlConfig, funcConfig, deployConfigDeclarations)).toEqual(expectedDeclarations)
+  expect(mergeDeclarations(tomlConfig, funcConfig, {}, deployConfigDeclarations)).toEqual(expectedDeclarations)
 })
 
 test("In-source config path property works if it's not an array and it's not present in toml or deploy config", () => {
@@ -110,7 +131,7 @@ test("In-source config path property works if it's not an array and it's not pre
     { function: 'json', path: '/json-isc', cache: 'manual' },
   ]
 
-  expect(mergeDeclarations(tomlConfig, funcConfig, deployConfigDeclarations)).toEqual(expectedDeclarations)
+  expect(mergeDeclarations(tomlConfig, funcConfig, {}, deployConfigDeclarations)).toEqual(expectedDeclarations)
 })
 
 test('In-source config works if path property is an empty array with cache value specified', () => {
@@ -122,7 +143,7 @@ test('In-source config works if path property is an empty array with cache value
 
   const expectedDeclarations = [{ function: 'json', path: '/json-toml', cache: 'manual' }]
 
-  expect(mergeDeclarations(tomlConfig, funcConfig, deployConfigDeclarations)).toEqual(expectedDeclarations)
+  expect(mergeDeclarations(tomlConfig, funcConfig, {}, deployConfigDeclarations)).toEqual(expectedDeclarations)
 })
 
 test('netlify.toml-defined excludedPath are respected', () => {
@@ -132,7 +153,7 @@ test('netlify.toml-defined excludedPath are respected', () => {
 
   const expectedDeclarations = [{ function: 'geolocation', path: '/geo/*', excludedPath: '/geo/exclude' }]
 
-  const declarations = mergeDeclarations(tomlConfig, funcConfig, deployConfigDeclarations)
+  const declarations = mergeDeclarations(tomlConfig, funcConfig, {}, deployConfigDeclarations)
 
   expect(declarations).toEqual(expectedDeclarations)
 })

node/declaration.ts

@@ -23,7 +23,8 @@ export type Declaration = DeclarationWithPath | DeclarationWithPattern
 
 export const mergeDeclarations = (
   tomlDeclarations: Declaration[],
-  functionsConfig: Record<string, FunctionConfig>,
+  userFunctionsConfig: Record<string, FunctionConfig>,
+  internalFunctionsConfig: Record<string, FunctionConfig>,
   deployConfigDeclarations: Declaration[],
 ) => {
   const declarations: Declaration[] = []
@@ -34,7 +35,7 @@ export const mergeDeclarations = (
   // a function configuration object, we replace the path because that object
   // takes precedence.
   for (const declaration of [...tomlDeclarations, ...deployConfigDeclarations]) {
-    const config = functionsConfig[declaration.function]
+    const config = userFunctionsConfig[declaration.function] || internalFunctionsConfig[declaration.function]
 
     if (!config) {
       // If no config is found, add the declaration as is.
@@ -59,15 +60,19 @@ export const mergeDeclarations = (
 
   // Finally, we must create declarations for functions that are not declared
   // in the TOML at all.
-  for (const name in functionsConfig) {
-    const { cache, path } = functionsConfig[name]
+  for (const name in { ...internalFunctionsConfig, ...userFunctionsConfig }) {
+    const { cache, path } = internalFunctionsConfig[name] || userFunctionsConfig[name]
 
     // If we have a path specified, create a declaration for each path.
     if (!functionsVisited.has(name) && path) {
       const paths = Array.isArray(path) ? path : [path]
 
       paths.forEach((singlePath) => {
-        declarations.push({ cache, function: name, path: singlePath })
+        const declaration: Declaration = { function: name, path: singlePath }
+        if (cache) {
+          declaration.cache = cache
+        }
+        declarations.push(declaration)
       })
     }
   }

node/manifest.test.ts

@@ -111,12 +111,12 @@ test('Includes failure modes in manifest', () => {
     { function: 'func-1', name: 'Display Name', path: '/f1/*' },
     { function: 'func-2', pattern: '^/f2/.*/?$' },
   ]
-  const functionConfig: Record<string, FunctionConfig> = {
+  const userFunctionConfig: Record<string, FunctionConfig> = {
     'func-1': {
       onError: '/custom-error',
     },
   }
-  const manifest = generateManifest({ bundles: [], declarations, functions, functionConfig })
+  const manifest = generateManifest({ bundles: [], declarations, functions, userFunctionConfig })
   expect(manifest.function_config).toEqual({
     'func-1': { excluded_patterns: [], on_error: '/custom-error' },
   })

node/manifest.ts

@@ -38,9 +38,10 @@ interface GenerateManifestOptions {
   declarations?: Declaration[]
   featureFlags?: FeatureFlags
   functions: EdgeFunction[]
-  functionConfig?: Record<string, FunctionConfig>
   importMap?: string
+  internalFunctionConfig?: Record<string, FunctionConfig>
   layers?: Layer[]
+  userFunctionConfig?: Record<string, FunctionConfig>
 }
 
 // JavaScript regular expressions are converted to strings with leading and
@@ -66,7 +67,8 @@ const generateManifest = ({
   declarations = [],
   featureFlags,
   functions,
-  functionConfig = {},
+  userFunctionConfig = {},
+  internalFunctionConfig = {},
   importMap,
   layers = [],
 }: GenerateManifestOptions) => {
@@ -76,7 +78,10 @@ const generateManifest = ({
     functions.map(({ name }) => [name, { excluded_patterns: [] }]),
   )
 
-  for (const [name, { excludedPath, onError }] of Object.entries(functionConfig)) {
+  for (const [name, { excludedPath, onError }] of Object.entries({
+    ...internalFunctionConfig,
+    ...userFunctionConfig,
+  })) {
     // If the config block is for a function that is not defined, discard it.
     if (manifestFunctionConfig[name] === undefined) {
       continue

test/integration/internal-functions/func2.ts

@@ -0,0 +1,3 @@
+export default async () => new Response('Hello')
+
+export const config = { path: '/func2' }