Skip to content

fix: externalize libraries instead of bundling them - #6097

Open
CGNonofr wants to merge 1 commit into
xtermjs:masterfrom
CodinGame:externalize-libraries-in-esm
Open

fix: externalize libraries instead of bundling them#6097
CGNonofr wants to merge 1 commit into
xtermjs:masterfrom
CodinGame:externalize-libraries-in-esm

Conversation

@CGNonofr

Copy link
Copy Markdown

fix #6096

Keeping the library as external for the esm version allows the final bundle to chose which version it needs (main, module, browser...)

@arsyadal

Copy link
Copy Markdown

The test-integration failures on all three browsers (Chromium/Firefox/WebKit) are real, not flaky — reproduced locally.

packages: 'external' is added to the shared commonOptions in bin/esbuild.mjs, which every build target spreads first, including the isDemoClient branch that produces demo/dist/client-bundle.js — the bundle demo/test.html loads for every integration test. That branch aliases @xterm/addon-ligatures to its own unbundled out-esbuild/LigaturesAddon output (see the HACK comment right above it, for an unrelated fs-at-startup issue), and when the demo-client bundle step re-bundles that addon's opentype.js/lru-cache imports with packages: 'external' on, esbuild emits __require("opentype.js") / __require("lru-cache") calls into the browser bundle instead of inlining them. There's no require in a browser, so the whole client-bundle.js throws during initial evaluation, before it ever reaches window.Terminal = Terminal — which is exactly the window.Terminal is not a constructor error in the CI logs.

Repro: add packages: 'external', to commonOptions on current master (no other change), npm run esbuild-demo-client, then grep -n "__require(" demo/dist/client-bundle.js — shows the two calls.

packages: 'external' fixes the original lru-cache/node:diagnostics_channel problem (#6096) correctly for the addon-ligatures build itself (platform: 'node' there, so an unresolved require is fine), but applying it through commonOptions leaks it into every other target, including the ones that need to stay self-contained in a browser <script> tag. Scoping it to just the config.addon === 'ligatures' block (alongside the existing bundleConfig.platform = 'node' line) instead of commonOptions should fix the CI failures without losing the original fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Esbuild bundles the node version of the dependencies

2 participants