-
Notifications
You must be signed in to change notification settings - Fork 7
build: Bundle vats with vite #763
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: main
Are you sure you want to change the base?
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
504110d to
1a13965
Compare
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Drop support for the legacy endoZipBase64 bundle format and remove the @endo/import-bundle dependency. All vat bundles now use the vite-iife format loaded via Compartment.evaluate(). - Remove @endo/import-bundle from ocap-kernel dependencies - Simplify bundle-loader.ts to only support vite-iife format - Update VatSupervisor to use synchronous loadBundle - Update CLI tests to mock bundleVat instead of @endo/bundle-source - Update serve integration test to check vite-iife format Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1a13965 to
b8e02c0
Compare
rekmarks
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.
Really nice!
| type ViteBundle = { | ||
| moduleFormat: 'vite-iife'; | ||
| code: string; | ||
| exports: string[]; | ||
| modules: Record<string, unknown>; | ||
| }; | ||
|
|
||
| const isViteBundle = (value: unknown): value is ViteBundle => |
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.
We should import the renamed VatBundle type here, ideally. The type may need to be relocated to kernel-utils to resolve dependency circularity issues. That would also make a nice home for isVatBundle.
packages/cli/src/vite/vat-bundler.ts
Outdated
| // SES rejects code containing `import(` patterns, even in comments. | ||
| // Replace them with a safe alternative that won't trigger detection. | ||
| const sanitizedCode = chunk.code.replace(/\bimport\s*\(/gu, 'IMPORT('); |
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.
This comment ought to say something about "we solemnly swear that the string import only occurs in comments".
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.
An alternative approach that might be more convincing is to filter comments from the AST using a rollup plugin.
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.
Recommend punting to #770
packages/cli/src/vite/vat-bundler.ts
Outdated
| import type { BundleMetadata } from './export-metadata-plugin.ts'; | ||
|
|
||
| export type VatBundle = BundleMetadata & { | ||
| moduleFormat: 'vite-iife'; |
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.
Continuing to harp on this topic: the number of times we declare this property is a code smell.
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
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.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
| // Skip until end of line | ||
| while (i < code.length && code[i] !== '\n') { | ||
| i += 1; | ||
| } |
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.
Comment stripper incorrectly handles regex literals
High Severity
The comment stripping logic treats any // sequence as a single-line comment, even when it appears inside regex literals. Code containing regex patterns like /https:\/\// or /\w+\/\// would be incorrectly parsed, with content after the escaped slashes stripped away. The plugin only detects string literals but not regex literals, causing valid JavaScript to be corrupted when regex patterns contain comment-like sequences.
| result += strChar; | ||
| i += 1; | ||
| } | ||
| } |
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.
Template literal expressions with comments not handled
Medium Severity
The comment stripper treats backticks as regular string delimiters, copying everything between them without recognizing ${...} template expressions. Comments within these expressions like `${foo /* comment */}` won't be stripped. Since SES rejects code with import( in comments, and this plugin's purpose is preventing that rejection, unstripped comments in template expressions could cause bundle evaluation to fail.
| i += 1; | ||
| } | ||
| i += 1; // Skip the closing / | ||
| } |
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.
Stripped multi-line comments create syntax errors
High Severity
Multi-line comments are removed without replacing them with whitespace, causing adjacent tokens to merge. Code like foo/*comment*/instanceof bar becomes fooinstanceof bar, creating a syntax error. The bundled code will fail to parse when loaded by the compartment, causing vat initialization to fail with confusing parse errors rather than executing correctly.
Summary
Replace
@endo/bundle-sourcewith vite for vat bundling and remove@endo/import-bundlefor bundle loading.Changes
build()API with IIFE output formatCompartment.evaluate()instead ofimportBundle()@endo/import-bundledependency from all packages.@endo/bundle-sourcedependency from all packages except kernel-shims.Bundle Format
New bundles use
vite-iifeformat:{ "moduleFormat": "vite-iife", "code": "var __vatExports__ = ...", "exports": ["buildRootObject"], "modules": { ... } }Closes: #742
Note
Replaces Endo bundle tooling with a Vite-based bundling pipeline and a new runtime loader.
bundleVat) producing IIFE bundles; include Rollup pluginsexport-metadataandstrip-commentsbundle,watch, and server tests; adjust mocks and typesimportBundle, fetch bundle as text, and load vialoadBundleusingCompartment.evaluate()VatBundletype andisVatBundleguard in@metamask/kernel-utils; update tests and fixtures to IIFE formatvite/rollup; remove@endo/bundle-sourceand@endo/import-bundle; update README accordinglyWritten by Cursor Bugbot for commit b9cfdcc. This will update automatically on new commits. Configure here.