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

Commit

Permalink
fix: make esbuild work with ESM and require calls (#1257)
Browse files Browse the repository at this point in the history
  • Loading branch information
danez committed Nov 4, 2022
1 parent 5a7562a commit a4ec376
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 1 deletion.
1 change: 1 addition & 0 deletions .eslintrc.cjs
Expand Up @@ -9,6 +9,7 @@ module.exports = {
'import/extensions': ['error', 'ignorePackages'],
'n/no-missing-import': 'off',
'no-magic-numbers': 'off',
'max-lines-per-function': 'off',
// This rule enforces using Buffers with `JSON.parse()`. However, TypeScript
// does not recognize yet that `JSON.parse()` accepts Buffers as argument.
'unicorn/prefer-json-parse-buffer': 'off',
Expand Down
3 changes: 3 additions & 0 deletions src/feature_flags.ts
Expand Up @@ -20,6 +20,9 @@ export const defaultFlags: Record<string, boolean> = {

// Load configuration from per-function JSON files.
project_deploy_configuration_api_use_per_function_configuration_files: false,

// Provide banner to esbuild which allows requires in ESM output
zisi_esbuild_require_banner: false,
}

export type FeatureFlag = keyof typeof defaultFlags
Expand Down
13 changes: 13 additions & 0 deletions src/runtimes/node/bundlers/esbuild/bundler.ts
Expand Up @@ -108,8 +108,21 @@ export const bundleJsFile = async function ({
const outExtension =
moduleFormat === ModuleFormat.ESM ? { [ModuleFileExtension.JS]: ModuleFileExtension.MJS } : undefined

// We add this banner so that calls to require() still work in ESM modules, especially when importing node built-ins
// We have to do this until this is fixed in esbuild: https://github.com/evanw/esbuild/pull/2067
const esmJSBanner = `
import {createRequire as ___nfyCreateRequire} from "module";
import {fileURLToPath as ___nfyFileURLToPath} from "url";
import {dirname as ___nfyPathDirname} from "path";
let __filename=___nfyFileURLToPath(import.meta.url);
let __dirname=___nfyPathDirname(___nfyFileURLToPath(import.meta.url));
let require=___nfyCreateRequire(import.meta.url);
`

try {
const { metafile = { inputs: {}, outputs: {} }, warnings } = await build({
banner:
moduleFormat === ModuleFormat.ESM && featureFlags.zisi_esbuild_require_banner ? { js: esmJSBanner } : undefined,
bundle: true,
entryPoints: [srcFile],
external,
Expand Down
3 changes: 3 additions & 0 deletions tests/fixtures/node-require-in-esm/functions/func1.mjs
@@ -0,0 +1,3 @@
import include from '../include.cjs';

export const handler = () => true
3 changes: 3 additions & 0 deletions tests/fixtures/node-require-in-esm/include.cjs
@@ -0,0 +1,3 @@
const path = require('path');

export default 1
33 changes: 32 additions & 1 deletion tests/main.test.ts
Expand Up @@ -14,9 +14,10 @@ import unixify from 'unixify'
import { afterAll, afterEach, describe, expect, test, vi } from 'vitest'

import type { Config } from '../src/config.js'
import { NodeBundlerType } from '../src/main.js'
import { ESBUILD_LOG_LIMIT } from '../src/runtimes/node/bundlers/esbuild/bundler.js'
import { NodeBundlerType } from '../src/runtimes/node/bundlers/types.js'
import { detectEsModule } from '../src/runtimes/node/utils/detect_es_module.js'
import { ModuleFormat } from '../src/runtimes/node/utils/module_format.js'
import { shellUtils } from '../src/utils/shell.js'

import {
Expand Down Expand Up @@ -2418,4 +2419,34 @@ describe('zip-it-and-ship-it', () => {
}
},
)

test('Provides require to esbuild if output format is ESM', async () => {
const fixtureName = 'node-require-in-esm'
const opts = {
basePath: join(FIXTURES_DIR, fixtureName),
config: {
'*': {
nodeBundler: NodeBundlerType.ESBUILD,
nodeModuleFormat: ModuleFormat.ESM,
},
},
featureFlags: {
zisi_esbuild_require_banner: true,
},
}
const { files, tmpDir } = await zipFixture([join(fixtureName, 'functions')], {
opts,
})

await unzipFiles(files, (path) => `${path}/../${basename(path)}_out`)

const funcFile = join(tmpDir, `func1.zip_out/func1.mjs`)

// We have to use execa because when we simply import the file here vitest does provide a `require` function
// and therefore we do not trigger the problem
const result = await execa.node(funcFile, [], { extendEnv: false, reject: false })

expect(result.stderr).not.toContain('Dynamic require of "path" is not supported')
expect(result).not.toBeInstanceOf(Error)
})
})

1 comment on commit a4ec376

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

⏱ Benchmark results

largeDepsEsbuild: 2.9s

largeDepsNft: 14.8s

largeDepsZisi: 21.3s

Please sign in to comment.