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: populate generator field if edge function is from a config file #312

Merged
merged 3 commits into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 9 additions & 3 deletions node/bundler.test.ts
Expand Up @@ -331,20 +331,26 @@ test('Loads declarations and import maps from the deploy configuration', async (
const directories = [join(basePath, 'netlify', 'edge-functions'), join(basePath, '.netlify', 'edge-functions')]
const result = await bundle(directories, distPath, declarations, {
basePath,
configPath: join(basePath, '.netlify', 'edge-functions', 'config.json'),
configPath: join(basePath, '.netlify', 'edge-functions', 'manifest.json'),
internalSrcFolder: directories[1],
featureFlags: { edge_functions_config_export: true },
})
const generatedFiles = await fs.readdir(distPath)

expect(result.functions.length).toBe(2)
expect(result.functions.length).toBe(3)
expect(generatedFiles.length).toBe(2)

const manifestFile = await fs.readFile(resolve(distPath, 'manifest.json'), 'utf8')
const manifest = JSON.parse(manifestFile)
const { bundles, function_config: functionConfig } = manifest
const { routes, bundles, function_config: functionConfig } = manifest

expect(bundles.length).toBe(1)
expect(bundles[0].format).toBe('eszip2')
expect(generatedFiles.includes(bundles[0].asset)).toBe(true)
expect(routes[0].generator).toBeUndefined()
expect(routes[1].name).toBe('Function two')
expect(routes[1].generator).toBe('@netlify/fake-plugin@1.0.0')
expect(routes[2].generator).toBe('internalFunc')

// respects excludedPath from deploy config
expect(functionConfig.func2).toEqual({ excluded_patterns: ['^/func2/skip/?$'] })
Expand Down
32 changes: 29 additions & 3 deletions node/bundler.ts
@@ -1,7 +1,8 @@
import { promises as fs } from 'fs'
import { join } from 'path'
import { join, resolve } from 'path'

import commonPathPrefix from 'common-path-prefix'
import isPathInside from 'is-path-inside'
import { v4 as uuidv4 } from 'uuid'

import { importMapSpecifier } from '../shared/consts.js'
Expand All @@ -11,6 +12,7 @@ import type { Bundle } from './bundle.js'
import { FunctionConfig, getFunctionConfig } from './config.js'
import { Declaration, getDeclarationsFromConfig } from './declaration.js'
import { load as loadDeployConfig } from './deploy_config.js'
import { EdgeFunction } from './edge_function.js'
import { FeatureFlags, getFlags } from './feature_flags.js'
import { findFunctions } from './finder.js'
import { bundle as bundleESZIP } from './formats/eszip.js'
Expand All @@ -30,6 +32,7 @@ interface BundleOptions {
onAfterDownload?: OnAfterDownloadHook
onBeforeDownload?: OnBeforeDownloadHook
systemLogger?: LogFunction
internalSrcFolder?: string
}

const bundle = async (
Expand All @@ -47,6 +50,7 @@ const bundle = async (
onAfterDownload,
onBeforeDownload,
systemLogger,
internalSrcFolder,
}: BundleOptions = {},
) => {
const logger = getLogger(systemLogger, debug)
Expand All @@ -58,7 +62,7 @@ const bundle = async (
onAfterDownload,
onBeforeDownload,
}

const internalFunctionsPath = internalSrcFolder && resolve(internalSrcFolder)
if (cacheDirectory !== undefined) {
options.denoDir = join(cacheDirectory, 'deno_dir')
}
Expand Down Expand Up @@ -113,7 +117,15 @@ const bundle = async (

// Creating a final declarations array by combining the TOML file with the
// deploy configuration API and the in-source configuration.
const declarations = getDeclarationsFromConfig(tomlDeclarations, functionsWithConfig, deployConfig)
const declarationsFromConfig = getDeclarationsFromConfig(tomlDeclarations, functionsWithConfig, deployConfig)

// If any declarations are autogenerated and are missing the generator field
// add a default string.
const declarations = internalFunctionsPath
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I find the ternary operator a necessary evil because it's a bit hard to read, and in this case I don't think it's necessary. Personally, I would rather do something like:

const declarations = declarationsFromConfig.map((declaration) => {
  if (internalFunctionsPath) {
    return addGeneratorFieldIfMissing(declaration, functions, internalFunctionsPath)
  }

  return declaration
)

Deferring to you, though.

? declarationsFromConfig.map((declaration) =>
addGeneratorFieldIfMissing(declaration, functions, internalFunctionsPath),
)
: declarationsFromConfig

const manifest = await writeManifest({
bundles: [functionBundle],
Expand Down Expand Up @@ -159,5 +171,19 @@ const getBasePath = (sourceDirectories: string[], inputBasePath?: string) => {
return commonPathPrefix(sourceDirectories)
}

export const addGeneratorFieldIfMissing = (
declaration: Declaration,
functions: EdgeFunction[],
internalFunctionsPath?: string,
) => {
const fullFuncPath = functions?.find((func) => func.name === declaration.function)?.path

// If function path is in the internalFunctionsPath, we assume it is autogenerated.
const isInternal = Boolean(internalFunctionsPath && fullFuncPath && isPathInside(fullFuncPath, internalFunctionsPath))

const generatorFallback = isInternal ? 'internalFunc' : undefined
return { ...declaration, generator: declaration.generator || generatorFallback }
}

export { bundle }
export type { BundleOptions }
1 change: 1 addition & 0 deletions node/deploy_config.test.ts
Expand Up @@ -34,6 +34,7 @@ test('Returns a config object with declarations, layers, and import map', async
{
function: 'func1',
path: '/func1',
generator: 'internalFunc',
},
],
layers: [
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Expand Up @@ -14,6 +14,7 @@
],
"scripts": {
"build": "tsc",
"build:dev": "tsc -w",
"prepare": "husky install node_modules/@netlify/eslint-config-node/.husky/",
"prepublishOnly": "npm ci && npm test",
"prepack": "npm run build",
Expand Down Expand Up @@ -84,6 +85,7 @@
"find-up": "^6.3.0",
"get-port": "^6.1.2",
"glob-to-regexp": "^0.4.1",
"is-path-inside": "^4.0.0",
"jsonc-parser": "^3.2.0",
"node-fetch": "^3.1.1",
"node-stream-zip": "^1.15.0",
Expand Down
@@ -0,0 +1,9 @@
import { Config } from 'https://edge.netlify.com'

export default async () => {
return new Response('Hello world')
}

export const config: Config = {
path: '/func-3',
}
Expand Up @@ -3,6 +3,8 @@
{
"function": "func2",
"path": "/func2/*",
"name": "Function two",
"generator": "@netlify/fake-plugin@1.0.0",
"excludedPath": "/func2/skip"
}
],
Expand Down