feat: reinstate ESM transpilation for NFT (#790)

* feat: reinstate ESM transpilation for NFT

* chore: add check for Node version

* chore: add comment

* chore: doh

* chore: simplify Node version check

Co-authored-by: Netlify Team Account 1 <90322326+netlify-team-account-1@users.noreply.github.com>

* refactor: simplify Map initialisation

Co-authored-by: Netlify Team Account 1 <90322326+netlify-team-account-1@users.noreply.github.com>

* refactor: fix `shouldTranspile`

* refactor: make `shouldTranspile` read from cache

* refactor: use non-null assertion operator

Co-authored-by: Netlify Team Account 1 <90322326+netlify-team-account-1@users.noreply.github.com>

* refactor: remove non-null assertion operator

Co-authored-by: Netlify Team Account 1 <90322326+netlify-team-account-1@users.noreply.github.com>

src/feature_flags.ts

@@ -4,6 +4,7 @@ const FLAGS: Record<string, boolean> = {
   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),
+  nftTranspile: false,
   parseWithEsbuild: false,
   traceWithNft: false,
 }

src/runtimes/node/bundlers/nft/es_modules.ts

@@ -1,35 +1,16 @@
-import { resolve } from 'path'
+import { basename, resolve } from 'path'
 
 import { NodeFileTraceReasons } from '@vercel/nft'
 
+import type { FunctionConfig } from '../../../../config'
 import { cachedReadFile, FsCache } from '../../../../utils/fs'
 import { PackageJson } from '../../utils/package_json'
 
