Repository: netlify/cli
Pull request #5642 (2023-04-19 11:29:13)

fix: correctly reload edge functions on change
🎉 Thanks for submitting a pull request! 🎉

#### Summary

The problem was that when the handling of internal and user functions was split, the detection for added/deleted function broke. All edge functions were always removed on change, because when detecting internal functions, the user functions were marked as deleted, and vice versa.



Ref https://github.com/netlify/pillar-support/issues/379




github-actions[bot]: ## 📊 Benchmark results

*Comparing with 865e7a15f0ef717c7b65a7b9103e6138882cab4f*

### Package size: 302 MB
 ⬆️ **0.00% increase** vs. 865e7a15f0ef717c7b65a7b9103e6138882cab4f
```
^  302 MB  302 MB  302 MB  302 MB  302 MB  302 MB  302 MB  302 MB  302 MB  302 MB 
│   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
```
<details>
<summary>Legend</summary>

- T-9 (6c4d01eea377b42df51a87163e5c7b94a5aeccef): 302 MB
- T-8 (fb40e9579d84a2a3ef341ca83ad52db85db61eaf): 302 MB
- T-7 (23d191ad88a323b440ef0f401ebf76922d748202): 302 MB
- T-6 (7e305a9bfb27b08e6936fe8e51c610bc920b211e): 302 MB
- T-5 (f30d4092d0e5d3c8c2b5d81d50678c9346e6c8c3): 302 MB
- T-4 (9066166ea214cbb3714b1114f7216102671b2de4): 302 MB
- T-3 (053523090c044322ee980af4d4faa103f763e4d7): 302 MB
- T-2 (30d9a4fbd8e8a2985fc9c15e183701fad32cfe5a): 302 MB
- T-1 (865e7a15f0ef717c7b65a7b9103e6138882cab4f): 302 MB
- **T (current commit): 302 MB**</details>
<!--delta:post-package-size@{}-->
danez: I adapted the test and removed the HTTP calls, as they weren't necessary to reproduce.

