feat: use esbuild by default for es modules, if detected (#625)

* feat: poc for es module detection

* fix: async calls to detect fn

* fix(tests): remove invalid test case

* fix: logical typo

Co-authored-by: ehmicky <ehmicky@users.noreply.github.com>

* test: assert that only imports and only exports work

* test: added a failing test that I think should pass

* fix(test): quirk in `requireExtractedFiles`

* refactor: addressing code review

* feat: surround in a feature flag

* fix: sadly, I don't think I can use ?. just yet

* chore: clean up as part of code review

Co-Authored-By: Eduardo Bouças <mail@eduardoboucas.com>

* fix: accidentally checked in test code

Co-authored-by: ehmicky <ehmicky@users.noreply.github.com>
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

README.md

@@ -412,7 +412,7 @@ These are supplied to each of the entrypoint functions (`zipFunction`, `zipFunct
 `listFunctionsFiles`) as a named parameter called `featureFlags`. It consists of an object where each key is the name of
 a feature flag and the values are Booleans indicating whether each feature flag is enabled or disabled.
 
-The list of all feature flags currently being used can be found at [here](src/feature_flags.js).
+The list of all feature flags currently being used can be found [here](src/feature_flags.js).
 
 # Troubleshooting
 

package.json

@@ -63,6 +63,7 @@
     "del": "^6.0.0",
     "elf-cam": "^0.1.1",
     "end-of-stream": "^1.4.4",
+    "es-module-lexer": "^0.7.1",
     "execa": "^5.0.0",
     "filter-obj": "^2.0.1",
     "find-up": "^5.0.0",

src/feature_flags.js

@@ -4,6 +4,7 @@ const { env } = require('process')
 const FLAGS = {
   buildGoSource: Boolean(env.NETLIFY_EXPERIMENTAL_BUILD_GO_SOURCE),
   buildRustSource: Boolean(env.NETLIFY_EXPERIMENTAL_BUILD_RUST_SOURCE),
+  defaultEsModulesToEsbuild: Boolean(env.NETLIFY_EXPERIMENTAL_DEFAULT_ES_MODULES_TO_ESBUILD),
 }
 
 const getFlags = (input = {}, flags = FLAGS) =>

src/runtimes/node/detect_es_module.js

@@ -0,0 +1,24 @@
+const { readFile } = require('fs')
+const { promisify } = require('util')
+
+const pReadFile = promisify(readFile)
+
+const { init, parse } = require('es-module-lexer')
+
+const detectEsModule = async ({ mainFile }) => {
+  if (!mainFile) {
+    return false
+  }
+
+  try {
+    const [mainFileContents] = await Promise.all([pReadFile(mainFile, 'utf8'), init])
+    const [imports, exports] = parse(mainFileContents)
+
+    return imports.length !== 0 || exports.length !== 0
+  } catch {
+    // If there are any problems with init or parsing, assume it's not an ES module
+    return false
+  }
+}
+
+module.exports = { detectEsModule }

src/runtimes/node/index.js

@@ -4,20 +4,34 @@ const cpFile = require('cp-file')
 
 const { JS_BUNDLER_ESBUILD, JS_BUNDLER_ESBUILD_ZISI, JS_BUNDLER_ZISI, RUNTIME_JS } = require('../../utils/consts')
 
+const { detectEsModule } = require('./detect_es_module')
 const { findFunctionsInPaths } = require('./finder')
 const { getSrcFiles } = require('./src_files')
 const { zipEsbuild } = require('./zip_esbuild')
 const { zipZisi } = require('./zip_zisi')
 
 // We use ZISI as the default bundler, except for certain extensions, for which
 // esbuild is the only option.
-const getDefaultBundler = ({ extension }) =>
-  ['.mjs', '.ts'].includes(extension) ? JS_BUNDLER_ESBUILD : JS_BUNDLER_ZISI
+const getDefaultBundler = async ({ extension, mainFile, featureFlags = {} }) => {
+  if (['.mjs', '.ts'].includes(extension)) {
+    return JS_BUNDLER_ESBUILD
+  }
+
+  if (featureFlags.defaultEsModulesToEsbuild) {
+    const isEsModule = await detectEsModule({ mainFile })
+
+    if (isEsModule) {
+      return JS_BUNDLER_ESBUILD
+    }
+  }
+
+  return JS_BUNDLER_ZISI
+}
 
 // A proxy for the `getSrcFiles` function which adds a default `bundler` using
 // the `getDefaultBundler` function.
-const getSrcFilesWithBundler = (parameters) => {
-  const bundler = parameters.config.nodeBundler || getDefaultBundler({ extension: parameters.extension })
+const getSrcFilesWithBundler = async (parameters) => {
+  const bundler = parameters.config.nodeBundler || (await getDefaultBundler({ extension: parameters.extension }))
 
   return getSrcFiles({ ...parameters, bundler })
 }
@@ -35,9 +49,9 @@ const zipFunction = async function ({
   srcDir,
   srcPath,
   stat,
+  featureFlags,
 }) {
-  const bundler = config.nodeBundler || getDefaultBundler({ extension })
-
+  const bundler = config.nodeBundler || (await getDefaultBundler({ extension, mainFile, featureFlags }))
   // If the file is a zip, we assume the function is bundled and ready to go.
   // We simply copy it to the destination path with no further processing.
   if (extension === '.zip') {

src/zip.js

@@ -94,6 +94,7 @@ const zipFunctions = async function (
         srcDir: func.srcDir,
         srcPath: func.srcPath,
         stat: func.stat,
+        featureFlags,
       })
 
       return { ...zipResult, mainFile: func.mainFile, name: func.name, runtime: func.runtime }
@@ -151,6 +152,7 @@ const zipFunction = async function (
     stat,
     runtime,
     pluginsModulesPath,
+    featureFlags,
   })
 
   return formatZipResult({ ...zipResult, mainFile, name, runtime })

tests/fixtures/local-require-esm/function_export_only/function_export_only.js

@@ -0,0 +1,2 @@
+// eslint-disable-next-line node/no-unsupported-features/es-syntax
+export const howdy = 'Yee haw!'

tests/fixtures/local-require-esm/function_import_only/function_import_only.js

@@ -0,0 +1,4 @@
+// eslint-disable-next-line node/no-unsupported-features/es-syntax
+import getZero from '../function/file'
+
+getZero()

tests/main.js

@@ -272,16 +272,39 @@ testBundlers('Can use dynamic import() with esbuild', [ESBUILD, ESBUILD_ZISI], a
   await zipNode(t, 'dynamic-import', { opts: { config: { '*': { nodeBundler: bundler } } } })
 })
 
-testBundlers('Bundling does not crash with dynamic import() with zisi', [DEFAULT], async (bundler, t) => {
-  await t.throwsAsync(zipNode(t, 'dynamic-import', { opts: { config: { '*': { nodeBundler: bundler } } } }), {
-    message: /export/,
-  })
-})
-
 testBundlers('Can require local files', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {
   await zipNode(t, 'local-require', { opts: { config: { '*': { nodeBundler: bundler } } } })
 })
 
+testBundlers(
+  'Can bundle functions with `.js` extension using ES Modules and feature flag ON',
+  [ESBUILD, ESBUILD_ZISI, DEFAULT],
+  async (bundler, t) => {
+    await zipNode(t, 'local-require-esm', {
+      length: 3,
+      opts: { featureFlags: { defaultEsModulesToEsbuild: true }, config: { '*': { nodeBundler: bundler } } },
+    })
+  },
+)
+
+testBundlers(
+  'Can bundle functions with `.js` extension using ES Modules and feature flag OFF',
+  [ESBUILD, ESBUILD_ZISI, DEFAULT],
+  async (bundler, t) => {
+    await (bundler === DEFAULT
+      ? t.throwsAsync(
+          zipNode(t, 'local-require-esm', {
+            length: 3,
+            opts: { featureFlags: { defaultEsModulesToEsbuild: false }, config: { '*': { nodeBundler: bundler } } },
+          }),
+        )
+      : zipNode(t, 'local-require-esm', {
+          length: 3,
+          opts: { featureFlags: { defaultEsModulesToEsbuild: false }, config: { '*': { nodeBundler: bundler } } },
+        }))
+  },
+)
+
 testBundlers('Can require local files deeply', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {
   await zipNode(t, 'local-deep-require', { opts: { config: { '*': { nodeBundler: bundler } } } })
 })