Repository: netlify/cli
Pull request #5623 (2023-04-13 09:31:35)

fix: determine duplicate function precedence based on extension
🎉 Thanks for submitting a pull request! 🎉

#### Summary

This again implements function precedence based on extension by reversing the functions array so it behaves similar to how production works. I also had to reverse the way our sourcedirectories are passed.

---

For us to review and ship your PR efficiently, please perform the following steps:

- [ ] 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.
- [ ] Read the [contribution guidelines](../CONTRIBUTING.md) 📖. This ensures your code follows our style guide and
      passes our tests.
- [ ] Update or add tests (if any source code was changed or added) 🧪
- [ ] Update or add documentation (if features were changed or added) 📝
- [ ] 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 17605fc6071084bb2c0532913a50622ee8434707*

### Package size: 313 MB
 ⬇️ **0.01% decrease** vs. 17605fc6071084bb2c0532913a50622ee8434707
```
^                                  306 MB  306 MB  306 MB  306 MB  313 MB  313 MB  313 MB  313 MB  313 MB 
│                          302 MB   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐  
│ ──────────────────────────┌──┐────┼──┼────┼──┼────┼──┼────┼──┼────┼──┼────┼──┼────┼──┼────┼──┼────|▒▒|──
│  264 MB  271 MB  271 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 (618a41e52b14e47d5aadcbe0e4461fccaecc7512): 264 MB
- T-29 (f437d3049becf6579c105cdd7e7490871d062441): 264 MB
- T-28 (1311734b01fc732e98eafb77a8ee45bf85b961ec): 264 MB
- T-27 (6d468504e0903dd50d1e6e3a9e4dcd166db85c1b): 264 MB
- T-26 (b468d2f073a3478c17ba119cfc4897753c2cc830): 264 MB
- T-25 (e11d53fc1eb0417661a1dc0873f668a21088c5c8): 264 MB
- T-24 (3bf1eef911b0365a3601498ca605ac5fe55b7b62): 264 MB
- T-23 (c9710cfa348a6608d558283eb1a83ab24e7d1a95): 264 MB
- T-22 (247c71703ac52e15d1404bb08af9130d1efe2525): 271 MB
- T-21 (145b9ea9304dd543fbd5cd3727ee4717bc613d07): 264 MB
- T-20 (06dc9674617b7a5d74a856f829977a0a525d2f53): 264 MB
- T-19 (cd6f33dd6a7185b12663e2022a7fc94e48de88f7): 264 MB
- T-18 (fd4d3587157b0dff694bb36fffc03301cc5c85d2): 264 MB
- T-17 (ace82dda5eb948a8ba29ef1a32a8f3444f30f81c): 264 MB
- T-16 (f82421c91b04ed8d131f3350a14947228e01ac3d): 264 MB
- T-15 (0c6278d153d143af8a725fcba3b5e358601843f6): 264 MB
- T-14 (899c567fa0bd2c09cf4fe07f2b723484479d4f71): 264 MB
- T-13 (812664878d73c347a4221f3820c37d30ed8143bd): 264 MB
- T-12 (54ecf6d0cbc584b442ecdce0bd58b21f31347d6d): 264 MB
- T-11 (187d3a80cb1b8a69774c63f81d5e4c49a27d4e19): 271 MB
- T-10 (d0cddc91045518a14246fb5b83023b09e41d7ab4): 271 MB
- T-9 (c1cb4563be8ed9968501e7cd6db53b630b135406): 302 MB
- T-8 (9775e407a2b3fe84b54dad1f441dfd1e0ac78d23): 306 MB
- T-7 (8bcc4c9b771d4ecdd9a7077a3791e94c8ca28cb9): 306 MB
- T-6 (80b575e58136020d8532ae8f60657e41be5227b9): 306 MB
- T-5 (160e8a08ee0304ac801e67e6f5b3e428994af839): 306 MB
- T-4 (219dd3cc590771d3e383f91d8059c15768f26506): 313 MB
- T-3 (8ae14561c600c86f3a69657eb641ac5431419d88): 313 MB
- T-2 (e91051e8d0838c0f675b8a69a46ff45dae304b39): 313 MB
- T-1 (17605fc6071084bb2c0532913a50622ee8434707): 313 MB
- **T (current commit): 313 MB**</details>
<!--delta:post-package-size@{}-->

Patch:
diff --git a/src/lib/functions/registry.mjs b/src/lib/functions/registry.mjs
index aea023fc..ff8e5772 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-and-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
diff --git a/src/lib/functions/server.mjs b/src/lib/functions/server.mjs
index c630a83a..af43794e 100644
--- a/src/lib/functions/server.mjs
+++ b/src/lib/functions/server.mjs
@@ -222,9 +222,9 @@ export const startFunctionsServer = async (options) => {
       functionsDirectories.push(distPath)
     }
   } else {
-    // The order of the function directories matters. Leftmost directories take
+    // The order of the function directories matters. Rightmost directories take
     // precedence.
-    const sourceDirectories = [settings.functions, internalFunctionsDir].filter(Boolean)
+    const sourceDirectories = [internalFunctionsDir, settings.functions].filter(Boolean)
 
     functionsDirectories.push(...sourceDirectories)
   }


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 = {