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

* fix(dev): reloading edge functions for child dependencies

* chore: add test

* refactor: if linter tells me to change this, I do it!

* fix: watch project directory for changes, even if there's no netlify toml

* refactor: remove some duplication

* refactor: extract multimap

* refactor: dont keep variables with same name as class variables

* refactor: create functionPaths closer to where the underlying data is updated

* refactor: remove useless initialisers

* chore: prettier

---------

Co-authored-by: Simon Knott <[email protected]>

Author: JGAntunes

src/lib/edge-functions/registry.ts

@@ -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'
@@ -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 {
@@ -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')
@@ -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({
@@ -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()
@@ -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
   }
 
   /**
@@ -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
     }
@@ -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() {

src/utils/multimap.ts

@@ -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) ?? []
+  }
+}

tests/integration/commands/dev/edge-functions.test.ts

@@ -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.
@@ -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 },