fix: detect native bindings and change extension priority (netlify/zi…

…p-it-and-ship-it#353)

* fix: remove externalModules optimisation

* fix: add esbuild plugin for node-fetch

* chore: skip esbuild plugins in Node 8

* feat: add plugin for native bindings

* chore: fix ESLint warning

* chore: pass populated context to plugins

* chore: fix tests

packages/zip-it-and-ship-it/src/bundler.js → packages/zip-it-and-ship-it/src/node_bundler/index.js

@@ -6,10 +6,13 @@ const { promisify } = require('util')
 const esbuild = require('esbuild')
 const semver = require('semver')
 
+const { getPlugins } = require('./plugins')
+
 const pUnlink = promisify(fs.unlink)
 
 const bundleJsFile = async function ({
   additionalModulePaths,
+  basePath,
   destFilename,
   destFolder,
   externalModules = [],
@@ -20,13 +23,19 @@ const bundleJsFile = async function ({
   const external = [...new Set([...externalModules, ...ignoredModules])]
   const jsFilename = `${basename(destFilename, extname(destFilename))}.js`
   const bundlePath = join(destFolder, jsFilename)
+  const pluginContext = {
+    nodeBindings: new Set(),
+  }
 
   // esbuild's async build API throws on Node 8.x, so we switch to the sync
   // version for that version range.
-  const shouldUseAsyncAPI = semver.satisfies(process.version, '>=9.x')
+  const supportsAsyncAPI = semver.satisfies(process.version, '>=9.x')
+
+  // The sync API does not support plugins.
+  const plugins = supportsAsyncAPI ? getPlugins({ additionalModulePaths, basePath, context: pluginContext }) : undefined
 
   // eslint-disable-next-line node/no-sync
-  const buildFunction = shouldUseAsyncAPI ? esbuild.build : esbuild.buildSync
+  const buildFunction = supportsAsyncAPI ? esbuild.build : esbuild.buildSync
   const data = await buildFunction({
     bundle: true,
     entryPoints: [srcFile],
@@ -35,6 +44,8 @@ const bundleJsFile = async function ({
     outfile: bundlePath,
     nodePaths: additionalModulePaths,
     platform: 'node',
+    plugins,
+    resolveExtensions: ['.js', '.jsx', '.mjs', '.cjs', '.json'],
     target: ['es2017'],
   })
   const cleanTempFiles = async () => {
@@ -44,8 +55,9 @@ const bundleJsFile = async function ({
       // no-op
     }
   }
+  const additionalSrcFiles = [...pluginContext.nodeBindings]
 
-  return { bundlePath, cleanTempFiles, data }
+  return { bundlePath, cleanTempFiles, data: { ...data, additionalSrcFiles } }
 }
 
 module.exports = { bundleJsFile }

packages/zip-it-and-ship-it/src/node_bundler/plugins.js

@@ -0,0 +1,22 @@
+const { relative, resolve } = require('path')
+
+const getNodeBindingHandlerPlugin = ({ basePath, context }) => ({
+  name: 'node-binding-handler',
+  setup(build) {
+    build.onResolve({ filter: /\.node$/ }, (args) => {
+      const fullPath = resolve(args.resolveDir, args.path)
+      const resolvedPath = relative(basePath, fullPath)
+
+      context.nodeBindings.add(fullPath)
+
+      return {
+        external: true,
+        path: resolvedPath,
+      }
+    })
+  },
+})
+
+const getPlugins = ({ basePath, context }) => [getNodeBindingHandlerPlugin({ basePath, context })]
+
+module.exports = { getPlugins }

packages/zip-it-and-ship-it/src/runtimes/node.js

@@ -2,7 +2,7 @@ const { basename, dirname, join, normalize } = require('path')
 
 const commonPathPrefix = require('common-path-prefix')
 
-const { bundleJsFile } = require('../bundler')
+const { bundleJsFile } = require('../node_bundler')
 const {
   getDependencyNamesAndPathsForDependencies,
   getExternalAndIgnoredModulesFromSpecialCases,
@@ -51,6 +51,7 @@ const getSrcFilesAndExternalModules = async function ({
   }
 }
 
+// eslint-disable-next-line max-statements
 const zipFunction = async function ({
   destFolder,
   extension,
@@ -69,28 +70,21 @@ const zipFunction = async function ({
     externalModules: externalModulesFromSpecialCases,
     ignoredModules: ignoredModulesFromSpecialCases,
   } = await getExternalAndIgnoredModulesFromSpecialCases({ srcDir })
-
-  // When a module is added to `externalModules`, we will traverse its main
-  // file recursively and look for all its dependencies, so that we can ship
-  // their files separately, inside a `node_modules` directory. Whenever we
-  // process a module this way, we can also flag it as external with esbuild
-  // since its source is already part of the artifact and there's no point in
-  // inlining it again in the bundle.
-  // As such, the dependency traversal logic will compile the names of these
-  // modules in `additionalExternalModules`.
-  const { moduleNames: externalModulesFromTraversal = [], paths: srcFiles } = await getSrcFilesAndExternalModules({
+  const externalModules = [...new Set([...externalModulesFromConfig, ...externalModulesFromSpecialCases])]
+  const { paths: srcFiles } = await getSrcFilesAndExternalModules({
     stat,
     mainFile,
     extension,
     srcPath,
     srcDir,
     pluginsModulesPath,
     jsBundler,
-    jsExternalModules: [...new Set([...externalModulesFromConfig, ...externalModulesFromSpecialCases])],
+    jsExternalModules: externalModules,
   })
-  const dirnames = srcFiles.map((filePath) => normalize(dirname(filePath)))
 
   if (jsBundler === JS_BUNDLER_ZISI) {
+    const dirnames = srcFiles.map((filePath) => normalize(dirname(filePath)))
+
     await zipNodeJs({
       basePath: commonPathPrefix(dirnames),
       destFolder,
@@ -104,16 +98,15 @@ const zipFunction = async function ({
     return { bundler: JS_BUNDLER_ZISI, path: destPath }
   }
 
+  const mainFilePath = dirname(mainFile)
   const { bundlePath, data, cleanTempFiles } = await bundleJsFile({
     additionalModulePaths: pluginsModulesPath ? [pluginsModulesPath] : [],
+    basePath: mainFilePath,
     destFilename: filename,
     destFolder,
-    externalModules: [
-      ...externalModulesFromConfig,
-      ...externalModulesFromSpecialCases,
-      ...externalModulesFromTraversal,
-    ],
+    externalModules,
     ignoredModules: [...ignoredModulesFromConfig, ...ignoredModulesFromSpecialCases],
+    srcDir,
     srcFile: mainFile,
   })
   const bundlerWarnings = data.warnings.length === 0 ? undefined : data.warnings
@@ -123,7 +116,9 @@ const zipFunction = async function ({
   const aliases = {
     [mainFile]: bundlePath,
   }
-  const basePath = commonPathPrefix([...dirnames, dirname(mainFile)])
+  const srcFilesAfterBundling = [...srcFiles, ...data.additionalSrcFiles]
+  const dirnames = srcFilesAfterBundling.map((filePath) => normalize(dirname(filePath)))
+  const basePath = commonPathPrefix([...dirnames, mainFilePath])
 
   try {
     await zipNodeJs({
@@ -134,7 +129,7 @@ const zipFunction = async function ({
       filename,
       mainFile,
       pluginsModulesPath,
-      srcFiles,
+      srcFiles: srcFilesAfterBundling,
     })
   } finally {
     await cleanTempFiles()

packages/zip-it-and-ship-it/tests/fixtures/node-fetch/function.js

@@ -0,0 +1,3 @@
+const fetch = require('node-fetch')
+
+module.exports = fetch

packages/zip-it-and-ship-it/tests/fixtures/node-fetch/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "function-requiring-node-fetch",
+  "version": "1.0.0",
+  "dependencies": {
+    "node-fetch": "^2.6.1"
+  }
+}

packages/zip-it-and-ship-it/tests/main.js

@@ -1,15 +1,17 @@
 const { readFile, chmod, symlink, unlink, rename } = require('fs')
 const { tmpdir } = require('os')
 const { normalize, resolve } = require('path')
-const { platform } = require('process')
+const { platform, version } = require('process')
 const { promisify } = require('util')
 
 const test = require('ava')
 const cpy = require('cpy')
 const del = require('del')
 const execa = require('execa')
 const pathExists = require('path-exists')
+const semver = require('semver')
 const { dir: getTmpDir, tmpName } = require('tmp-promise')
+const unixify = require('unixify')
 
 const { zipFunction, listFunctions, listFunctionsFiles } = require('..')
 const { JS_BUNDLER_ESBUILD: ESBUILD, JS_BUNDLER_ESBUILD_ZISI: ESBUILD_ZISI } = require('../src/utils/consts')
@@ -24,6 +26,8 @@ const pSymlink = promisify(symlink)
 const pUnlink = promisify(unlink)
 const pRename = promisify(rename)
 
+const supportsEsbuildPlugins = semver.satisfies(version, '>=9.x')
+
 // Alias for the default bundler.
 const DEFAULT = undefined
 const EXECUTABLE_PERMISSION = 0o755
@@ -54,13 +58,29 @@ testBundlers('Zips Node.js function files', [ESBUILD, ESBUILD_ZISI, DEFAULT], as
   t.true(files.every(({ runtime }) => runtime === 'js'))
 })
 
-testBundlers('Handles Node module with native bindings', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {
-  const jsExternalModules = bundler === ESBUILD || bundler === ESBUILD ? ['test'] : undefined
-  const { files } = await zipNode(t, 'node-module-native', {
-    opts: { jsBundler: bundler, jsExternalModules },
-  })
-  t.true(files.every(({ runtime }) => runtime === 'js'))
-})
+// Bundling modules with native bindings with esbuild is expected to fail on
+// Node 8 because esbuild plugins are not supported. The ZISI fallback should
+// kick in.
+testBundlers(
+  'Handles Node module with native bindings',
+  [ESBUILD_ZISI, DEFAULT, supportsEsbuildPlugins && ESBUILD].filter(Boolean),
+  async (bundler, t) => {
+    const { files, tmpDir } = await zipNode(t, 'node-module-native', {
+      opts: { jsBundler: bundler },
+    })
+    const requires = await getRequires({ filePath: resolve(tmpDir, 'src/function.js') })
+    const normalizedRequires = new Set(requires.map(unixify))
+
+    t.true(files.every(({ runtime }) => runtime === 'js'))
+    t.true(await pathExists(`${tmpDir}/src/node_modules/test/native.node`))
+
+    if (files[0].bundler === 'esbuild') {
+      t.true(normalizedRequires.has('node_modules/test/native.node'))
+    } else {
+      t.true(normalizedRequires.has('test'))
+    }
+  },
+)
 
 testBundlers('Can require node modules', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {
   await zipNode(t, 'local-node-module', { opts: { jsBundler: bundler } })
@@ -557,6 +577,17 @@ testBundlers(
   },
 )
 
+testBundlers(
+  'Exposes the main export of `node-fetch` when imported using `require()`',
+  [ESBUILD, ESBUILD_ZISI, DEFAULT],
+  async (bundler, t) => {
+    const { files, tmpDir } = await zipFixture(t, 'node-fetch', { opts: { jsBundler: bundler } })
+    await unzipFiles(files)
+    // eslint-disable-next-line import/no-dynamic-require, node/global-require
+    t.true(typeof require(`${tmpDir}/function.js`) === 'function')
+  },
+)
+
 test('Zips Rust function files', async (t) => {
   const { files, tmpDir } = await zipFixture(t, 'rust-simple', { length: 1 })