Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

feat: add pure ESM support to esbuild and default bundlers #1018

Merged
merged 6 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/feature_flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import { env } from 'process'
export const defaultFlags: 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),
parseWithEsbuild: false,
traceWithNft: false,
zisi_detect_esm: false,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This determines whether we'll look at a .js file and figure out whether it's CJS or ESM.

zisi_pure_esm: false,
}

Expand Down
19 changes: 17 additions & 2 deletions src/runtimes/node/bundlers/esbuild/bundler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import { build, Metafile } from '@netlify/esbuild'
import { tmpName } from 'tmp-promise'

import type { FunctionConfig } from '../../../../config.js'
import { FeatureFlags } from '../../../../feature_flags.js'
import { getPathWithExtension, safeUnlink } from '../../../../utils/fs.js'
import type { RuntimeName } from '../../../runtime.js'
import type { NodeBundlerName } from '../index.js'

import { getBundlerTarget } from './bundler_target.js'
import { getBundlerTarget, getModuleFormat } from './bundler_target.js'
import { getDynamicImportsPlugin } from './plugin_dynamic_imports.js'
import { getNativeModulesPlugin } from './plugin_native_modules.js'
import { getNodeBuiltinPlugin } from './plugin_node_builtin.js'
Expand All @@ -28,6 +29,7 @@ export const bundleJsFile = async function ({
basePath,
config,
externalModules = [],
featureFlags,
ignoredModules = [],
name,
srcDir,
Expand All @@ -37,6 +39,7 @@ export const bundleJsFile = async function ({
basePath?: string
config: FunctionConfig
externalModules: string[]
featureFlags: FeatureFlags
ignoredModules: string[]
name: string
srcDir: string
Expand Down Expand Up @@ -84,11 +87,21 @@ export const bundleJsFile = async function ({
// URLs, not paths, so even on Windows they should use forward slashes.
const sourceRoot = targetDirectory.replace(/\\/g, '/')

// Configuring the output format of esbuild. The `includedFiles` array we get
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were not including a package.json when bundling with esbuild. This becomes a problem with ESM functions, since an ESM file and a lack of a package.json with {"type": "module"} would lead to a CJS function with ESM syntax.

// here contains additional paths to include with the bundle, like the path
// to a `package.json` with {"type": "module"} in case of an ESM function.
const { includedFiles: includedFilesFromModuleDetection, moduleFormat } = await getModuleFormat(
srcDir,
featureFlags,
config.nodeVersion,
)

try {
const { metafile = { inputs: {}, outputs: {} }, warnings } = await build({
bundle: true,
entryPoints: [srcFile],
external,
format: moduleFormat,
logLevel: 'warning',
logLimit: ESBUILD_LOG_LIMIT,
metafile: true,
Expand All @@ -108,12 +121,14 @@ export const bundleJsFile = async function ({
})
const inputs = Object.keys(metafile.inputs).map((path) => resolve(path))
const cleanTempFiles = getCleanupFunction([...bundlePaths.keys()])
const additionalPaths = [...dynamicImportsIncludedPaths, ...includedFilesFromModuleDetection]

return {
additionalPaths: [...dynamicImportsIncludedPaths],
additionalPaths,
bundlePaths,
cleanTempFiles,
inputs,
moduleFormat,
nativeNodeModules,
nodeModulesWithDynamicImports: [...nodeModulesWithDynamicImports],
warnings,
Expand Down
32 changes: 29 additions & 3 deletions src/runtimes/node/bundlers/esbuild/bundler_target.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
const DEFAULT_VERSION = 'node12'
import { FeatureFlags } from '../../../../feature_flags'
import { ModuleFormat } from '../../utils/module_format'
import { DEFAULT_NODE_VERSION, getNodeSupportMatrix } from '../../utils/node_version'
import { getClosestPackageJson } from '../../utils/package_json'

const versionMap = {
'8.x': 'node8',
Expand All @@ -10,18 +13,41 @@ const versionMap = {
type VersionKeys = keyof typeof versionMap
type VersionValues = typeof versionMap[VersionKeys]

export const getBundlerTarget = (suppliedVersion?: string): VersionValues => {
const getBundlerTarget = (suppliedVersion?: string): VersionValues => {
const version = normalizeVersion(suppliedVersion)

if (version && version in versionMap) {
return versionMap[version as VersionKeys]
}

return DEFAULT_VERSION
return versionMap[`${DEFAULT_NODE_VERSION}.x`]
}

const getModuleFormat = async (
srcDir: string,
featureFlags: FeatureFlags,
configVersion?: string,
): Promise<{ includedFiles: string[]; moduleFormat: ModuleFormat }> => {
const packageJsonFile = await getClosestPackageJson(srcDir)
Copy link
Member

@Skn0tt Skn0tt Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we introduce + use getClosestPackageJson. srcDir would be smth like projectDir/netlify/functions/my-function.ts, right? The difference between getClosesPackageJson and getPackageJson is that one has an optimisation to not read further than the nearest node_modules folder, which would only ever apply if we searched the package.json for a NPM dependency. This isn't the case here - what am I missing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getClosestPackageJson returns the contents of the package.json and its path, whereas getPackageJson returns just the contents. I agree that we should consolidate these, but I'd prefer to do that as a separate PR to reduce the amount of changes. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, somehow missed this reply. agree! I'll work on that.

const nodeSupport = getNodeSupportMatrix(configVersion)

if (featureFlags.zisi_pure_esm && packageJsonFile?.contents.type === 'module' && nodeSupport.esm) {
return {
includedFiles: [packageJsonFile.path],
moduleFormat: 'esm',
}
}

return {
includedFiles: [],
moduleFormat: 'cjs',
}
}

const normalizeVersion = (version?: string) => {
const match = version && version.match(/^nodejs(.*)$/)

return match ? match[1] : version
}

export { getBundlerTarget, getModuleFormat }
4 changes: 3 additions & 1 deletion src/runtimes/node/bundlers/esbuild/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const bundle: BundleFunction = async ({
bundlePaths,
cleanTempFiles,
inputs,
moduleFormat,
nativeNodeModules = {},
nodeModulesWithDynamicImports,
warnings,
Expand All @@ -77,6 +78,7 @@ const bundle: BundleFunction = async ({
basePath,
config,
externalModules,
featureFlags,
ignoredModules,
name,
srcDir,
Expand Down Expand Up @@ -122,7 +124,7 @@ const bundle: BundleFunction = async ({
bundlerWarnings,
inputs,
mainFile: normalizedMainFile,
moduleFormat: 'cjs',
moduleFormat,
nativeNodeModules,
nodeModulesWithDynamicImports,
srcFiles: [...supportingSrcFiles, ...bundlePaths.keys()],
Expand Down
12 changes: 5 additions & 7 deletions src/runtimes/node/bundlers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,21 +101,19 @@ export const getDefaultBundler = async ({
mainFile: string
featureFlags: FeatureFlags
}): Promise<NodeBundlerName> => {
const { defaultEsModulesToEsbuild, traceWithNft } = featureFlags

if (['.mjs', '.ts'].includes(extension)) {
return 'esbuild'
}

if (traceWithNft) {
if (featureFlags.traceWithNft) {
return 'nft'
}

if (defaultEsModulesToEsbuild) {
const isEsModule = await detectEsModule({ mainFile })
if (featureFlags.zisi_detect_esm) {
const functionIsESM = await detectEsModule({ mainFile })

if (isEsModule) {
return 'esbuild'
if (functionIsESM) {
return 'nft'
}
}

Expand Down
73 changes: 56 additions & 17 deletions src/runtimes/node/utils/package_json.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { promises as fs } from 'fs'
import { basename, join } from 'path'

import findUp from 'find-up'
import pkgDir from 'pkg-dir'

export interface PackageJson {
Expand All @@ -16,18 +18,39 @@ export interface PackageJson {
type?: string
}

const sanitiseFiles = (files: unknown): string[] | undefined => {
if (!Array.isArray(files)) {
return undefined
export interface PackageJsonFile {
contents: PackageJson
path: string
}

export const getClosestPackageJson = async (resolveDir: string): Promise<PackageJsonFile | null> => {
const packageJsonPath = await findUp(
async (directory) => {
// We stop traversing if we're about to leave the boundaries of any
// node_modules directory.
if (basename(directory) === 'node_modules') {
return findUp.stop
}

const path = join(directory, 'package.json')
const hasPackageJson = await findUp.exists(path)

return hasPackageJson ? path : undefined
},
{ cwd: resolveDir },
)

if (packageJsonPath === undefined) {
return null
}

return files.filter((file) => typeof file === 'string')
}
const packageJson = await readPackageJson(packageJsonPath)

export const sanitisePackageJson = (packageJson: Record<string, unknown>): PackageJson => ({
...packageJson,
files: sanitiseFiles(packageJson.files),
})
return {
contents: packageJson,
path: packageJsonPath,
}
}

// Retrieve the `package.json` of a specific project or module
export const getPackageJson = async function (srcDir: string): Promise<PackageJson> {
Expand All @@ -37,14 +60,7 @@ export const getPackageJson = async function (srcDir: string): Promise<PackageJs
return {}
}

const packageJsonPath = `${packageRoot}/package.json`
try {
// The path depends on the user's build, i.e. must be dynamic
const packageJson = JSON.parse(await fs.readFile(packageJsonPath, 'utf8'))
return sanitisePackageJson(packageJson)
} catch (error) {
throw new Error(`${packageJsonPath} is invalid JSON: ${error.message}`)
}
return readPackageJson(`${packageRoot}/package.json`)
}

export const getPackageJsonIfAvailable = async (srcDir: string): Promise<PackageJson> => {
Expand All @@ -56,3 +72,26 @@ export const getPackageJsonIfAvailable = async (srcDir: string): Promise<Package
return {}
}
}

const readPackageJson = async (path: string) => {
try {
// The path depends on the user's build, i.e. must be dynamic
const packageJson = JSON.parse(await fs.readFile(path, 'utf8'))
return sanitisePackageJson(packageJson)
} catch (error) {
throw new Error(`${path} is invalid JSON: ${error.message}`)
}
}

const sanitiseFiles = (files: unknown): string[] | undefined => {
if (!Array.isArray(files)) {
return undefined
}

return files.filter((file) => typeof file === 'string')
}

export const sanitisePackageJson = (packageJson: Record<string, unknown>): PackageJson => ({
...packageJson,
files: sanitiseFiles(packageJson.files),
})
30 changes: 17 additions & 13 deletions tests/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -431,18 +431,18 @@ testMany(

testMany(
'Can bundle ESM functions and transpile them to CJS when the Node version is <14',
['bundler_nft'],
['bundler_default', 'bundler_esbuild', 'bundler_nft'],
async (options, t) => {
const length = 4
const fixtureName = 'local-require-esm'
const opts = merge(options, {
basePath: `${FIXTURES_DIR}/${fixtureName}`,
basePath: join(FIXTURES_DIR, fixtureName),
config: {
'*': {
nodeVersion: 'nodejs12.x',
},
},
featureFlags: { defaultEsModulesToEsbuild: false },
featureFlags: { zisi_detect_esm: true, zisi_pure_esm: false },
})
const { files, tmpDir } = await zipFixture(t, fixtureName, {
length,
Expand Down Expand Up @@ -485,19 +485,19 @@ testMany(

testMany(
'Can bundle ESM functions and transpile them to CJS when the Node version is <14 and `archiveType` is `none`',
['bundler_esbuild', 'bundler_nft'],
['bundler_default', 'bundler_esbuild', 'bundler_nft'],
async (options, t) => {
const length = 4
const fixtureName = 'local-require-esm'
const opts = merge(options, {
archiveFormat: 'none',
basePath: `${FIXTURES_DIR}/${fixtureName}`,
basePath: join(FIXTURES_DIR, fixtureName),
config: {
'*': {
nodeVersion: 'nodejs12.x',
},
},
featureFlags: { defaultEsModulesToEsbuild: false },
featureFlags: { zisi_detect_esm: true, zisi_pure_esm: false },
})
const { tmpDir } = await zipFixture(t, fixtureName, {
length,
Expand Down Expand Up @@ -538,11 +538,14 @@ testMany(

testMany(
'Can bundle CJS functions that import ESM files with an `import()` expression',
['bundler_esbuild', 'bundler_nft'],
['bundler_default', 'bundler_esbuild', 'bundler_nft'],
async (options, t) => {
const fixtureName = 'node-cjs-importing-mjs'
const opts = merge(options, {
featureFlags: { zisi_detect_esm: true },
})
const { files, tmpDir } = await zipFixture(t, fixtureName, {
opts: options,
opts,
})

await unzipFiles(files)
Expand All @@ -561,13 +564,13 @@ testMany(

testMany(
'Can bundle native ESM functions when the Node version is >=14 and the `zisi_pure_esm` flag is on',
['bundler_nft'],
['bundler_default', 'bundler_nft', 'bundler_esbuild'],
async (options, t) => {
const length = 2
const fixtureName = 'node-esm'
const opts = merge(options, {
basePath: `${FIXTURES_DIR}/${fixtureName}`,
featureFlags: { zisi_pure_esm: true },
basePath: join(FIXTURES_DIR, fixtureName),
featureFlags: { zisi_detect_esm: true, zisi_pure_esm: true },
})
const { files, tmpDir } = await zipFixture(t, fixtureName, {
length,
Expand All @@ -593,12 +596,13 @@ testMany(

testMany(
'Can bundle ESM functions and transpile them to CJS when the Node version is >=14 and the `zisi_pure_esm` flag is off',
['bundler_nft'],
['bundler_default', 'bundler_esbuild', 'bundler_nft'],
async (options, t) => {
const length = 2
const fixtureName = 'node-esm'
const opts = merge(options, {
basePath: `${FIXTURES_DIR}/${fixtureName}`,
basePath: join(FIXTURES_DIR, fixtureName),
featureFlags: { zisi_detect_esm: true },
})
const { files, tmpDir } = await zipFixture(t, fixtureName, {
length,
Expand Down