Repository: netlify/cli
Pull request #5617 (2023-04-12 08:38:29)

fix: determine duplicate function precedence based on extension
🎉 Thanks for submitting a pull request! 🎉

#### Summary

Fixes https://github.com/netlify/pod-compute/issues/462

Right now, the cli doesn't behave the same as production when it comes to duplicate function precedence. This should fix it. For more detail check the issue1

---

For us to review and ship your PR efficiently, please perform the following steps:

- [x] Open a [bug/issue](https://github.com/netlify/cli/issues/new/choose) before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you\`re fixing a typo or something that\`s on fire 🔥 (e.g. incident related), you can skip this step.
- [x] Read the [contribution guidelines](../CONTRIBUTING.md) 📖. This ensures your code follows our style guide and
      passes our tests.
- [x] Update or add tests (if any source code was changed or added) 🧪
- [x] Update or add documentation (if features were changed or added) 📝
- [x] Make sure the status checks below are successful ✅

**A picture of a cute animal (not mandatory, but encouraged)**


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

*Comparing with 8bcc4c9b771d4ecdd9a7077a3791e94c8ca28cb9*

### Package size: 306 MB
 (no change)
```
^                                                                          302 MB  306 MB  306 MB  306 MB 
│                                                                           ┌──┐    ┌──┐    ┌──┐    ┌──┐  
│                                                          271 MB  271 MB   |  |    |  |    |  |    |▒▒|  
│ ─264 MB──264 MB──264 MB──264 MB──264 MB──264 MB──264 MB───┌──┐────┌──┐────┼──┼────┼──┼────┼──┼────|▒▒|──
│   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
```
<details>
<summary>Legend</summary>

- T-30 (4a4d2a321be2cf301b1e487af895b5756c1a75ed): 264 MB
- T-29 (93e6f025a5bf7ce911a8e142027db94e6396d2d7): 264 MB
- T-28 (be1f66d277ed88133df92dc579fd72d97edd31e3): 264 MB
- T-27 (e33790708d2d0b6e627a69194904cf3af6ac8514): 271 MB
- T-26 (694d0058548c355467a33c401dc8c2ce0192b3e7): 271 MB
- T-25 (e1185555cb37b9606ce55704e17c0655a1fb0425): 271 MB
- T-24 (618a41e52b14e47d5aadcbe0e4461fccaecc7512): 264 MB
- T-23 (f437d3049becf6579c105cdd7e7490871d062441): 264 MB
- T-22 (1311734b01fc732e98eafb77a8ee45bf85b961ec): 264 MB
- T-21 (6d468504e0903dd50d1e6e3a9e4dcd166db85c1b): 264 MB
- T-20 (b468d2f073a3478c17ba119cfc4897753c2cc830): 264 MB
- T-19 (e11d53fc1eb0417661a1dc0873f668a21088c5c8): 264 MB
- T-18 (3bf1eef911b0365a3601498ca605ac5fe55b7b62): 264 MB
- T-17 (c9710cfa348a6608d558283eb1a83ab24e7d1a95): 264 MB
- T-16 (247c71703ac52e15d1404bb08af9130d1efe2525): 271 MB
- T-15 (145b9ea9304dd543fbd5cd3727ee4717bc613d07): 264 MB
- T-14 (06dc9674617b7a5d74a856f829977a0a525d2f53): 264 MB
- T-13 (cd6f33dd6a7185b12663e2022a7fc94e48de88f7): 264 MB
- T-12 (fd4d3587157b0dff694bb36fffc03301cc5c85d2): 264 MB
- T-11 (ace82dda5eb948a8ba29ef1a32a8f3444f30f81c): 264 MB
- T-10 (f82421c91b04ed8d131f3350a14947228e01ac3d): 264 MB
- T-9 (0c6278d153d143af8a725fcba3b5e358601843f6): 264 MB
- T-8 (899c567fa0bd2c09cf4fe07f2b723484479d4f71): 264 MB
- T-7 (812664878d73c347a4221f3820c37d30ed8143bd): 264 MB
- T-6 (54ecf6d0cbc584b442ecdce0bd58b21f31347d6d): 264 MB
- T-5 (187d3a80cb1b8a69774c63f81d5e4c49a27d4e19): 271 MB
- T-4 (d0cddc91045518a14246fb5b83023b09e41d7ab4): 271 MB
- T-3 (c1cb4563be8ed9968501e7cd6db53b630b135406): 302 MB
- T-2 (9775e407a2b3fe84b54dad1f441dfd1e0ac78d23): 306 MB
- T-1 (8bcc4c9b771d4ecdd9a7077a3791e94c8ca28cb9): 306 MB
- **T (current commit): 306 MB**</details>
<!--delta:post-package-size@{}-->
khendrikse: I will!

Patch:
diff --git a/src/lib/functions/registry.mjs b/src/lib/functions/registry.mjs
index aea023fc..f4d1c1a0 100644
--- a/src/lib/functions/registry.mjs
+++ b/src/lib/functions/registry.mjs
@@ -212,7 +212,10 @@ export class FunctionsRegistry {
     await Promise.all(deletedFunctions.map((func) => this.unregisterFunction(func.name)))
 
     await Promise.all(
-      functions.map(async ({ displayName, mainFile, name, runtime: runtimeName }) => {
+      // zip-it-ship-it returns an array sorted based on which extension should have precedence.
+      // where the last ones precede the previous ones. This is why
+      // we reverse the array so we get the right functions precedence in the cli.
+      functions.reverse().map(async ({ displayName, mainFile, name, runtime: runtimeName }) => {
         const runtime = runtimes[runtimeName]
 
         // If there is no matching runtime, it means this function is not yet


Test patch:
diff --git a/tests/unit/lib/functions/registry.test.mjs b/tests/unit/lib/functions/registry.test.mjs
index 78b60167..2a44990d 100644
--- a/tests/unit/lib/functions/registry.test.mjs
+++ b/tests/unit/lib/functions/registry.test.mjs
@@ -1,8 +1,40 @@
-import { expect, test, vi } from 'vitest'
+import { mkdir, mkdtemp, writeFile } from 'fs/promises'
+import { tmpdir } from 'os'
+import { join } from 'path'
+
+import { describe, expect, test, vi } from 'vitest'
 
 import { FunctionsRegistry } from '../../../../src/lib/functions/registry.mjs'
 import { watchDebounced } from '../../../../src/utils/command-helpers.mjs'
 
+const duplicateFunctions = [
+  {
+    filename: 'hello.js',
+    content: `exports.handler = async (event) => ({ statusCode: 200, body: JSON.stringify({ message: 'Hello World from .js' }) })`,
+  },
+  {
+    filename: 'hello.ts',
+    content: `exports.handler = async (event) => ({ statusCode: 200, body: JSON.stringify({ message: 'Hello World from .ts' }) })`,
+  },
+  {
+    filename: 'hello2.js',
+    content: `exports.handler = async (event) => ({ statusCode: 200, body: JSON.stringify({ message: 'Hello World from .ts' }) })`,
+  },
+  {
+    filename: 'hello2/main.go',
+    subDir: 'hello2',
+    content: `package main
+    import (
+      "fmt"
+    )
+
+    func main() {
+      fmt.Println("Hello, world from a go function!")
+    }
+    `,
+  },
+]
+
 vi.mock('../../../../src/utils/command-helpers.mjs', async () => {
   const helpers = await vi.importActual('../../../../src/utils/command-helpers.mjs')
 
@@ -35,6 +67,40 @@ test('registry should only pass functions config to zip-it-and-ship-it', async (
   await prepareDirectoryScanStub.mockRestore()
 })
 
+describe('the registry handles duplicate functions based on extension precedence', () => {
+  test('where .js takes precedence over .go, and .go over .ts', async () => {
+    const projectRoot = await mkdtemp(join(tmpdir(), 'functions-extension-precedence'))
+    const functionsDirectory = join(projectRoot, 'functions')
+    await mkdir(functionsDirectory)
+
+    duplicateFunctions.forEach(async (func) => {
+      if (func.subDir) {
+        const subDir = join(functionsDirectory, func.subDir)
+        await mkdir(subDir)
+      }
+      const file = join(functionsDirectory, func.filename)
+      await writeFile(file, func.content)
+    })
+    const functionsRegistry = new FunctionsRegistry({
+      projectRoot,
+      config: {},
+      timeouts: { syncFunctions: 1, backgroundFunctions: 1 },
+      settings: { port: 8888 },
+    })
+    const prepareDirectoryScanStub = vi.spyOn(FunctionsRegistry, 'prepareDirectoryScan').mockImplementation(() => {})
+    const setupDirectoryWatcherStub = vi.spyOn(functionsRegistry, 'setupDirectoryWatcher').mockImplementation(() => {})
+
+    await functionsRegistry.scan([functionsDirectory])
+    const { functions } = functionsRegistry
+
+    expect(functions.get('hello').runtime.name).toBe('js')
+    expect(functions.get('hello2').runtime.name).toBe('go')
+
+    await setupDirectoryWatcherStub.mockRestore()
+    await prepareDirectoryScanStub.mockRestore()
+  })
+})
+
 test('should add included_files to watcher', async () => {
   const registry = new FunctionsRegistry({})
   const func = {