feat!: split user and internal ISC function configs

- add a new test for the order of internal functions - ensures that the cache property is only added to the manifest if not undefined BREAKING CHANGE: `bundle()` has a new mandatory param `internalSourceDirectory` for the internal function directory
netlify · Mar 22, 2023 · 7996715 · 7996715
1 parent e8ba284
commit 7996715
Show file tree

Hide file tree

Showing 10 changed files with 166 additions and 64 deletions.
diff --git a/node/__snapshots__/declaration.test.ts.snap b/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",
+  },
+]
+`;
diff --git a/node/bundler.test.ts b/node/bundler.test.ts
@@ -26,7 +26,7 @@ test('Produces an ESZIP bundle', async () => {
   ]
   const userDirectory = join(basePath, 'user-functions')
   const internalDirectory = join(basePath, 'functions')
-  const result = await bundle([userDirectory, internalDirectory], distPath, declarations, {
+  const result = await bundle([userDirectory], internalDirectory, distPath, declarations, {
     basePath,
     configPath: join(internalDirectory, 'config.json'),
     importMapPaths: [join(userDirectory, 'import_map.json')],
@@ -67,7 +67,8 @@ test('Uses the vendored eszip module instead of fetching it from deno.land', asy
     },
   ]
   const sourceDirectory = join(basePath, 'functions')
-  const result = await bundle([sourceDirectory], distPath, declarations, {
+  const internalDirectory = join(basePath, 'notexist')
+  const result = await bundle([sourceDirectory], internalDirectory, distPath, declarations, {
     basePath,
     configPath: join(sourceDirectory, 'config.json'),
   })
@@ -92,6 +93,7 @@ test('Adds a custom error property to user errors during bundling', async () =>
 
   const { basePath, cleanup, distPath } = await useFixture('invalid_functions')
   const sourceDirectory = join(basePath, 'functions')
+  const internalDirectory = join(basePath, 'notexist')
   const declarations: Declaration[] = [
     {
       function: 'func1',
@@ -100,7 +102,7 @@ test('Adds a custom error property to user errors during bundling', async () =>
   ]
 
   try {
-    await bundle([sourceDirectory], distPath, declarations, { basePath })
+    await bundle([sourceDirectory], internalDirectory, distPath, declarations, { basePath })
   } catch (error) {
     expect(error).toBeInstanceOf(BundleError)
     expect((error as BundleError).customErrorInfo).toEqual({
@@ -120,6 +122,7 @@ test('Prints a nice error message when user tries importing NPM module', async (
 
   const { basePath, cleanup, distPath } = await useFixture('imports_npm_module')
   const sourceDirectory = join(basePath, 'functions')
+  const internalDirectory = join(basePath, 'notexist')
   const declarations: Declaration[] = [
     {
       function: 'func1',
@@ -128,7 +131,7 @@ test('Prints a nice error message when user tries importing NPM module', async (
   ]
 
   try {
-    await bundle([sourceDirectory], distPath, declarations, { basePath })
+    await bundle([sourceDirectory], internalDirectory, distPath, declarations, { basePath })
   } catch (error) {
     expect(error).toBeInstanceOf(BundleError)
     expect((error as BundleError).message).toEqual(
@@ -144,6 +147,7 @@ test('Prints a nice error message when user tries importing NPM module with npm:
 
   const { basePath, cleanup, distPath } = await useFixture('imports_npm_module_scheme')
   const sourceDirectory = join(basePath, 'functions')
+  const internalDirectory = join(basePath, 'notexist')
   const declarations: Declaration[] = [
     {
       function: 'func1',
@@ -152,7 +156,7 @@ test('Prints a nice error message when user tries importing NPM module with npm:
   ]
 
   try {
-    await bundle([sourceDirectory], distPath, declarations, { basePath })
+    await bundle([sourceDirectory], internalDirectory, distPath, declarations, { basePath })
   } catch (error) {
     expect(error).toBeInstanceOf(BundleError)
     expect((error as BundleError).message).toEqual(
@@ -179,6 +183,7 @@ test('Uses the cache directory as the `DENO_DIR` value', async () => {
 
   const { basePath, cleanup, distPath } = await useFixture('with_import_maps')
   const sourceDirectory = join(basePath, 'functions')
+  const internalDirectory = join(basePath, 'notexist')
   const cacheDir = await tmp.dir()
   const declarations: Declaration[] = [
     {
@@ -192,7 +197,7 @@ test('Uses the cache directory as the `DENO_DIR` value', async () => {
     configPath: join(sourceDirectory, 'config.json'),
   }
 
-  const result = await bundle([sourceDirectory], distPath, declarations, options)
+  const result = await bundle([sourceDirectory], internalDirectory, distPath, declarations, options)
   const outFiles = await fs.readdir(distPath)
 
   expect(result.functions.length).toBe(1)
@@ -208,13 +213,14 @@ test('Uses the cache directory as the `DENO_DIR` value', async () => {
 test('Supports import maps with relative paths', async () => {
   const { basePath, cleanup, distPath } = await useFixture('with_import_maps')
   const sourceDirectory = join(basePath, 'functions')
+  const internalDirectory = join(basePath, 'notexist')
   const declarations: Declaration[] = [
     {
       function: 'func1',
       path: '/func1',
     },
   ]
-  const result = await bundle([sourceDirectory], distPath, declarations, {
+  const result = await bundle([sourceDirectory], internalDirectory, distPath, declarations, {
     basePath,
     configPath: join(sourceDirectory, 'config.json'),
   })
@@ -237,6 +243,7 @@ test('Supports import maps with relative paths', async () => {
 test('Ignores any user-defined `deno.json` files', async () => {
   const { basePath, cleanup, distPath } = await useFixture('with_import_maps')
   const sourceDirectory = join(basePath, 'functions')
+  const internalDirectory = join(basePath, 'notexist')
   const declarations: Declaration[] = [
     {
       function: 'func1',
@@ -279,7 +286,7 @@ test('Ignores any user-defined `deno.json` files', async () => {
   await fs.writeFile(denoConfigPath, JSON.stringify(denoConfig))
 
   expect(() =>
-    bundle([sourceDirectory], distPath, declarations, {
+    bundle([sourceDirectory], internalDirectory, distPath, declarations, {
       basePath,
       configPath: join(sourceDirectory, 'config.json'),
     }),
@@ -292,14 +299,15 @@ test('Ignores any user-defined `deno.json` files', async () => {
 test('Processes a function that imports a custom layer', async () => {
   const { basePath, cleanup, distPath } = await useFixture('with_layers')
   const sourceDirectory = join(basePath, 'functions')
+  const internalDirectory = join(basePath, 'notexist')
   const declarations: Declaration[] = [
     {
       function: 'func1',
       path: '/func1',
     },
   ]
   const layer = { name: 'https://edge-function-layer-template.netlify.app/mod.ts', flag: 'edge-functions-layer-test' }
-  const result = await bundle([sourceDirectory], distPath, declarations, {
+  const result = await bundle([sourceDirectory], internalDirectory, distPath, declarations, {
     basePath,
     configPath: join(sourceDirectory, 'config.json'),
   })
@@ -329,11 +337,13 @@ test('Loads declarations and import maps from the deploy configuration', async (
       path: '/func1',
     },
   ]
-  const directories = [join(basePath, 'netlify', 'edge-functions'), join(basePath, '.netlify', 'edge-functions')]
-  const result = await bundle(directories, distPath, declarations, {
+
+  const sourceDirectory = join(basePath, 'netlify', 'edge-functions')
+  const internalDirectory = join(basePath, '.netlify', 'edge-functions')
+
+  const result = await bundle([sourceDirectory], internalDirectory, distPath, declarations, {
     basePath,
     configPath: join(basePath, '.netlify', 'edge-functions', 'manifest.json'),
-    internalSrcFolder: directories[1],
   })
   const generatedFiles = await fs.readdir(distPath)
 
@@ -362,6 +372,7 @@ test("Ignores entries in `importMapPaths` that don't point to an existing import
   const systemLogger = vi.fn()
   const { basePath, cleanup, distPath } = await useFixture('with_import_maps')
   const sourceDirectory = join(basePath, 'user-functions')
+  const internalDirectory = join(basePath, 'notexist')
 
   // Creating import map file
   const importMap = await tmp.file()
@@ -381,6 +392,7 @@ test("Ignores entries in `importMapPaths` that don't point to an existing import
   const nonExistingImportMapPath = join(distPath, 'some-file-that-does-not-exist.json')
   const result = await bundle(
     [sourceDirectory],
+    internalDirectory,
     distPath,
     [
       {

diff --git a/node/bundler.ts b/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'
@@ -32,11 +32,11 @@ interface BundleOptions {
   onAfterDownload?: OnAfterDownloadHook
   onBeforeDownload?: OnBeforeDownloadHook
   systemLogger?: LogFunction
-  internalSrcFolder?: string
 }
 
 const bundle = async (
   sourceDirectories: string[],
+  internalSourceDirectory: string,
   distDirectory: string,
   tomlDeclarations: Declaration[] = [],
   {
@@ -50,8 +50,8 @@ const bundle = async (
     onAfterDownload,
     onBeforeDownload,
     systemLogger,
-    internalSrcFolder,
   }: BundleOptions = {},
+  // eslint-disable-next-line max-params
 ) => {
   const logger = getLogger(systemLogger, debug)
   const featureFlags = getFlags(inputFeatureFlags)
@@ -62,7 +62,7 @@ const bundle = async (
     onAfterDownload,
     onBeforeDownload,
   }
-  const internalFunctionsPath = internalSrcFolder && resolve(internalSrcFolder)
+
   if (cacheDirectory !== undefined) {
     options.denoDir = join(cacheDirectory, 'deno_dir')
   }
@@ -88,7 +88,10 @@ const bundle = async (
 
   await importMap.addFiles([deployConfig?.importMap, ...importMapPaths], logger)
 
-  const functions = await findFunctions(sourceDirectories)
+  const userFunctions = sourceDirectories.length === 0 ? [] : await findFunctions(sourceDirectories)
+  const internalFunctions = await findFunctions([internalSourceDirectory])
+  const functions = [...internalFunctions, ...userFunctions]
+
   const functionBundle = await bundleESZIP({
     basePath,
     buildID,
@@ -106,36 +109,40 @@ const bundle = async (
   // rename the bundles to their permanent names.
   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)),
-  )
-
   // 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 internalFunctionsWithConfig: Record<string, FunctionConfig> = {}
+  for (const func of internalFunctions) {
+    internalFunctionsWithConfig[func.name] = await getFunctionConfig(func, importMap, deno, logger, featureFlags)
+  }
+
+  const userFunctionsWithConfig: Record<string, FunctionConfig> = {}
+  for (const func of userFunctions) {
+    userFunctionsWithConfig[func.name] = await getFunctionConfig(func, importMap, deno, logger, featureFlags)
+  }
 
   // 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),
-      )
-    : declarationsFromConfig
+  const declarations = declarationsFromConfig.map((declaration) =>
+    addGeneratorFieldIfMissing(declaration, functions, internalSourceDirectory),
+  )
 
   const manifest = await writeManifest({
     bundles: [functionBundle],
     declarations,
     distDirectory,
     featureFlags,
     functions,
-    functionConfig: functionsWithConfig,
+    userFunctionConfig: userFunctionsWithConfig,
+    internalFunctionConfig: internalFunctionsWithConfig,
     importMap: importMapSpecifier,
     layers: deployConfig.layers,
   })

diff --git a/node/config.test.ts b/node/config.test.ts
@@ -184,7 +184,7 @@ test('Loads function paths from the in-source `config` function', async () => {
       path: '/user-func2',
     },
   ]
-  const result = await bundle([internalDirectory, userDirectory], distPath, declarations, {
+  const result = await bundle([userDirectory], internalDirectory, distPath, declarations, {
     basePath,
     configPath: join(internalDirectory, 'config.json'),
   })
@@ -202,9 +202,9 @@ test('Loads function paths from the in-source `config` function', async () => {
   expect(generatedFiles.includes(bundles[0].asset)).toBe(true)
 
   expect(routes.length).toBe(6)
-  expect(routes[0]).toEqual({ function: 'framework-func2', pattern: '^/framework-func2/?$' })
+  expect(routes[0]).toEqual({ function: 'framework-func2', generator: 'internalFunc', pattern: '^/framework-func2/?$' })
   expect(routes[1]).toEqual({ function: 'user-func2', pattern: '^/user-func2/?$' })
-  expect(routes[2]).toEqual({ function: 'framework-func1', pattern: '^/framework-func1/?$' })
+  expect(routes[2]).toEqual({ function: 'framework-func1', generator: 'internalFunc', pattern: '^/framework-func1/?$' })
   expect(routes[3]).toEqual({ function: 'user-func1', pattern: '^/user-func1/?$' })
   expect(routes[4]).toEqual({ function: 'user-func3', pattern: '^/user-func3/?$' })
   expect(routes[5]).toEqual({ function: 'user-func5', pattern: '^/user-func5/.*/?$' })