-const getESMPackageJsons = (esmPaths: Set<string>, reasons: NodeFileTraceReasons, basePath?: string) => {
-  const packageJsons: string[] = [...reasons.entries()]
-    .filter(([, reason]) => {
-      if (reason.type !== 'resolve') {
-        return false
-      }
-
-      const hasESMParent = [...reason.parents].some((parentPath) => esmPaths.has(parentPath))
+import { transpile } from './transpile'
 
-      return hasESMParent
-    })
-    .map(([path]) => (basePath ? resolve(basePath, path) : resolve(path)))
-
-  return packageJsons
-}
-
-const getPatchedESMPackages = async (
-  esmPaths: Set<string>,
-  reasons: NodeFileTraceReasons,
-  fsCache: FsCache,
-  basePath?: string,
-) => {
-  const packages = getESMPackageJsons(esmPaths, reasons, basePath)
+const getPatchedESMPackages = async (packages: string[], fsCache: FsCache) => {
   const patchedPackages = await Promise.all(packages.map((path) => patchESMPackage(path, fsCache)))
-  const patchedPackagesMap = new Map()
+  const patchedPackagesMap = new Map<string, string>()
 
   packages.forEach((packagePath, index) => {
     patchedPackagesMap.set(packagePath, patchedPackages[index])
@@ -49,4 +30,78 @@ const patchESMPackage = async (path: string, fsCache: FsCache) => {
   return JSON.stringify(patchedPackageJson)
 }
 
-export { getPatchedESMPackages }
+const shouldTranspile = (path: string, cache: Map<string, boolean>, reasons: NodeFileTraceReasons): boolean => {
+  if (cache.has(path)) {
+    return cache.get(path) as boolean
+  }
+
+  const reason = reasons.get(path)
+
+  // This isn't an expected case, but if the path doesn't exist in `reasons` we
+  // don't transpile it.
+  if (reason === undefined) {
+    cache.set(path, false)
+
+    return false
+  }
+
+  const { parents } = reason
+
+  // If the path doesn't have any parents, it's safe to transpile.
+  if (parents.size === 0) {
+    cache.set(path, true)
+
+    return true
+  }
+
+  // The path should be transpiled if every parent will also be transpiled, or
+  // if there is no parent.
+  const shouldTranspilePath = [...parents].every((parentPath) => shouldTranspile(parentPath, cache, reasons))
+
+  cache.set(path, shouldTranspilePath)
+
+  return shouldTranspilePath
+}
+
+const transpileESM = async ({
+  basePath,
+  config,
+  esmPaths,
+  fsCache,
+  reasons,
+}: {
+  basePath: string | undefined
+  config: FunctionConfig
+  esmPaths: Set<string>
+  fsCache: FsCache
+  reasons: NodeFileTraceReasons
+}) => {
+  const cache: Map<string, boolean> = new Map()
+  const paths = [...esmPaths].filter((path) => shouldTranspile(path, cache, reasons))
+  const pathsSet = new Set(paths)
+  const packageJsonPaths: string[] = [...reasons.entries()]
+    .filter(([path, reason]) => {
+      if (basename(path) !== 'package.json') {
+        return false
+      }
+
+      const needsPatch = [...reason.parents].some((parentPath) => pathsSet.has(parentPath))
+
+      return needsPatch
+    })
+    .map(([path]) => (basePath ? resolve(basePath, path) : resolve(path)))
+  const rewrites = await getPatchedESMPackages(packageJsonPaths, fsCache)
+
+  await Promise.all(
+    paths.map(async (path) => {
+      const absolutePath = basePath ? resolve(basePath, path) : resolve(path)
+      const transpiled = await transpile(absolutePath, config)
+
+      rewrites.set(absolutePath, transpiled)
+    }),
+  )
+
+  return rewrites
+}
+
+export { transpileESM }

src/runtimes/node/bundlers/nft/index.ts

@@ -12,6 +12,8 @@ import type { GetSrcFilesFunction } from '../../../runtime'
 import { getBasePath } from '../../utils/base_path'
 import { filterExcludedPaths, getPathsOfIncludedFiles } from '../../utils/included_files'
 
+import { transpileESM } from './es_modules'
+
 // Paths that will be excluded from the tracing process.
 const ignore = ['node_modules/aws-sdk/**']
 
@@ -20,6 +22,7 @@ const appearsToBeModuleName = (name: string) => !name.startsWith('.')
 const bundle: BundleFunction = async ({
   basePath,
   config,
+  featureFlags,
   mainFile,
   pluginsModulesPath,
   repositoryRoot = basePath,
@@ -29,11 +32,12 @@ const bundle: BundleFunction = async ({
     includedFiles,
     includedFilesBasePath || basePath,
   )
-  const { paths: dependencyPaths } = await traceFilesAndTranspile({
+  const { paths: dependencyPaths, rewrites } = await traceFilesAndTranspile({
     basePath: repositoryRoot,
     config,
     mainFile,
     pluginsModulesPath,
+    transpile: featureFlags.nftTranspile,
   })
   const filteredIncludedPaths = filterExcludedPaths([...dependencyPaths, ...includedFilePaths], excludedPaths)
   const dirnames = filteredIncludedPaths.map((filePath) => normalize(dirname(filePath))).sort()
@@ -45,6 +49,7 @@ const bundle: BundleFunction = async ({
     basePath: getBasePath(dirnames),
     inputs: dependencyPaths,
     mainFile,
+    rewrites,
     srcFiles,
   }
 }
@@ -58,16 +63,23 @@ const ignoreFunction = (path: string) => {
 
 const traceFilesAndTranspile = async function ({
   basePath,
+  config,
   mainFile,
   pluginsModulesPath,
+  transpile,
 }: {
   basePath?: string
   config: FunctionConfig
   mainFile: string
   pluginsModulesPath?: string
+  transpile: boolean
 }) {
   const fsCache: FsCache = {}
-  const { fileList: dependencyPaths } = await nodeFileTrace([mainFile], {
+  const {
+    fileList: dependencyPaths,
+    esmFileList,
+    reasons,
+  } = await nodeFileTrace([mainFile], {
     base: basePath,
     ignore: ignoreFunction,
     readFile: async (path: string) => {
@@ -103,9 +115,13 @@ const traceFilesAndTranspile = async function ({
   const normalizedDependencyPaths = [...dependencyPaths].map((path) =>
     basePath ? resolve(basePath, path) : resolve(path),
   )
+  const rewrites = transpile
+    ? await transpileESM({ basePath, config, esmPaths: esmFileList, fsCache, reasons })
+    : undefined
 
   return {
     paths: normalizedDependencyPaths,
+    rewrites,
   }
 }
 

src/runtimes/node/bundlers/nft/transpile.ts

@@ -20,18 +20,4 @@ const transpile = async (path: string, config: FunctionConfig) => {
   return transpiled.outputFiles[0].text
 }
 
-const transpileMany = async (paths: string[], config: FunctionConfig) => {
-  const transpiledPaths: Map<string, string> = new Map()
-
-  await Promise.all(
-    paths.map(async (path) => {
-      const transpiled = await transpile(path, config)
-
-      transpiledPaths.set(path, transpiled)
-    }),
-  )
-
-  return transpiledPaths
-}
-
-export { transpileMany }
+export { transpile }

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

@@ -0,0 +1,5 @@
+module.exports = async function getZero() {
+  const esm = import('esm-module')
+
+  return esm && 0
+}

tests/helpers/main.js

@@ -59,14 +59,15 @@ const requireExtractedFiles = async function (t, files) {
   t.true(jsFiles.every(Boolean))
 }
 
-const unzipFiles = async function (files) {
-  await Promise.all(files.map(unzipFile))
+const unzipFiles = async function (files, targetPathGenerator) {
+  await Promise.all(files.map(({ path }) => unzipFile({ path, targetPathGenerator })))
 }
 
-const unzipFile = async function ({ path }) {
+const unzipFile = async function ({ path, targetPathGenerator }) {
   const zip = new AdmZip(path)
   const pExtractAll = promisify(zip.extractAllToAsync.bind(zip))
-  await pExtractAll(`${path}/..`, false)
+  const targetPath = targetPathGenerator ? targetPathGenerator(path) : `${path}/..`
+  await pExtractAll(targetPath, false)
 }
 
 const replaceUnzipPath = function ({ path }) {

tests/main.js

@@ -1,7 +1,7 @@
 const { readFile, chmod, symlink, unlink, rename, stat, writeFile } = require('fs')
 const { tmpdir } = require('os')
-const { dirname, isAbsolute, join, normalize, resolve, sep } = require('path')
-const { arch, env, platform } = require('process')
+const { basename, dirname, isAbsolute, join, normalize, resolve, sep } = require('path')
+const { arch, env, platform, version: nodeVersion } = require('process')
 const { promisify } = require('util')
 
 const test = require('ava')
@@ -11,6 +11,7 @@ const del = require('del')
 const execa = require('execa')
 const makeDir = require('make-dir')
 const pathExists = require('path-exists')
+const semver = require('semver')
 const sinon = require('sinon')
 const sortOn = require('sort-on')
 const { dir: getTmpDir, tmpName } = require('tmp-promise')
@@ -84,6 +85,10 @@ const testMany = makeTestMany(test, {
   bundler_nft: {
     config: { '*': { nodeBundler: 'nft' } },
   },
+  bundler_nft_transpile: {
+    config: { '*': { nodeBundler: 'nft' } },
+    featureFlags: { nftTranspile: true },
+  },
 })
 
 const getNodeBundlerString = (variation) => {
@@ -425,40 +430,43 @@ testMany(
 )
 
 testMany(
-  'Can bundle functions with `.js` extension using ES Modules and feature flag ON',
-  ['bundler_esbuild', 'bundler_default', 'todo:bundler_nft'],
-  async (options, t) => {
-    const opts = merge(options, { featureFlags: { defaultEsModulesToEsbuild: true } })
-
-    await zipNode(t, 'local-require-esm', {
-      length: 3,
-      opts,
-    })
-  },
-)
-
-testMany(
-  'Can bundle functions with `.js` extension using ES Modules and feature flag OFF',
-  ['bundler_esbuild', 'bundler_default', 'todo:bundler_nft'],
-  async (options, t) => {
+  'Can bundle functions with `.js` extension using ES Modules',
+  ['bundler_esbuild', 'bundler_nft', 'bundler_nft_transpile'],
+  async (options, t, variation) => {
+    const length = 4
     const fixtureName = 'local-require-esm'
     const opts = merge(options, {
       basePath: `${FIXTURES_DIR}/${fixtureName}`,
       featureFlags: { defaultEsModulesToEsbuild: false },
     })
-    const bundler = options.config['*'].nodeBundler
+    const { files, tmpDir } = await zipFixture(t, 'local-require-esm', {
+      length,
+      opts,
+    })
+
+    await unzipFiles(files, (path) => `${path}/../${basename(path)}_out`)
+
+    const func1 = () => require(join(tmpDir, 'function.zip_out', 'function.js'))
+    const func2 = () => require(join(tmpDir, 'function_cjs.zip_out', 'function_cjs.js'))
+    const func3 = () => require(join(tmpDir, 'function_export_only.zip_out', 'function_export_only.js'))
+    const func4 = () => require(join(tmpDir, 'function_import_only.zip_out', 'function_import_only.js'))
+
+    // Dynamic imports are not supported in Node <13.2.0.
+    if (semver.gte(nodeVersion, '13.2.0')) {
+      t.is(await func2()(), 0)
+    }
+
+    if (variation === 'bundler_nft') {
+      t.throws(func1)
+      t.throws(func3)
+      t.throws(func4)
+
+      return
+    }
 
-    await (bundler === undefined
-      ? t.throwsAsync(
-          zipNode(t, 'local-require-esm', {
-            length: 3,
-            opts,
-          }),
-        )
-      : zipNode(t, 'local-require-esm', {
-          length: 3,
-          opts,
-        }))
+    t.is(func1().ZERO, 0)
+    t.is(typeof func3().howdy, 'string')
+    t.deepEqual(func4(), {})
   },
 )