Repository: netlify/build
Pull request #3942 (2021-12-14 15:25:33)

chore!: use pure ES modules with `functions-utils`
Part of https://github.com/netlify/build/issues/3742

This switches `functions-utils` from CommonJS to pure ES modules.

---

For us to review and ship your PR efficiently, please perform the following steps:

- [x] Open a [bug/issue](https://github.com/netlify/build/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 ✅


Patch:
diff --git a/.eslintrc.js b/.eslintrc.js
index 7a7cee90a..57a9b63ab 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -102,7 +102,11 @@ module.exports = {
 
     // Those packages are using pure ES modules
     {
-      files: ['packages/git-utils/**/*.{cjs,mjs,js}', 'packages/run-utils/**/*.{cjs,mjs,js}'],
+      files: [
+        'packages/git-utils/**/*.{cjs,mjs,js}',
+        'packages/functions-utils/**/*.{cjs,mjs,js}',
+        'packages/run-utils/**/*.{cjs,mjs,js}',
+      ],
       parserOptions: {
         sourceType: 'module',
       },
diff --git a/packages/build/src/plugins/child/utils.js b/packages/build/src/plugins/child/utils.js
index 8e200834d..90dfa5991 100644
--- a/packages/build/src/plugins/child/utils.js
+++ b/packages/build/src/plugins/child/utils.js
@@ -1,7 +1,6 @@
 'use strict'
 
 const { bindOpts: cacheBindOpts } = require('@netlify/cache-utils')
-const { add: functionsAdd, list: functionsList, listAll: functionsListAll } = require('@netlify/functions-utils')
 
 const { failBuild, failPlugin, cancelBuild, failPluginWithWarning } = require('../error')
 const { isSoftFailEvent } = require('../events')
@@ -10,17 +9,22 @@ const { addLazyProp } = require('./lazy')
 const { show } = require('./status')
 
 const gitUtilsPromise = import('@netlify/git-utils')
+const functionsUtilsPromise = import('@netlify/functions-utils')
 const runUtilsPromise = import('@netlify/run-utils')
 
 // Retrieve the `utils` argument.
 const getUtils = async function ({ event, constants: { FUNCTIONS_SRC, INTERNAL_FUNCTIONS_SRC, CACHE_DIR }, runState }) {
-  const [{ getGitUtils }, { run, runCommand }] = await Promise.all([gitUtilsPromise, runUtilsPromise])
+  const [functionsUtils, { getGitUtils }, { run, runCommand }] = await Promise.all([
+    functionsUtilsPromise,
+    gitUtilsPromise,
+    runUtilsPromise,
+  ])
   // eslint-disable-next-line fp/no-mutation
   run.command = runCommand
 
   const build = getBuildUtils(event)
   const cache = getCacheUtils(CACHE_DIR)
-  const functions = getFunctionsUtils(FUNCTIONS_SRC, INTERNAL_FUNCTIONS_SRC)
+  const functions = getFunctionsUtils(functionsUtils, FUNCTIONS_SRC, INTERNAL_FUNCTIONS_SRC)
   const status = getStatusUtils(runState)
   const utils = { build, cache, run, functions, status }
   addLazyProp(utils, 'git', () => getGitUtils())
@@ -43,11 +47,11 @@ const getCacheUtils = function (CACHE_DIR) {
   return cacheBindOpts({ cacheDir: CACHE_DIR })
 }
 
-const getFunctionsUtils = function (FUNCTIONS_SRC, INTERNAL_FUNCTIONS_SRC) {
+const getFunctionsUtils = function (functionsUtils, FUNCTIONS_SRC, INTERNAL_FUNCTIONS_SRC) {
   const functionsDirectories = [INTERNAL_FUNCTIONS_SRC, FUNCTIONS_SRC].filter(Boolean)
-  const add = (src) => functionsAdd(src, INTERNAL_FUNCTIONS_SRC, { fail: failBuild })
-  const list = functionsList.bind(null, functionsDirectories, { fail: failBuild })
-  const listAll = functionsListAll.bind(null, functionsDirectories, { fail: failBuild })
+  const add = (src) => functionsUtils.add(src, INTERNAL_FUNCTIONS_SRC, { fail: failBuild })
+  const list = functionsUtils.list.bind(null, functionsDirectories, { fail: failBuild })
+  const listAll = functionsUtils.listAll.bind(null, functionsDirectories, { fail: failBuild })
   return { add, list, listAll }
 }
 
diff --git a/packages/functions-utils/package.json b/packages/functions-utils/package.json
index 2f2a7dbc4..5d6687811 100644
--- a/packages/functions-utils/package.json
+++ b/packages/functions-utils/package.json
@@ -2,7 +2,9 @@
   "name": "@netlify/functions-utils",
   "version": "3.0.3",
   "description": "Utility for adding Functions files in Netlify Build",
-  "main": "src/main.js",
+  "type": "module",
+  "exports": "./src/main.js",
+  "main": "./src/main.js",
   "files": [
     "src/**/*.js"
   ],
diff --git a/packages/functions-utils/src/main.js b/packages/functions-utils/src/main.js
index 6b8f2ba46..88884a7b6 100644
--- a/packages/functions-utils/src/main.js
+++ b/packages/functions-utils/src/main.js
@@ -1,18 +1,16 @@
-'use strict'
+import { stat } from 'fs'
+import { basename, dirname } from 'path'
+import { promisify } from 'util'
 
-const { stat } = require('fs')
-const { basename, dirname } = require('path')
-const { promisify } = require('util')
-
-const { listFunctions, listFunctionsFiles } = require('@netlify/zip-it-and-ship-it')
-const cpy = require('cpy')
-const pathExists = require('path-exists')
+import { listFunctions, listFunctionsFiles } from '@netlify/zip-it-and-ship-it'
+import cpy from 'cpy'
+import pathExists from 'path-exists'
 
 const pStat = promisify(stat)
 
 // Add a Netlify Function file to the `functions` directory so it is processed
 // by `@netlify/plugin-functions-core`
-const add = async function (src, dist, { fail = defaultFail } = {}) {
+export const add = async function (src, dist, { fail = defaultFail } = {}) {
   if (src === undefined) {
     return fail('No function source directory was specified')
   }
@@ -40,7 +38,7 @@ const getSrcGlob = async function (src, srcBasename) {
   return srcBasename
 }
 
-const list = async function (functionsSrc, { fail = defaultFail } = {}) {
+export const list = async function (functionsSrc, { fail = defaultFail } = {}) {
   if (functionsSrc === undefined || functionsSrc.length === 0) {
     return fail('No function directory was specified')
   }
@@ -52,7 +50,7 @@ const list = async function (functionsSrc, { fail = defaultFail } = {}) {
   }
 }
 
-const listAll = async function (functionsSrc, { fail = defaultFail } = {}) {
+export const listAll = async function (functionsSrc, { fail = defaultFail } = {}) {
   if (functionsSrc === undefined || functionsSrc.length === 0) {
     return fail('No function directory was specified')
   }
@@ -67,5 +65,3 @@ const listAll = async function (functionsSrc, { fail = defaultFail } = {}) {
 const defaultFail = function (message) {
   throw new Error(message)
 }
-
-module.exports = { add, list, listAll }


Test patch:
diff --git a/packages/functions-utils/tests/fixtures/directory/test/index.js b/packages/functions-utils/tests/fixtures/directory/test/index.js
index 60ae42e4e..49cc1ebe1 100644
--- a/packages/functions-utils/tests/fixtures/directory/test/index.js
+++ b/packages/functions-utils/tests/fixtures/directory/test/index.js
@@ -1,3 +1,1 @@
-'use strict'
-
-module.exports = () => {}
+export default function test() {}
diff --git a/packages/functions-utils/tests/fixtures/file/test.js b/packages/functions-utils/tests/fixtures/file/test.js
deleted file mode 100644
index 5ebfe3bd8..000000000
--- a/packages/functions-utils/tests/fixtures/file/test.js
+++ /dev/null
@@ -1,3 +0,0 @@
-'use strict'
-
-module.exports = () => 'one'
diff --git a/packages/functions-utils/tests/fixtures/file/test.mjs b/packages/functions-utils/tests/fixtures/file/test.mjs
new file mode 100644
index 000000000..15198ffc9
--- /dev/null
+++ b/packages/functions-utils/tests/fixtures/file/test.mjs
@@ -0,0 +1,3 @@
+export const func1 = function () {
+  return 'one'
+}
diff --git a/packages/functions-utils/tests/fixtures/file/test_2.js b/packages/functions-utils/tests/fixtures/file/test_2.js
deleted file mode 100644
index bcd9fd164..000000000
--- a/packages/functions-utils/tests/fixtures/file/test_2.js
+++ /dev/null
@@ -1,3 +0,0 @@
-'use strict'
-
-module.exports = () => 'two'
diff --git a/packages/functions-utils/tests/fixtures/file/test_2.mjs b/packages/functions-utils/tests/fixtures/file/test_2.mjs
new file mode 100644
index 000000000..4e4a1e121
--- /dev/null
+++ b/packages/functions-utils/tests/fixtures/file/test_2.mjs
@@ -0,0 +1,3 @@
+export const func2 = function () {
+  return 'two'
+}
diff --git a/packages/functions-utils/tests/fixtures/list/four.js/four.js.js b/packages/functions-utils/tests/fixtures/list/four.js/four.js.js
index 710c8b9fd..9d05dedbf 100644
--- a/packages/functions-utils/tests/fixtures/list/four.js/four.js.js
+++ b/packages/functions-utils/tests/fixtures/list/four.js/four.js.js
@@ -1,3 +1,2 @@
-'use strict'
-
-module.exports = true
+const value = true
+export default value
diff --git a/packages/functions-utils/tests/fixtures/list/one/index.js b/packages/functions-utils/tests/fixtures/list/one/index.js
index 710c8b9fd..9d05dedbf 100644
--- a/packages/functions-utils/tests/fixtures/list/one/index.js
+++ b/packages/functions-utils/tests/fixtures/list/one/index.js
@@ -1,3 +1,2 @@
-'use strict'
-
-module.exports = true
+const value = true
+export default value
diff --git a/packages/functions-utils/tests/fixtures/list/test.js b/packages/functions-utils/tests/fixtures/list/test.js
index 710c8b9fd..9d05dedbf 100644
--- a/packages/functions-utils/tests/fixtures/list/test.js
+++ b/packages/functions-utils/tests/fixtures/list/test.js
@@ -1,3 +1,2 @@
-'use strict'
-
-module.exports = true
+const value = true
+export default value
diff --git a/packages/functions-utils/tests/fixtures/list/two/three.js b/packages/functions-utils/tests/fixtures/list/two/three.js
index 710c8b9fd..5aaa1f89b 100644
--- a/packages/functions-utils/tests/fixtures/list/two/three.js
+++ b/packages/functions-utils/tests/fixtures/list/two/three.js
@@ -1,3 +1,1 @@
-'use strict'
-
-module.exports = true
+export const value = true
diff --git a/packages/functions-utils/tests/fixtures/list/two/two.js b/packages/functions-utils/tests/fixtures/list/two/two.js
index 2fec5f155..00e430495 100644
--- a/packages/functions-utils/tests/fixtures/list/two/two.js
+++ b/packages/functions-utils/tests/fixtures/list/two/two.js
@@ -1,3 +1,1 @@
-'use strict'
-
-module.exports = require('./three')
+export { value as default } from './three.js'
diff --git a/packages/functions-utils/tests/helpers/main.js b/packages/functions-utils/tests/helpers/main.js
index 666e92870..ff5be4d6f 100644
--- a/packages/functions-utils/tests/helpers/main.js
+++ b/packages/functions-utils/tests/helpers/main.js
@@ -1,15 +1,13 @@
-'use strict'
-
-const del = require('del')
-const { tmpName, dir: tmpDir } = require('tmp-promise')
+import del from 'del'
+import { tmpName, dir as tmpDir } from 'tmp-promise'
 
 // Retrieve name of a temporary directory
-const getDist = function () {
+export const getDist = function () {
   return tmpName({ prefix: PREFIX })
 }
 
 // Create temporary directory
-const createDist = async function () {
+export const createDist = async function () {
   const { path } = await tmpDir({ prefix: PREFIX })
   return path
 }
@@ -17,8 +15,6 @@ const createDist = async function () {
 const PREFIX = 'test-functions-utils-'
 
 // Remove temporary directory
-const removeDist = async function (dir) {
+export const removeDist = async function (dir) {
   await del(dir, { force: true })
 }
-
-module.exports = { getDist, createDist, removeDist }
diff --git a/packages/functions-utils/tests/main.js b/packages/functions-utils/tests/main.mjs
similarity index 74%
rename from packages/functions-utils/tests/main.js
rename to packages/functions-utils/tests/main.mjs
index 265928537..0779e0f66 100644
--- a/packages/functions-utils/tests/main.js
+++ b/packages/functions-utils/tests/main.mjs
@@ -1,25 +1,24 @@
-'use strict'
+import { normalize } from 'path'
+import { fileURLToPath, pathToFileURL } from 'url'
 
-const { normalize } = require('path')
+import test from 'ava'
+import cpy from 'cpy'
+import del from 'del'
+import pathExists from 'path-exists'
+import { spy } from 'sinon'
+import sortOn from 'sort-on'
 
-const test = require('ava')
-const cpy = require('cpy')
-const del = require('del')
-const pathExists = require('path-exists')
-const { spy } = require('sinon')
-const sortOn = require('sort-on')
+import { add, list, listAll } from '../src/main.js'
 
-const { add, list, listAll } = require('..')
+import { getDist, createDist, removeDist } from './helpers/main.js'
 
-const { getDist, createDist, removeDist } = require('./helpers/main')
-
-const FIXTURES_DIR = `${__dirname}/fixtures`
+const FIXTURES_DIR = fileURLToPath(new URL('fixtures', import.meta.url))
 
 test('Should copy a source file to a dist directory', async (t) => {
   const dist = await getDist()
   try {
-    await add(`${FIXTURES_DIR}/file/test.js`, dist)
-    t.true(await pathExists(`${dist}/test.js`))
+    await add(`${FIXTURES_DIR}/file/test.mjs`, dist)
+    t.true(await pathExists(`${dist}/test.mjs`))
   } finally {
     await removeDist(dist)
   }
@@ -63,14 +62,14 @@ test('Should throw when source points to non-existing file', async (t) => {
 })
 
 test('Should throw when dist is undefined', async (t) => {
-  await t.throwsAsync(add(`${FIXTURES_DIR}/file/test.js`))
+  await t.throwsAsync(add(`${FIXTURES_DIR}/file/test.mjs`))
 })
 
 test('Should copy a source file even if dist directory already exists', async (t) => {
   const dist = await createDist()
   try {
-    await add(`${FIXTURES_DIR}/file/test.js`, dist)
-    t.true(await pathExists(`${dist}/test.js`))
+    await add(`${FIXTURES_DIR}/file/test.mjs`, dist)
+    t.true(await pathExists(`${dist}/test.mjs`))
   } finally {
     await removeDist(dist)
   }
@@ -80,27 +79,23 @@ test('Should overwrite dist file if it already exists', async (t) => {
   const dist = await getDist()
   const fixtureDir = `${FIXTURES_DIR}/file`
 
-  await cpy(`${fixtureDir}/test.js`, fixtureDir, { rename: 'test.js.backup' })
+  await cpy(`${fixtureDir}/test.mjs`, fixtureDir, { rename: 'test.mjs.backup' })
 
   try {
-    await add(`${fixtureDir}/test.js`, dist)
-
-    // eslint-disable-next-line import/no-dynamic-require, node/global-require
-    const func1 = require(`${dist}/test.js`)
+    await add(`${fixtureDir}/test.mjs`, dist)
 
-    await cpy(`${fixtureDir}/test_2.js`, fixtureDir, { rename: 'test.js' })
-    await add(`${fixtureDir}/test.js`, dist)
+    const { func1 } = await import(`${pathToFileURL(`${dist}/test.mjs`).href}?one`)
 
-    delete require.cache[require.resolve(`${dist}/test.js`)]
+    await cpy(`${fixtureDir}/test_2.mjs`, fixtureDir, { rename: 'test.mjs' })
+    await add(`${fixtureDir}/test.mjs`, dist)
 
-    // eslint-disable-next-line import/no-dynamic-require, node/global-require
-    const func2 = require(`${dist}/test.js`)
+    const { func2 } = await import(`${pathToFileURL(`${dist}/test.mjs`).href}?two`)
 
     t.is(func1(), 'one')
     t.is(func2(), 'two')
   } finally {
-    await cpy(`${fixtureDir}/test.js.backup`, fixtureDir, { rename: 'test.js' })
-    await del(`${fixtureDir}/test.js.backup`, { force: true })
+    await cpy(`${fixtureDir}/test.mjs.backup`, fixtureDir, { rename: 'test.mjs' })
+    await del(`${fixtureDir}/test.mjs.backup`, { force: true })
     await removeDist(dist)
   }
 })