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

fix(dev): reloading edge functions for child dependencies #6276

Merged
merged 12 commits into from
Jan 10, 2024
123 changes: 75 additions & 48 deletions src/lib/edge-functions/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
watchDebounced,
isNodeError,
} from '../../utils/command-helpers.js'
import { MultiMap } from '../../utils/multimap.js'
import { getPathInProject } from '../settings.js'

import { INTERNAL_EDGE_FUNCTIONS_FOLDER } from './consts.js'
Expand All @@ -28,6 +29,8 @@ type EdgeFunctionEvent = 'buildError' | 'loaded' | 'reloaded' | 'reloading' | 'r
type Route = Omit<Manifest['routes'][0], 'pattern'> & { pattern: RegExp }
type RunIsolate = Awaited<ReturnType<typeof import('@netlify/edge-bundler').serve>>

type ModuleJson = ModuleGraph['modules'][number]

const featureFlags = { edge_functions_correct_order: true }

interface EdgeFunctionsRegistryOptions {
Expand All @@ -44,6 +47,38 @@ interface EdgeFunctionsRegistryOptions {
importMapFromTOML?: string
}

/**
* Helper method which, given a edge bundler graph module and an index of modules by path, traverses its dependency tree
* and returns an array of all of ist local dependencies
*/
function traverseLocalDependencies(
{ dependencies = [] }: ModuleJson,
modulesByPath: Map<string, ModuleJson>,
): string[] {
return dependencies.flatMap((dependency) => {
// We're interested in tracking local dependencies, so we only look at
// specifiers with the `file:` protocol.
if (
dependency.code === undefined ||
typeof dependency.code.specifier !== 'string' ||
!dependency.code.specifier.startsWith('file://')
) {
return []
}
const { specifier: dependencyURL } = dependency.code
const dependencyPath = fileURLToPath(dependencyURL)
const dependencyModule = modulesByPath.get(dependencyPath)

// No module indexed for this dependency
if (dependencyModule === undefined) {
return [dependencyPath]
}

// Keep traversing the child dependencies and return the current dependency path
return [...traverseLocalDependencies(dependencyModule, modulesByPath), dependencyPath]
})
}

export class EdgeFunctionsRegistry {
private buildError: Error | null = null
private bundler: typeof import('@netlify/edge-bundler')
Expand All @@ -52,19 +87,28 @@ export class EdgeFunctionsRegistry {
private importMapFromTOML?: string
private declarationsFromDeployConfig: Declaration[] = []
private declarationsFromTOML: Declaration[]
private dependencyPaths = new Map<string, string[]>()

// Mapping file URLs to names of functions that use them as dependencies.
private dependencyPaths = new MultiMap<string, string>()

private directories: string[]
private directoryWatchers = new Map<string, import('chokidar').FSWatcher>()
private env: Record<string, string>

private userFunctions: EdgeFunction[] = []
private internalFunctions: EdgeFunction[] = []

// a Map from `this.functions` that maps function paths to function
// names. This allows us to match modules against functions in O(1) time as
// opposed to O(n).
private functionPaths = new Map<string, string>()

private getUpdatedConfig: () => Promise<Config>
private initialScan: Promise<void>
private internalFunctions: EdgeFunction[] = []
private manifest: Manifest | null = null
private routes: Route[] = []
private runIsolate: RunIsolate
private servePath: string
private userFunctions: EdgeFunction[] = []
private projectDir: string

constructor({
Expand All @@ -91,13 +135,6 @@ export class EdgeFunctionsRegistry {
this.declarationsFromTOML = EdgeFunctionsRegistry.getDeclarationsFromTOML(config)
this.env = EdgeFunctionsRegistry.getEnvironmentVariables(env)

this.buildError = null
this.directoryWatchers = new Map()
this.dependencyPaths = new Map()
this.functionPaths = new Map()
this.userFunctions = []
this.internalFunctions = []

this.initialScan = this.doInitialScan()

this.setupWatchers()
Expand Down Expand Up @@ -416,50 +453,37 @@ export class EdgeFunctionsRegistry {
return
}

// Creating a Map from `this.functions` that maps function paths to function
// names. This allows us to match modules against functions in O(1) time as
// opposed to O(n).
// eslint-disable-next-line unicorn/prefer-spread
const functionPaths = new Map(Array.from(this.functions, (func) => [func.path, func.name]))
this.dependencyPaths = new MultiMap<string, string>()

// Mapping file URLs to names of functions that use them as dependencies.
const dependencyPaths = new Map<string, string[]>()
// Mapping file URLs to modules. Used by the traversal function.
const modulesByPath = new Map<string, ModuleJson>()

const { modules } = graph

modules.forEach(({ dependencies = [], specifier }) => {
// a set of edge function modules that we'll use to start traversing the dependency tree from
const functionModules = new Set<{ functionName: string; module: ModuleJson }>()
graph.modules.forEach((module) => {
// We're interested in tracking local dependencies, so we only look at
// specifiers with the `file:` protocol.
const { specifier } = module
if (!specifier.startsWith('file://')) {
return
}

const path = fileURLToPath(specifier)
const functionMatch = functionPaths.get(path)
modulesByPath.set(path, module)

if (!functionMatch) {
return
const functionName = this.functionPaths.get(path)
if (functionName) {
functionModules.add({ functionName, module })
}
})

dependencies.forEach((dependency) => {
// We're interested in tracking local dependencies, so we only look at
// specifiers with the `file:` protocol.
if (
dependency.code === undefined ||
typeof dependency.code.specifier !== 'string' ||
!dependency.code.specifier.startsWith('file://')
) {
return
}

const { specifier: dependencyURL } = dependency.code
const dependencyPath = fileURLToPath(dependencyURL)
const functions = dependencyPaths.get(dependencyPath) || []

dependencyPaths.set(dependencyPath, [...functions, functionMatch])
// We start from our functions and we traverse through their dependency tree
functionModules.forEach(({ functionName, module }) => {
const traversedPaths = traverseLocalDependencies(module, modulesByPath)
traversedPaths.forEach((dependencyPath) => {
this.dependencyPaths.add(dependencyPath, functionName)
})
})

this.dependencyPaths = dependencyPaths
this.functionPaths = functionPaths
}

/**
Expand Down Expand Up @@ -541,10 +565,19 @@ export class EdgeFunctionsRegistry {
this.internalFunctions = internalFunctions
this.userFunctions = userFunctions

// eslint-disable-next-line unicorn/prefer-spread
this.functionPaths = new Map(Array.from(this.functions, (func) => [func.path, func.name]))

return { all: functions, new: newFunctions, deleted: deletedFunctions }
}

private async setupWatchers() {
// While functions are guaranteed to be inside one of the configured
// directories, they might be importing files that are located in
// parent directories. So we watch the entire project directory for
// changes.
await this.setupWatcherForDirectory()

if (!this.configPath) {
return
}
Expand All @@ -560,12 +593,6 @@ export class EdgeFunctionsRegistry {
await this.checkForAddedOrDeletedFunctions()
},
})

// While functions are guaranteed to be inside one of the configured
// directories, they might be importing files that are located in
// parent directories. So we watch the entire project directory for
// changes.
await this.setupWatcherForDirectory()
}

private async setupWatcherForDirectory() {
Expand Down
11 changes: 11 additions & 0 deletions src/utils/multimap.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export class MultiMap<K, V> {
private map = new Map<K, V[]>()

add(key: K, value: V) {
this.map.set(key, [...(this.map.get(key) ?? []), value])
}

get(key: K): readonly V[] {
return this.map.get(key) ?? []
}
}
45 changes: 44 additions & 1 deletion tests/integration/commands/dev/edge-functions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ import { rename } from 'fs/promises'
import { join } from 'path'

import execa from 'execa'
import fetch from 'node-fetch'
import { describe, expect, expectTypeOf, test } from 'vitest'

import { withDevServer } from '../../utils/dev-server.js'
import { FixtureTestContext, setupFixtureTests } from '../../utils/fixture.js'
import fetch from 'node-fetch'
import { pause } from '../../utils/pause.js'
import { withSiteBuilder } from '../../utils/site-builder.js'

// Skipping tests on Windows because of an issue with the Deno CLI throwing IO
// errors when running in the CI.
Expand Down Expand Up @@ -164,6 +166,47 @@ describe.skipIf(isWindows)('edge functions', () => {
})
})

test('should reload on change to transitive dependency', async (t) => {
await withSiteBuilder(t, async (builder) => {
await builder
.withContentFile({
path: 'parent.js',
content: "export { foo } from './child.js'",
})
.withContentFile({
path: 'child.js',
content: "export const foo = 'foo'",
})
.withContentFile({
path: 'netlify/edge-functions/func.js',
content: `
import { foo } from '../../parent.js'
export default async () => new Response(foo)
export const config = { path: '/' }
`,
})
.build()

await withDevServer({ cwd: builder.directory }, async (server) => {
const response = await fetch(server.url, {}).then((res) => res.text())
t.expect(response).toEqual('foo')

// update file
await builder
.withContentFile({
path: 'child.js',
content: "export const foo = 'bar'",
})
.build()

await pause(500)

const response2 = await fetch(server.url, {}).then((res) => res.text())
t.expect(response2).toEqual('bar')
})
})
})

setupFixtureTests(
'dev-server-with-edge-functions-and-npm-modules',
{ devServer: true, mockApi: { routes }, setup },
Expand Down
Loading