-
Notifications
You must be signed in to change notification settings - Fork 254
feat: patch @endo/bundle-source to esbuild if bundle is too big
#12212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
@endo/bundle-source to use esbuild if the bundle gets too big
@endo/bundle-source to use esbuild if the bundle gets too big@endo/bundle-source to esbuild if bundle is too big
9d68bb8 to
5599ea2
Compare
|
@copilot please summarize this PR by writing a comment with the same format as the PR's current description. |
|
@michaelfig I've opened a new pull request, #12227, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR patches @endo/bundle-source to automatically fall back to esbuild-based bundling when bundles exceed 500KB, addressing bundle size issues. The patch is applied via Yarn's patch mechanism and includes updates to the test infrastructure to properly handle bundle IDs that now incorporate hashes.
Key Changes
- Patches
@endo/bundle-sourceto add esbuild fallback when bundles exceed 500KB (configurable viabyteLimitoption) - Adds
registerBundleandgetBundleIDhelpers tofakeVatAdminfor test code that automatically append hash suffixes to bundle IDs for endoZipBase64 bundles - Uses dynamic imports with
avoidBundlingvariable pattern to prevent bundlers from including modules intended to be loaded dynamically
Reviewed Changes
Copilot reviewed 60 out of 63 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
.yarn/patches/@endo-bundle-source-npm-4.1.2-80da9522ea.patch |
Core patch adding esbuild fallback logic and new options |
package.json |
Adds patch reference for @endo/bundle-source |
yarn.lock |
Updates lock file with patch reference and adds esbuild dependencies |
packages/zoe/tools/fakeVatAdmin.js |
Adds registerBundle and getBundleID helpers, improves bundle mismatch error messages |
packages/zoe/test/**/*.test.js |
Updates tests to use registerBundle instead of installBundle directly |
packages/SwingSet/**/*.js |
Adds byteLimit: Infinity to disable limits for kernel/system bundles |
Various *.build.js files |
Applies avoidBundling pattern to prevent bundling of dynamic imports |
multichain-testing/ and a3p-integration/ |
Updates to use patched @endo/bundle-source |
81793ff to
2b02681
Compare
turadg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised to see bundle-source patched instead of wrapped. We could have a local module that wraps it and use resolutions to wire it in wherever bundle-source is imported.
Since this is a stopgap I won't block on that.
But I do want to avoid the code changes to accomodate esbuild.
| export const parseChainHubOpts = async scriptArgs => { | ||
| // import dynamically so the modules can work in CoreEval environment | ||
| const { parseArgs } = await import('node:util'); | ||
| const avoidBundling = 'node:util'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose esbuild it bundling node-util when it sees the literal. But the code here shouldn't be concerned with that.
Instead please configure esbuild with the --external option to exclude what shouldn't be bundled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for raising this.
I started with surgical use of --external, but the list was growing too large for my comfort, and it was fragile.
When digging a little deeper, I see that I can use (await import('node:module')).builtinModules and 'node:*' to catch 'em all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, now I hit a different hurdle that makes --external fail for a different reason. We need to package the esbuild output with the original @endo/bundle-source implementation, and that chokes on all the modules that esbuild has passed through.
It seems @endo/compartment-mapper hasn't sufficiently implemented externals for this use, but I'll ask @kriskowal if there's a way to make this work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still digging. I hope to have a solution by tomorrow.
closes: #12193
[Best reviewed commit-by-commit.]
Description
Patches
@endo/bundle-sourceto automatically retry with esbuild minimisation when bundles exceed 500KB, helping to prevent RPC nodemax_body_bytesrejections until bundle chunking is available.Patch implementation (
.yarn/patches/@endo-bundle-source-npm-4.1.2-*.patch):byteLimitoption (default 500kB) andesbuildPathoptionesbuild --bundle --minify-whitespaceand re-bundles the outputpackage.jsonif needed for@endo/compartment-mapper's module resolutionPrevent esbuild from bundling dynamic import expressions (~20 files):
const mod = '@pkg/name'; await import(mod)prevents esbuild from traversing@pkg/name@endo/bundle-sourcealready treats as externalKernel bundle exemption:
byteLimit: Infinityin SwingSet bundling paths (kernel bundles don't transit RPC)Test fidelity:
fakeVatAdmin.registerBundle()now generates bundleIDs containing the bundle hashendoZipBase64bundlesSecurity Considerations
Introduces esbuild as a dependency for minimisation. Only activated when standard Endo bundles exceed 500KB.
Scaling Considerations
Raises the effective contract size ceiling without requiring on-chain changes (even though this PR has kernel changes, they only need to be deployed during the next coordinated chain software update). Bundles under 500KB are unchanged; larger bundles trade legibility for deployability.
Documentation Considerations
Transparent to consumers—no API changes required. The intermediate-variable pattern for dynamic imports is fairly self-explanatory. It revealed a lenience that
@endo/bundle-sourcehas, but other conventional JS bundlers do not.Testing Considerations
Existing tests pass. Enhanced bundleID generation improves test realism without causing behaviour regressions.
Upgrade Considerations
The patch itself is temporary and may be removable once bundle chunking ships. No migration needed—
byteLimitis optional and backward-compatible.