Repository: netlify/zip-it-and-ship-it
Pull request #374 (2021-03-12 11:02:29)

fix: make JS files take precedence over TS files with the same name
**- Summary**

When we started introducing support for TS files, the goal was for them to take precedence over JS functions with the same name. This was the case when the functions were in a sub-directory (e.g. `functions/sub-dir/function.js` vs. `functions/sub-dir/function.ts`). Unfortunately, this wasn't the case when the functions were at the root level (e.g. `functions/function.js` vs `functions/function.ts`). This PR addresses that.

**- Test plan**

- Added test to assert that `.js` files take precedence over `.ts` files
- Added test to assert that  `.js` files take precedence over `.zip` files (I verified this was the current behaviour) before we introduced these changes

**- Description for the changelog**

fix: make JS files take precedence over TS files with the same name

**- A picture of a cute animal (not mandatory but encouraged)**

![images](https://user-images.githubusercontent.com/4162329/110931488-3bc7ab00-8322-11eb-92ad-58a1fe8e4218.jpg)



Patch:
diff --git a/src/runtimes/node/finder.js b/src/runtimes/node/finder.js
index a6fca07..f76472f 100644
--- a/src/runtimes/node/finder.js
+++ b/src/runtimes/node/finder.js
@@ -6,19 +6,42 @@ const locatePath = require('locate-path')
 
 const pLstat = promisify(lstat)
 
-const allowedExtensions = new Set(['.cjs', '.js', '.mjs', '.ts', '.zip'])
+// List of extensions that this runtime will look for, in order of precedence.
+const allowedExtensions = ['.js', '.zip', '.cjs', '.mjs', '.ts']
+
+// Sorting function, compatible with the callback of Array.sort, which sorts
+// entries by extension according to their position in `allowedExtensions`.
+// It places extensions with a higher precedence last in the array, so that
+// they "win" when the array is flattened into a Map.
+const sortByExtension = (fA, fB) => {
+  const indexA = allowedExtensions.indexOf(fA.extension)
+  const indexB = allowedExtensions.indexOf(fB.extension)
+
+  return indexB - indexA
+}
 
 const findFunctionsInPaths = async function (paths) {
   const functions = await Promise.all(paths.map(getFunctionAtPath))
 
-  // We sort the functions array to put directories first. This is so that
-  // `{name}/{name}.js` takes precedence over `{name}.js`.
-  //
   // It's fine to mutate the array since its scope is local to this function.
   // eslint-disable-next-line fp/no-mutating-methods
-  const sortedFunctions = functions
-    .filter(Boolean)
-    .sort((fA, fB) => Number(fA.stat.isDirectory()) - Number(fB.stat.isDirectory()))
+  const sortedFunctions = functions.filter(Boolean).sort((fA, fB) => {
+    // We first sort the functions array to put directories first. This is so
+    // that `{name}/{name}.js` takes precedence over `{name}.js`.
+    const directorySort = Number(fA.stat.isDirectory()) - Number(fB.stat.isDirectory())
+
+    if (directorySort !== 0) {
+      return directorySort
+    }
+
+    // If the functions have the same name, we sort them according to the order
+    // defined in `allowedExtensions`.
+    if (fA.name === fB.name) {
+      return sortByExtension(fA, fB)
+    }
+
+    return 0
+  })
 
   return sortedFunctions
 }
@@ -37,10 +60,11 @@ const getFunctionAtPath = async function (srcPath) {
     return
   }
 
+  const extension = extname(srcPath)
   const srcDir = stat.isDirectory() ? srcPath : dirname(srcPath)
   const name = basename(srcPath, extname(srcPath))
 
-  return { mainFile, name, srcDir, srcPath, stat }
+  return { extension, mainFile, name, srcDir, srcPath, stat }
 }
 
 // Each `srcPath` can also be a directory with an `index` file or a file using
@@ -60,7 +84,7 @@ const getMainFile = function (srcPath, filename, stat) {
 
   const extension = extname(srcPath)
 
-  if (allowedExtensions.has(extension)) {
+  if (allowedExtensions.includes(extension)) {
     return srcPath
   }
 }


Test patch:
diff --git a/tests/fixtures/conflicting-names-5/function.js b/tests/fixtures/conflicting-names-5/function.js
new file mode 100644
index 0000000..895c249
--- /dev/null
+++ b/tests/fixtures/conflicting-names-5/function.js
@@ -0,0 +1,3 @@
+module.exports = {
+  type: 'function-js-file',
+}
diff --git a/tests/fixtures/conflicting-names-5/function.zip b/tests/fixtures/conflicting-names-5/function.zip
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/conflicting-names-5/function.zip
@@ -0,0 +1,1 @@
+test
diff --git a/tests/main.js b/tests/main.js
index 3c72f5d..345fa4c 100644
--- a/tests/main.js
+++ b/tests/main.js
@@ -402,8 +402,8 @@ testBundlers(
     t.deepEqual(
       functions,
       [
-        { name: 'test', mainFile: 'test.js', runtime: 'js', extension: '.js' },
         { name: 'test', mainFile: 'test.zip', runtime: 'js', extension: '.zip' },
+        { name: 'test', mainFile: 'test.js', runtime: 'js', extension: '.js' },
         { name: 'four', mainFile: 'four.js/four.js.js', runtime: 'js', extension: '.js' },
         { name: 'one', mainFile: 'one/index.js', runtime: 'js', extension: '.js' },
         { name: 'two', mainFile: 'two/two.js', runtime: 'js', extension: '.js' },
@@ -422,8 +422,8 @@ testBundlers(
     t.deepEqual(
       functions,
       [
-        { name: 'test', mainFile: 'test.js', runtime: 'js', extension: '.js', srcFile: 'test.js' },
         { name: 'test', mainFile: 'test.zip', runtime: 'js', extension: '.zip', srcFile: 'test.zip' },
+        { name: 'test', mainFile: 'test.js', runtime: 'js', extension: '.js', srcFile: 'test.js' },
         {
           name: 'four',
           mainFile: 'four.js/four.js.js',
@@ -605,6 +605,20 @@ testBundlers(
   },
 )
 
+testBundlers('{name}.js takes precedence over {name}.ts', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {
+  const { files, tmpDir } = await zipFixture(t, 'conflicting-names-4', { opts: { jsBundler: bundler } })
+  await unzipFiles(files)
+  // eslint-disable-next-line import/no-dynamic-require, node/global-require
+  t.is(require(`${tmpDir}/function.js`).type, 'function-js-file')
+})
+
+testBundlers('{name}.js takes precedence over {name}.zip', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {
+  const { files, tmpDir } = await zipFixture(t, 'conflicting-names-5', { opts: { jsBundler: bundler } })
+  await unzipFiles(files)
+  // eslint-disable-next-line import/no-dynamic-require, node/global-require
+  t.is(require(`${tmpDir}/function.js`).type, 'function-js-file')
+})
+
 testBundlers('Handles a TypeScript function ({name}.ts)', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {
   const { files, tmpDir } = await zipFixture(t, 'node-typescript', { opts: { jsBundler: bundler } })
   await unzipFiles(files)