Repository navigation
feat(typescript-web): scaffold the Web bridge release - #4052
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a WebAssembly-backed TypeScript Web SDK package, browser and workerd packaging scripts, unified TypeScript test integration, and release workflows that build, verify, publish, and version the web bridge. ChangesWeb SDK delivery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReleasePlan
participant BuildWebSDK
participant WebArtifact
participant PublishWebSDK
participant NpmRegistry
ReleasePlan->>BuildWebSDK: provide release_plan_json and source_sha
BuildWebSDK->>WebArtifact: upload web-sdk-dist
PublishWebSDK->>WebArtifact: download web-sdk-dist
PublishWebSDK->>NpmRegistry: publish package with channel tag
NpmRegistry-->>PublishWebSDK: report published or existing version
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Binary size checks passed✅ 7 passed
Generated by |
0c41654 to
57373b4
Compare
57373b4 to
a712c84
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57373b405f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const instance = new WebAssembly.Instance(workerdModule, { | ||
| "./bridge_web_core_bg.js": imports, | ||
| }); | ||
| imports.__wbg_set_wasm(instance.exports); |
There was a problem hiding this comment.
Call the wasm-bindgen start hook in Workerd
When the workerd export is used, this replacement bypasses wasm-pack's generated bundler wrapper and only sets the wasm exports; it never invokes the wasm-bindgen start export afterward. The browser build still goes through the generated init path, but in Workers any initialization emitted into __wbindgen_start is skipped before stageRuntimeBytecode/callFunction* run, so the Worker artifact can fail or behave differently from the browser artifact. Add the same start call, guarded if necessary, after setting the wasm exports.
Useful? React with 👍 / 👎.
a712c84 to
fa1d751
Compare
fa1d751 to
30b7aec
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
baml_language/sdks/typescript/bridge_typescript_web/scripts/sync-node-runtime.mjs (1)
23-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant modification of
distProto/baml_cffi.js.The loop iterates over
distPrototo patchbaml_cffi.js, but this patched file is immediately overwritten by the esbuild step on line 30. The modification indistProtois completely redundant since the bundled output will reflect the patchedshared/protoentry point anyway.You can simplify the loop to strictly target
shared/proto.♻️ Proposed refactor
-for (const directory of [resolve(shared, "proto"), distProto]) { - const protoPath = resolve(directory, "baml_cffi.js"); - const proto = readFileSync(protoPath, "utf8").replace('import $protobuf from "protobufjs/minimal.js";', 'import * as $protobuf from "protobufjs/minimal.js";'); - writeFileSync(protoPath, proto); -} +const protoPath = resolve(shared, "proto/baml_cffi.js"); +const proto = readFileSync(protoPath, "utf8").replace('import $protobuf from "protobufjs/minimal.js";', 'import * as $protobuf from "protobufjs/minimal.js";'); +writeFileSync(protoPath, proto); + await build({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript_web/scripts/sync-node-runtime.mjs` around lines 23 - 34, Remove distProto from the directory iteration in sync-node-runtime.mjs, so the replacement runs only on shared/proto/baml_cffi.js. Keep the subsequent build call unchanged, using the patched shared entry point to generate the distProto output.baml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjs (1)
20-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the protobuf import replacement more resilient.
Relying on the exact string
"var $protobuf = __toESM(require_minimal2(), 1);"is brittle because it targets a specific internal esbuild identifier (require_minimal2()). If esbuild updates its generation strategy or dependencies change, this identifier could becomerequire_minimal()orrequire_minimal3(), immediately breaking the build script.Consider using a regular expression to match the import structure instead.
♻️ Proposed refactor
-const protobufImport = "var $protobuf = __toESM(require_minimal2(), 1);"; -const workerdSafeProto = workerdProto.replace(protobufImport, `${protobufImport} -$protobuf.util.Buffer = null; -$protobuf.configure();`); +const protobufImportRegex = /(var\s+\$protobuf\s*=\s*__toESM[^;]+;)/; +const workerdSafeProto = workerdProto.replace(protobufImportRegex, `$1\n$protobuf.util.Buffer = null;\n$protobuf.configure();`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjs` around lines 20 - 23, Update the protobuf import replacement in the prepare-workerd-package script to match the generated import structure with a regular expression rather than the exact require_minimal2() identifier. Preserve the existing injected Buffer reset and protobuf configuration statements, while allowing esbuild’s require_minimal identifier suffix to vary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@baml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjs`:
- Around line 20-23: Update the protobuf import replacement in the
prepare-workerd-package script to match the generated import structure with a
regular expression rather than the exact require_minimal2() identifier. Preserve
the existing injected Buffer reset and protobuf configuration statements, while
allowing esbuild’s require_minimal identifier suffix to vary.
In
`@baml_language/sdks/typescript/bridge_typescript_web/scripts/sync-node-runtime.mjs`:
- Around line 23-34: Remove distProto from the directory iteration in
sync-node-runtime.mjs, so the replacement runs only on
shared/proto/baml_cffi.js. Keep the subsequent build call unchanged, using the
patched shared entry point to generate the distProto output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 78b5bb90-d537-4673-b9f1-907ff546d893
⛔ Files ignored due to path filters (2)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/sdks/typescript/bridge_typescript_web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
.github/workflows/build2-web-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish2-web-sdk.yaml.github/workflows/release-baml-language.ymlbaml_language/.config/nextest.tomlbaml_language/Cargo.tomlbaml_language/sdk_tests/crates/typescript/setup.ps1baml_language/sdk_tests/crates/typescript/setup.shbaml_language/sdk_tests/harness_setup/src/templates/package.jsonbaml_language/sdks/typescript/bridge_typescript_web/.gitignorebaml_language/sdks/typescript/bridge_typescript_web/Cargo.tomlbaml_language/sdks/typescript/bridge_typescript_web/package.jsonbaml_language/sdks/typescript/bridge_typescript_web/pnpm-workspace.yamlbaml_language/sdks/typescript/bridge_typescript_web/scripts/build-wasm-release.mjsbaml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-wasm-package.mjsbaml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjsbaml_language/sdks/typescript/bridge_typescript_web/scripts/sync-node-runtime.mjsbaml_language/sdks/typescript/bridge_typescript_web/src/lib.rsbaml_language/sdks/typescript/bridge_typescript_web/tsconfig.jsonbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/index.tsbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/native.tsbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/wasm/bridge_web_core.d.tsbaml_language/stow.tomlscripts/baml-language-version
30b7aec to
38782ae
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
baml_language/sdk_tests/crates/typescript/setup.ps1 (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the em-dash to resolve the encoding warning.
The em-dash (
—) is a non-ASCII character, which triggers aPSScriptAnalyzerwarning (PSUseBOMForUnicodeEncodedFile) because the file lacks a Byte Order Mark (BOM). Replacing it with a regular ASCII dash (-) will resolve the warning without requiring you to change the file's encoding.♻️ Proposed fix
-# Per-fixture pnpm setup for the sdk_test_typescript crate — Windows. +# Per-fixture pnpm setup for the sdk_test_typescript crate - Windows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdk_tests/crates/typescript/setup.ps1` at line 1, Replace the em dash in the setup.ps1 header comment with a standard ASCII hyphen, leaving the rest of the comment and script unchanged.Source: Linters/SAST tools
baml_language/sdk_tests/harness_setup/src/typescript.rs (1)
482-482: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding an
attwcheck for the newbridge_typescript_webpackage.Since this PR unifies the test integration framework and adds the new
bridge_typescript_webpackage, it would be highly beneficial to assert its npm packaging structure is correct usingattw(Are The Types Wrong) inside the generated tests, exactly as is currently done forbridge_typescriptin the lines preceding this block.🚀 Proposed addition for web bridge validation
Modify the top of the test scaffold block to append the parallel web test within
write_fixtures_tests_rs:"sdks/typescript/bridge_typescript", "pnpm exec attw --pack --profile esm-only", "{cache_subdir}", "{cache_env_var}", ); }} }} + +mod bridge_typescript_web {{ + #[test] + fn attw() {{ + ::sdk_test_harness_runner::run_workspace_cmd( + "sdks/typescript/bridge_typescript_web", + "pnpm exec attw --pack --profile esm-only", + "{cache_subdir}", + "{cache_env_var}", + ); + }} +}} "#, cache_subdir = CACHE_SUBDIR,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdk_tests/harness_setup/src/typescript.rs` at line 482, Extend write_fixtures_tests_rs to generate an attw packaging validation for the bridge_typescript_web package, mirroring the existing bridge_typescript check in the surrounding test scaffold. Ensure the generated test verifies the web package’s npm type structure using the same setup and assertions as the existing bridge validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@baml_language/sdk_tests/crates/typescript/setup.ps1`:
- Line 1: Replace the em dash in the setup.ps1 header comment with a standard
ASCII hyphen, leaving the rest of the comment and script unchanged.
In `@baml_language/sdk_tests/harness_setup/src/typescript.rs`:
- Line 482: Extend write_fixtures_tests_rs to generate an attw packaging
validation for the bridge_typescript_web package, mirroring the existing
bridge_typescript check in the surrounding test scaffold. Ensure the generated
test verifies the web package’s npm type structure using the same setup and
assertions as the existing bridge validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1e8df05b-5219-4c3e-8bc2-f1c78b1c5bc6
⛔ Files ignored due to path filters (2)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/sdks/typescript/bridge_typescript_web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
.github/workflows/build2-web-sdk.reusable.yaml.github/workflows/cargo-tests.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish2-web-sdk.yaml.github/workflows/release-baml-language.ymlbaml_language/.config/nextest.tomlbaml_language/Cargo.tomlbaml_language/sdk_tests/DEVELOPMENT.mdbaml_language/sdk_tests/README.mdbaml_language/sdk_tests/crates/python_pydantic2/setup.shbaml_language/sdk_tests/crates/typescript/Cargo.tomlbaml_language/sdk_tests/crates/typescript/llm_functions/customizable/streaming_e2e.node.test.tsbaml_language/sdk_tests/crates/typescript/setup.ps1baml_language/sdk_tests/crates/typescript/setup.shbaml_language/sdk_tests/crates/typescript/src/lib.rsbaml_language/sdk_tests/harness_runner/Cargo.tomlbaml_language/sdk_tests/harness_runner/src/lib.rsbaml_language/sdk_tests/harness_setup/src/lib.rsbaml_language/sdk_tests/harness_setup/src/python_pydantic2.rsbaml_language/sdk_tests/harness_setup/src/templates/package.jsonbaml_language/sdk_tests/harness_setup/src/typescript.rsbaml_language/sdks/typescript/bridge_typescript_web/.gitignorebaml_language/sdks/typescript/bridge_typescript_web/Cargo.tomlbaml_language/sdks/typescript/bridge_typescript_web/package.jsonbaml_language/sdks/typescript/bridge_typescript_web/pnpm-workspace.yamlbaml_language/sdks/typescript/bridge_typescript_web/scripts/build-wasm-release.mjsbaml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-wasm-package.mjsbaml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjsbaml_language/sdks/typescript/bridge_typescript_web/scripts/sync-node-runtime.mjsbaml_language/sdks/typescript/bridge_typescript_web/src/lib.rsbaml_language/sdks/typescript/bridge_typescript_web/tsconfig.jsonbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/index.tsbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/native.tsbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/wasm/bridge_web_core.d.tsbaml_language/stow.tomlscripts/baml-language-version
🚧 Files skipped from review as they are similar to previous changes (20)
- baml_language/sdks/typescript/bridge_typescript_web/.gitignore
- baml_language/sdks/typescript/bridge_typescript_web/Cargo.toml
- baml_language/sdks/typescript/bridge_typescript_web/typescript_src/wasm/bridge_web_core.d.ts
- baml_language/sdks/typescript/bridge_typescript_web/tsconfig.json
- baml_language/sdks/typescript/bridge_typescript_web/scripts/build-wasm-release.mjs
- baml_language/stow.toml
- baml_language/Cargo.toml
- baml_language/sdks/typescript/bridge_typescript_web/package.json
- scripts/baml-language-version
- baml_language/sdk_tests/crates/typescript/setup.sh
- baml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjs
- .github/workflows/publish2-web-sdk.yaml
- baml_language/sdk_tests/harness_setup/src/templates/package.json
- baml_language/sdks/typescript/bridge_typescript_web/scripts/sync-node-runtime.mjs
- baml_language/sdks/typescript/bridge_typescript_web/typescript_src/index.ts
- baml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-wasm-package.mjs
- .github/workflows/release-baml-language.yml
- .github/workflows/ci.yaml
- .github/workflows/build2-web-sdk.reusable.yaml
- baml_language/sdks/typescript/bridge_typescript_web/typescript_src/native.ts
…#4062) ## Summary - add `sdk_test_typescript_web` for browser and Cloudflare Workers test generation - run Web/Workers ESM and TypeScript validation against the scaffold bridge - copy one shared test source tree into every runtime and select runtime-specific suites inline through `BAML_TEST_RUNTIME` - remove the `.node.test.ts`/`.web.test.ts`/`.workers.test.ts` routing convention - import the generated SDK in the runtime-selection test - generate `vitest_web` and `vitest_workers` tests but keep them ignored until BoundaryML#4006 ## Stack BoundaryML#4052 is merged. This PR adds the mergeable test scaffolding on `canary`; BoundaryML#4006 supplies the runtime implementation and removes only the two Vitest ignore annotations. ## Validation - `cargo nextest run -p sdk_test_typescript_web -E all() --no-fail-fast`: 18 passed, 8 skipped - all Web/Workers ESM and TypeScript checks pass - exactly the Web/Workers Vitest checks are skipped - `mise run fmt`, `mise run stow`, and `mise run clippy` pass - cumulative BoundaryML#4006: 26 passed, 0 skipped <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added TypeScript SDK test coverage for browser and Cloudflare Workers runtimes. * Added runtime-aware testing for Node, Web, and Workers environments. * Expanded validation for cancellation, standard-library functions, media, and handle-based values. * **Bug Fixes** * Improved WebAssembly tooling installation reliability across supported build and test workflows. * **Documentation** * Updated SDK testing documentation with the new runtime targets, commands, and setup guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
@boundaryml/baml-bridge-webpackage with browser and workerd exports and a TypeScript API aligned with the sharedbridge_cfficall surface.Runtime boundary
This PR intentionally does not add
sys_wasm, browser fetch/read-file operations, host-call dispatch, or otherSysOpwiring. Runtime initialization exposes the intended API but returns@boundaryml/baml-bridge-web runtime support is not implemented yet; the next PR replaces these stubs with the functional implementation.Impact
This is stack 2 of 3 and is based on
sam/bridge-cffi-wasm. It establishes a releasable package and workflow independently from the platform-specific runtime implementation.Checks
mise run stowmise run fmtmise run clippypnpm buildnpm pack --dry-run --jsonscripts/baml-language-version checkactionlintfor the Web build, publish, and language release workflowsSummary by CodeRabbit
@boundaryml/baml-bridge-webnpm package with default and Workerd entrypoints.