From 6a9a9f86ec0f9f5bd32d944ebad33057c53f14ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Bou=C3=A7as?= Date: Thu, 4 Nov 2021 14:51:50 +0000 Subject: [PATCH] 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 | 1 + src/runtimes/node/bundlers/nft/es_modules.ts | 105 +++++++++++++----- src/runtimes/node/bundlers/nft/index.ts | 20 +++- src/runtimes/node/bundlers/nft/transpile.ts | 16 +-- .../function_cjs/function_cjs.js | 5 + tests/helpers/main.js | 9 +- tests/main.js | 68 +++++++----- 7 files changed, 148 insertions(+), 76 deletions(-) create mode 100644 tests/fixtures/local-require-esm/function_cjs/function_cjs.js diff --git a/src/feature_flags.ts b/src/feature_flags.ts index 1e06e282c..e3baba234 100644 --- a/src/feature_flags.ts +++ b/src/feature_flags.ts @@ -4,6 +4,7 @@ const FLAGS: Record = { 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, } diff --git a/src/runtimes/node/bundlers/nft/es_modules.ts b/src/runtimes/node/bundlers/nft/es_modules.ts index 3734c953f..621e26879 100644 --- a/src/runtimes/node/bundlers/nft/es_modules.ts +++ b/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, 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, - 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() 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, 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 + fsCache: FsCache + reasons: NodeFileTraceReasons +}) => { + const cache: Map = 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 } diff --git a/src/runtimes/node/bundlers/nft/index.ts b/src/runtimes/node/bundlers/nft/index.ts index cfab7b114..47f5d5af4 100644 --- a/src/runtimes/node/bundlers/nft/index.ts +++ b/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, } } diff --git a/src/runtimes/node/bundlers/nft/transpile.ts b/src/runtimes/node/bundlers/nft/transpile.ts index 757a7599e..73de0f3d5 100644 --- a/src/runtimes/node/bundlers/nft/transpile.ts +++ b/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 = 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 } diff --git a/tests/fixtures/local-require-esm/function_cjs/function_cjs.js b/tests/fixtures/local-require-esm/function_cjs/function_cjs.js new file mode 100644 index 000000000..d32a049f1 --- /dev/null +++ b/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 +} diff --git a/tests/helpers/main.js b/tests/helpers/main.js index a4db3008e..df4015f49 100644 --- a/tests/helpers/main.js +++ b/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 }) { diff --git a/tests/main.js b/tests/main.js index b233a59df..a76b7b233 100644 --- a/tests/main.js +++ b/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(), {}) }, )