Skip to content

Commit

Permalink
fix: esm imports with node: prefix on esbuild (#802)
Browse files Browse the repository at this point in the history
* chore: add reproduction for esbuild and node-prefixed imports

* fix: mark node:-prefixed modules as external for esbuild

* chore: move builtin-logic into its own plugin

* chore: run nodebuiltin plugin first

Co-authored-by: Netlify Team Account 1 <netlify-team-account-1@users.noreply.github.com>
  • Loading branch information
1 parent 9c6d107 commit 199f6ed
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/runtimes/node/bundlers/esbuild/bundler.ts
Expand Up @@ -11,6 +11,7 @@ import type { RuntimeName } from '../../../runtime'
import { getBundlerTarget } from './bundler_target'
import { getDynamicImportsPlugin } from './plugin_dynamic_imports'
import { getNativeModulesPlugin } from './plugin_native_modules'
import { getNodeBuiltinPlugin } from './plugin_node_builtin'

// Maximum number of log messages that an esbuild instance will produce. This
// limit is important to avoid out-of-memory errors due to too much data being
Expand Down Expand Up @@ -63,6 +64,7 @@ const bundleJsFile = async function ({

// The list of esbuild plugins to enable for this build.
const plugins = [
getNodeBuiltinPlugin(),
getNativeModulesPlugin(nativeNodeModules),
getDynamicImportsPlugin({
basePath,
Expand Down
10 changes: 10 additions & 0 deletions src/runtimes/node/bundlers/esbuild/plugin_node_builtin.ts
@@ -0,0 +1,10 @@
import type { Plugin } from '@netlify/esbuild'

const getNodeBuiltinPlugin = (): Plugin => ({
name: 'builtin-modules',
setup(build) {
build.onResolve({ filter: /^node:/ }, () => ({ external: true }))
},
})

export { getNodeBuiltinPlugin }
5 changes: 5 additions & 0 deletions tests/fixtures/node-force-builtin-esm/function.js
@@ -0,0 +1,5 @@
import console from 'node:console'

export const handler = () => {
console.log('hello world')
}
12 changes: 11 additions & 1 deletion tests/main.js
Expand Up @@ -2251,7 +2251,7 @@ testMany(
)

testMany(
'Handles built-in modules imported with the `node:` prefix',
'Handles built-in modules required with the `node:` prefix',
['bundler_default', 'bundler_default_nft', 'bundler_nft', 'bundler_esbuild', 'bundler_esbuild_zisi'],
async (options, t) => {
t.plan(3)
Expand Down Expand Up @@ -2279,6 +2279,16 @@ testMany(
},
)

testMany(
'Handles built-in modules imported with the `node:` prefix',
['bundler_default', 'bundler_default_nft', 'bundler_nft', 'bundler_esbuild', 'bundler_esbuild_zisi'],
async (options, t) => {
await zipFixture(t, 'node-force-builtin-esm', {
opts: options,
})
},
)

testMany(
'Returns a `size` property with the size of each generated archive',
['bundler_default', 'bundler_esbuild', 'bundler_nft'],
Expand Down

1 comment on commit 199f6ed

@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: 7.4s

largeDepsNft: 47.3s

largeDepsZisi: 1m 1.6s

Please sign in to comment.