Patch:
diff --git a/src/lib/edge-functions/registry.mjs b/src/lib/edge-functions/registry.mjs
index b8e8bd1e..2ab0cebb 100644
--- a/src/lib/edge-functions/registry.mjs
+++ b/src/lib/edge-functions/registry.mjs
@@ -122,12 +122,7 @@ export class EdgeFunctionsRegistry {
    * @returns {Promise<void>}
    */
   async #doInitialScan() {
-    const [internalFunctions, userFunctions] = await Promise.all([
-      this.#scanForFunctions(this.#internalDirectories),
-      this.#scanForFunctions(this.#directories),
-    ])
-    this.#internalFunctions = internalFunctions.all
-    this.#userFunctions = userFunctions.all
+    await this.#scanForFunctions()
 
     this.#functions.forEach((func) => {
       this.#logAddedFunction(func)
@@ -190,16 +185,7 @@ export class EdgeFunctionsRegistry {
    * @returns {Promise<void>}
    */
   async #checkForAddedOrDeletedFunctions() {
-    const [internalFunctions, userFunctions] = await Promise.all([
-      this.#scanForFunctions(this.#internalDirectories),
-      this.#scanForFunctions(this.#directories),
-    ])
-
-    this.#internalFunctions = internalFunctions.all
-    this.#userFunctions = userFunctions.all
-
-    const newFunctions = [...internalFunctions.new, ...userFunctions.new]
-    const deletedFunctions = [...internalFunctions.deleted, ...userFunctions.deleted]
+    const { deleted: deletedFunctions, new: newFunctions } = await this.#scanForFunctions()
 
     if (newFunctions.length === 0 && deletedFunctions.length === 0) {
       return
@@ -446,12 +432,16 @@ export class EdgeFunctionsRegistry {
   }
 
   /**
-   *
-   * @param {string[]} directories
    * @returns {Promise<{all: EdgeFunction[], new: EdgeFunction[], deleted: EdgeFunction[]}>}
    */
-  async #scanForFunctions(directories) {
-    const functions = await this.#bundler.find(directories)
+  async #scanForFunctions() {
+    const [internalFunctions, userFunctions] = await Promise.all([
+      this.#bundler.find(this.#internalDirectories),
+      this.#bundler.find(this.#directories),
+    ])
+
+    const functions = [...internalFunctions, ...userFunctions]
+
     const newFunctions = functions.filter((func) => {
       const functionExists = this.#functions.some(
         (existingFunc) => func.name === existingFunc.name && func.path === existingFunc.path,
@@ -467,6 +457,9 @@ export class EdgeFunctionsRegistry {
       return !functionExists
     })
 
+    this.#internalFunctions = internalFunctions
+    this.#userFunctions = userFunctions
+
     return { all: functions, new: newFunctions, deleted: deletedFunctions }
   }
 


Test patch:
diff --git a/tests/integration/commands/dev/edge-functions.test.ts b/tests/integration/commands/dev/edge-functions.test.ts
index 6e16d784..6ac7a98a 100644
--- a/tests/integration/commands/dev/edge-functions.test.ts
+++ b/tests/integration/commands/dev/edge-functions.test.ts
@@ -2,6 +2,7 @@ import { describe, expect, test } from 'vitest'
 
 import { FixtureTestContext, setupFixtureTests } from '../../utils/fixture.js'
 import got from '../../utils/got.cjs'
+import { pause } from '../../utils/pause.cjs'
 
 describe('edge functions', () => {
   setupFixtureTests('dev-server-with-edge-functions', { devServer: true }, () => {
@@ -15,4 +16,23 @@ describe('edge functions', () => {
       expect(response.body).toMatchSnapshot()
     })
   })
+
+  setupFixtureTests('dev-server-with-edge-functions', { devServer: true }, () => {
+    test<FixtureTestContext>('should not remove other edge functions on change', async ({ devServer, fixture }) => {
+      // we need to wait till file watchers are loaded
+      await pause(500)
+
+      await fixture.builder
+        .withEdgeFunction({
+          name: 'new',
+          handler: async (_, context) => new Response('hello'),
+          config: { path: ['/new'] },
+        })
+        .build()
+
+      await devServer.waitForLogMatching('Loaded edge function new')
+
+      expect(devServer.output).not.toContain('Removed edge function')
+    })
+  })
 })
diff --git a/tests/integration/commands/dev/scheduled-functions.test.ts b/tests/integration/commands/dev/scheduled-functions.test.ts
index 8f85d621..0c27cc13 100644
--- a/tests/integration/commands/dev/scheduled-functions.test.ts
+++ b/tests/integration/commands/dev/scheduled-functions.test.ts
@@ -38,7 +38,7 @@ describe('scheduled functions', () => {
             }
           })`,
         })
-        .buildAsync()
+        .build()
 
       const DETECT_FILE_CHANGE_DELAY = 500
       await pause(DETECT_FILE_CHANGE_DELAY)
diff --git a/tests/integration/utils/dev-server.cjs b/tests/integration/utils/dev-server.cjs
index 95870031..faa67069 100644
--- a/tests/integration/utils/dev-server.cjs
+++ b/tests/integration/utils/dev-server.cjs
@@ -119,11 +119,12 @@ const startDevServer = async (options, expectFailure) => {
   // eslint-disable-next-line fp/no-loops
   for (let attempt = 1; attempt <= maxAttempts; attempt++) {
     try {
-      const { timeout, ...server } = await startServer({ ...options, expectFailure })
-      if (timeout) {
-        throw new Error(`Timed out starting dev server.\nServer Output:\n${server.output}`)
+      // do not use destruction, as we use getters which otherwise would be evaluated here
+      const devServer = await startServer({ ...options, expectFailure })
+      if (devServer.timeout) {
+        throw new Error(`Timed out starting dev server.\nServer Output:\n${devServer.output}`)
       }
-      return server
+      return devServer
     } catch (error) {
       if (attempt === maxAttempts || expectFailure) {
         throw error
diff --git a/tests/integration/utils/site-builder.cjs b/tests/integration/utils/site-builder.cjs
index 28ef663f..21723fff 100644
--- a/tests/integration/utils/site-builder.cjs
+++ b/tests/integration/utils/site-builder.cjs
@@ -82,6 +82,11 @@ class SiteBuilder {
     return this
   }
 
+  /**
+   *
+   * @param {{config?:any, handler:any, internal?:boolean, name?:string, pathPrefix?:string}} param0
+   * @returns
+   */
   withEdgeFunction({ config, handler, internal = false, name = 'function', pathPrefix = '' }) {
     const edgeFunctionsDirectory = internal ? '.netlify/edge-functions' : 'netlify/edge-functions'
     const dest = path.join(this.directory, pathPrefix, edgeFunctionsDirectory, `${name}.js`)