Repository navigation
test(typescript): add browser and Workers Vitest runtimes - #4062
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR separates Node and Web/Workers TypeScript SDK test generation, adds a dedicated web test crate and runtime selection, updates related tests and documentation, and changes wasm-pack installation from the Cargo source to the GitHub source. ChangesTypeScript SDK test infrastructure
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Nextest
participant TypeScriptWebHarness
participant GeneratedSDK
participant Vitest
Nextest->>TypeScriptWebHarness: prepare web test crate
TypeScriptWebHarness->>GeneratedSDK: generate web and workers outputs
GeneratedSDK->>Vitest: execute runtime-specific checks
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
f6ad214 to
f5fe46c
Compare
f5fe46c to
d4c4543
Compare
d4c4543 to
1b59380
Compare
1b59380 to
16f7ba2
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16f7ba2a3f
ℹ️ 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".
| buffer.push_str( | ||
| r#" | ||
| #[test] | ||
| #[ignore = "typescript/web runtime lands in the Web implementation change"] |
There was a problem hiding this comment.
When a fixture has browser tests, this emits #[ignore] on vitest_web (and the same pattern is used for vitest_workers just below), so the CI step that runs cargo nextest run -p sdk_test_typescript_web -E 'package(=sdk_test_typescript_web)' will skip the new browser/Workers Vitest commands entirely unless --run-ignored is added. That leaves only the ESM/tsc checks running and defeats the runtime coverage added by this crate; remove these ignores or explicitly run ignored tests.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@baml_language/sdk_tests/DEVELOPMENT.md`:
- Around line 119-120: Update the setup-guard section in DEVELOPMENT.md to
include SDK_TEST_TYPESCRIPT_WEB_SETUP=1 and reference
harness_setup/src/typescript_web.rs, alongside the existing Python and Node
setup entries, so the Web TypeScript harness contract is documented.
In `@baml_language/sdk_tests/harness_setup/src/templates/test_runtime.ts`:
- Around line 9-10: Update the configuredRuntime initialization to guard access
to process before reading process.env, while preserving the
import.meta.env.BAML_TEST_RUNTIME value and fallback behavior. Ensure
environments without a global process do not throw a ReferenceError.
In `@baml_language/sdk_tests/harness_setup/src/typescript.rs`:
- Around line 79-89: Update the harness setup flow around write_codegen_output
so it creates the baml_sdk directory itself and reports directory-creation
failures through BuildDiagnostics instead of unwrapping. Replace the
test_runtime.ts fs::write unwrap with diagnostic-aware error handling,
preserving the build’s soft-fail behavior and allowing workspace builds to
continue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4e0aaa0f-03d7-49b3-b3b9-6dc27b41aa9c
⛔ 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 (45)
.github/actions/setup-mise/action.yml.github/workflows/build2-web-sdk.reusable.yaml.github/workflows/cargo-tests.reusable.yaml.github/workflows/wasm-pack-tests.reusable.yaml.github/workflows/webview-tests.reusable.yamlbaml_language/.config/nextest.tomlbaml_language/Cargo.tomlbaml_language/sdk_tests/DEVELOPMENT.mdbaml_language/sdk_tests/README.mdbaml_language/sdk_tests/crates/typescript/build.rsbaml_language/sdk_tests/crates/typescript/docstrings_etc/customizable/main.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/cancellation.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/cancellation.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/runtime.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/runtime.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/stdlib_entrypoints.node.test.tsbaml_language/sdk_tests/crates/typescript/function_calls/customizable/stdlib_entrypoints.test.tsbaml_language/sdk_tests/crates/typescript/llm_functions/customizable/replay_harness.tsbaml_language/sdk_tests/crates/typescript/llm_functions/customizable/streaming_e2e.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/crates/typescript/type_shapes/customizable/roundtrip_handles.node.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_handles.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_media.node.test.tsbaml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_media.test.tsbaml_language/sdk_tests/crates/typescript_web/Cargo.tomlbaml_language/sdk_tests/crates/typescript_web/build.rsbaml_language/sdk_tests/crates/typescript_web/setup.ps1baml_language/sdk_tests/crates/typescript_web/setup.shbaml_language/sdk_tests/crates/typescript_web/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/templates/package_node.jsonbaml_language/sdk_tests/harness_setup/src/templates/package_web.jsonbaml_language/sdk_tests/harness_setup/src/templates/test_runtime.tsbaml_language/sdk_tests/harness_setup/src/templates/tsconfig_web.jsonbaml_language/sdk_tests/harness_setup/src/templates/vitest_node.config.tsbaml_language/sdk_tests/harness_setup/src/templates/vitest_web.config.tsbaml_language/sdk_tests/harness_setup/src/templates/vitest_workers.config.tsbaml_language/sdk_tests/harness_setup/src/typescript.rsbaml_language/sdk_tests/harness_setup/src/typescript_web.rsbaml_language/sdks/typescript/bridge_typescript_web/package.jsonmise.toml
💤 Files with no reviewable changes (5)
- baml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_media.node.test.ts
- baml_language/sdk_tests/crates/typescript/function_calls/customizable/stdlib_entrypoints.node.test.ts
- baml_language/sdk_tests/crates/typescript/type_shapes/customizable/roundtrip_handles.node.test.ts
- baml_language/sdk_tests/crates/typescript/function_calls/customizable/cancellation.node.test.ts
- baml_language/sdk_tests/crates/typescript/function_calls/customizable/runtime.node.test.ts
| - For Node TypeScript: `sdk_test_typescript` builds only `bridge_typescript`, installs Node-only fixture manifests, and runs `esm_node`, `tsc_node`, `vitest_node`, and `attw`. | ||
| - For Web TypeScript: `sdk_test_typescript_web` builds only `bridge_typescript_web`, copies the canonical sibling tests into local generated Web/Workers trees, installs Chromium, and runs the Web and Workers ESM and TypeScript checks. The Web and Workers Vitest checks remain ignored until the Web implementation change is applied. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the Web setup breadcrumb.
The setup-guard section below still lists only Python and Node. Add SDK_TEST_TYPESCRIPT_WEB_SETUP=1 and reference harness_setup/src/typescript_web.rs so the documented contract covers the newly introduced harness.
🤖 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/DEVELOPMENT.md` around lines 119 - 120, Update the
setup-guard section in DEVELOPMENT.md to include SDK_TEST_TYPESCRIPT_WEB_SETUP=1
and reference harness_setup/src/typescript_web.rs, alongside the existing Python
and Node setup entries, so the Web TypeScript harness contract is documented.
| const configuredRuntime = | ||
| import.meta.env.BAML_TEST_RUNTIME ?? process.env.BAML_TEST_RUNTIME; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Safeguard process.env access to prevent ReferenceError in non-Node environments.
In standard browser or Cloudflare Workers environments, process may be undefined. If import.meta.env.BAML_TEST_RUNTIME evaluates to undefined, the fallback to process.env will throw a ReferenceError. Safely check if process is defined before accessing process.env.
🛡️ Proposed fix to safely access `process`
const configuredRuntime =
- import.meta.env.BAML_TEST_RUNTIME ?? process.env.BAML_TEST_RUNTIME;
+ import.meta.env.BAML_TEST_RUNTIME ?? (typeof process !== "undefined" ? process.env?.BAML_TEST_RUNTIME : undefined);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const configuredRuntime = | |
| import.meta.env.BAML_TEST_RUNTIME ?? process.env.BAML_TEST_RUNTIME; | |
| const configuredRuntime = | |
| import.meta.env.BAML_TEST_RUNTIME ?? (typeof process !== "undefined" ? process.env?.BAML_TEST_RUNTIME : undefined); |
🤖 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/templates/test_runtime.ts` around
lines 9 - 10, Update the configuredRuntime initialization to guard access to
process before reading process.env, while preserving the
import.meta.env.BAML_TEST_RUNTIME value and fallback behavior. Ensure
environments without a global process do not throw a ReferenceError.
| fs::create_dir_all(node.join("baml_sdk")).unwrap(); | ||
| let output = sdkgen_typescript::to_source_code_with_bytecode( | ||
| &loaded.pool, | ||
| &loaded.baml_bytecode, | ||
| NamingConvention::PreserveCase, | ||
| ); | ||
| write_codegen_output( | ||
| generated.join("node/baml_sdk"), | ||
| node_output, | ||
| fixture, | ||
| diagnostics, | ||
| ); | ||
| let web_output = sdkgen_typescript_web::to_source_code_with_bytecode( | ||
| &pool, | ||
| &baml_bytecode, | ||
| NamingConvention::PreserveCase, | ||
| ); | ||
| write_codegen_output( | ||
| generated.join("web/baml_sdk"), | ||
| web_output.clone(), | ||
| fixture, | ||
| diagnostics, | ||
| ); | ||
| write_codegen_output( | ||
| generated.join("workers/baml_sdk"), | ||
| web_output, | ||
| fixture, | ||
| diagnostics, | ||
| ); | ||
|
|
||
| for runtime in TestRuntime::ALL { | ||
| if custom.exists() { | ||
| copy_customizable_for_runtime(custom, &generated.join(runtime.dir()), runtime); | ||
| } | ||
| write_codegen_output(node.join("baml_sdk"), output, fixture, diagnostics); | ||
| if custom.exists() { | ||
| copy_customizable(custom, &node); | ||
| } | ||
| rewrite_test_bridge_imports(&generated.join("web")); | ||
| rewrite_test_bridge_imports(&generated.join("workers")); | ||
| fs::write(node.join("test_runtime.ts"), TEST_RUNTIME).unwrap(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Preserve the build script’s soft-fail contract.
Directory creation and test_runtime.ts writes panic here, bypassing BuildDiagnostics and aborting workspace builds. Let write_codegen_output create the SDK directory and record the runtime-template write failure.
Proposed fix
let node = generated.join("node");
- fs::create_dir_all(node.join("baml_sdk")).unwrap();
let output = sdkgen_typescript::to_source_code_with_bytecode(
&loaded.pool,
&loaded.baml_bytecode,
NamingConvention::PreserveCase,
);
write_codegen_output(node.join("baml_sdk"), output, fixture, diagnostics);
if custom.exists() {
copy_customizable(custom, &node);
}
- fs::write(node.join("test_runtime.ts"), TEST_RUNTIME).unwrap();
+ if let Err(error) = fs::write(node.join("test_runtime.ts"), TEST_RUNTIME) {
+ diagnostics.record("test_runtime_write", fixture, error);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fs::create_dir_all(node.join("baml_sdk")).unwrap(); | |
| let output = sdkgen_typescript::to_source_code_with_bytecode( | |
| &loaded.pool, | |
| &loaded.baml_bytecode, | |
| NamingConvention::PreserveCase, | |
| ); | |
| write_codegen_output( | |
| generated.join("node/baml_sdk"), | |
| node_output, | |
| fixture, | |
| diagnostics, | |
| ); | |
| let web_output = sdkgen_typescript_web::to_source_code_with_bytecode( | |
| &pool, | |
| &baml_bytecode, | |
| NamingConvention::PreserveCase, | |
| ); | |
| write_codegen_output( | |
| generated.join("web/baml_sdk"), | |
| web_output.clone(), | |
| fixture, | |
| diagnostics, | |
| ); | |
| write_codegen_output( | |
| generated.join("workers/baml_sdk"), | |
| web_output, | |
| fixture, | |
| diagnostics, | |
| ); | |
| for runtime in TestRuntime::ALL { | |
| if custom.exists() { | |
| copy_customizable_for_runtime(custom, &generated.join(runtime.dir()), runtime); | |
| } | |
| write_codegen_output(node.join("baml_sdk"), output, fixture, diagnostics); | |
| if custom.exists() { | |
| copy_customizable(custom, &node); | |
| } | |
| rewrite_test_bridge_imports(&generated.join("web")); | |
| rewrite_test_bridge_imports(&generated.join("workers")); | |
| fs::write(node.join("test_runtime.ts"), TEST_RUNTIME).unwrap(); | |
| let output = sdkgen_typescript::to_source_code_with_bytecode( | |
| &loaded.pool, | |
| &loaded.baml_bytecode, | |
| NamingConvention::PreserveCase, | |
| ); | |
| write_codegen_output(node.join("baml_sdk"), output, fixture, diagnostics); | |
| if custom.exists() { | |
| copy_customizable(custom, &node); | |
| } | |
| if let Err(error) = fs::write(node.join("test_runtime.ts"), TEST_RUNTIME) { | |
| diagnostics.record("test_runtime_write", fixture, error); | |
| } |
🤖 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` around lines 79 -
89, Update the harness setup flow around write_codegen_output so it creates the
baml_sdk directory itself and reports directory-creation failures through
BuildDiagnostics instead of unwrapping. Replace the test_runtime.ts fs::write
unwrap with diagnostic-aware error handling, preserving the build’s soft-fail
behavior and allowing workspace builds to continue.
Summary
sdk_test_typescript_webfor browser and Cloudflare Workers test generationBAML_TEST_RUNTIME.node.test.ts/.web.test.ts/.workers.test.tsrouting conventionvitest_webandvitest_workerstests but keep them ignored until feat(typescript-web): implement browser and Workers runtimes #4006Stack
#4052 is merged. This PR adds the mergeable test scaffolding on
canary; #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 skippedmise run fmt,mise run stow, andmise run clippypassSummary by CodeRabbit
New Features
Bug Fixes
Documentation