-
Notifications
You must be signed in to change notification settings - Fork 41
Add ubrn build web --and-generate command
#253
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
Conversation
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 adds the “ubrn build web --and-generate” command to build the wasm target and generate the corresponding template files and bindings. Key changes include the introduction of new wasm template files (index.web.ts and Cargo.toml), new wasm-related modules (commands, config, bindings) and updates to the build command infrastructure to support the web build.
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ubrn_cli/templates/wasm/index.web.ts | New template file for the wasm entry point using templating loops. |
| crates/ubrn_cli/templates/wasm/Cargo.toml | New Cargo.toml template for the wasm crate with dynamic features. |
| crates/ubrn_cli/src/wasm/* | New modules for wasm configuration, commands, and bindings integration. |
| crates/ubrn_cli/src/jsi/* | Renaming of iOS and Android command structures to improve clarity. |
| crates/ubrn_cli/src/commands/building.rs | Extended build command support to include a new “Web” variant with a then_build hook. |
| crates/ubrn_cli/src/commands/generate.rs | Added conditional generation logic for WASM vs. JSI bindings. |
| crates/ubrn_cli/src/config/* | Added new fields (e.g. project_version) and validation hooks for manifest paths. |
| crates/ubrn_bindgen/src/bindings/gen_rust/mod.rs | Minor adjustments in code generation for callback modules. |
Comments suppressed due to low confidence (1)
crates/ubrn_cli/src/commands/building.rs:56
- The new Web build command and its then_build logic should have accompanying tests or integration examples to verify functionality, particularly given the uncertainty expressed in the PR description regarding testing with create-react-native-library and yarn create expo-app.
self.cmd.then_build()
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 adds the new "ubrn build web --and-generate" command functionality for building and generating WebAssembly bindings and TypeScript files. Key changes include updates to wasm build configuration and commands, renaming of platform-specific build arguments for consistency, and modifications in code generation and binding routines.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ubrn_cli/templates/wasm/Cargo.toml | Adds a templated Cargo.toml for the wasm crate with conditional blocks. |
| crates/ubrn_cli/templates/jsi/crossplatform/index.tsx | Introduces a no-op async initialization function for parity with web. |
| crates/ubrn_cli/src/wasm/* | Adds new modules (bindings, commands, config) and exports WASM config. |
| crates/ubrn_cli/src/jsi/* | Renames iOS and Android build argument structs for consistency. |
| crates/ubrn_cli/src/config/* | Updates configuration parsing for manifest paths and project version. |
| crates/ubrn_cli/src/commands/* | Introduces WebBuildArgs and updates build command logic accordingly. |
| crates/ubrn-bindgen/src/bindings/gen_rust/mod.rs | Adjusts identifier naming inside generated Rust binding code. |
Comments suppressed due to low confidence (2)
crates/ubrn_bindgen/src/bindings/gen_rust/mod.rs:360
- [nitpick] The alternate identifier '_rs_return' is used when no return type is present. Consider adding an inline comment to clarify the rationale for choosing different identifiers in these cases to improve maintainability.
let rs_ret_name = if rt.is_some() { "rs_return_" } else { "_rs_return" };
crates/ubrn_cli/src/config/rust_crate.rs:79
- [nitpick] The custom deserialization for manifest_path checks that the string ends with 'Cargo.toml'. It may be beneficial to enhance the validation (or error message) by clarifying if the path should be relative or conform to additional expected patterns.
if !path_str.ends_with("Cargo.toml") {
| let mut files = match self { | ||
| Self::Android(a) => a.build()?, | ||
| Self::Ios(a) => a.build()?, | ||
| Self::Web(a) => a.build()?, |
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.
does this code need wasm cfg/clap stuff as well, won't it be undefined if the wasm feature is missing?
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.
Yes, you're right. I wonder why clippy didn't spot this.
zzorba
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.
LGTM, couple possible nits
# 0.29.3-0 ## ✨ What's New ✨ ### 🌏🕸️ WASM! After 6 months of development, we are releasing the first version of `uniffi-bindgen-react-native` for use with WASM: - Different fixtures running: - Fixtures `chronological` and `gc-callbacks-crasher` (#238) - Fixture `async-callbacks` (#237) - Configuration file and `ubrn` command line: - Enable entrypoint and ts bindings directory to be customized for wasm (#259) - Add `ubrn build web --and-generate` command (#253) - Add CLI testing for `uniffi-bindgen-react-native` command. (#257) - Refactor of ubrn_cli into config and commands modules (#251) - `uniffi-runtime-javascript` runtime, now on `crates.io`: - Add runtimeVersion to vary version of uniffi-runtime-javascript (#256) - Prepare uniffi-runtime-javascript crate for release (#248) ## 🦊 What's Changed - Add default value for the --config option in all ubrn commands (#265) - Change Windows path separators in CMakeLists.txt (#261) - Bump `uniffi-rs` version to 0.29.3 (#267) - Bump bob & RN versions (#242) and (#260) - Run yarn pack as part of compatibility tests (#250) - Add to "who is using" section of readme (#239) - Fix wrong key name of `manifestPath` in docs (#240) ##⚠️ Breaking Changes - Bump Typescript version to 5.8, affecting `ArrayBuffer` (#271) **Full Changelog**: 0.29.0-0...0.29.3-3
This adds extra functionality to the
ubrn build webcommand; the following is done automatically:cargo build --target wasm32-unknown-unknown, with the correct profile, taken from the command line.wasm-bindgenon the resulting wasm bundle.More command line options are added:
--no-generate: opts out of all except the initialcargo build.--no-wasm-pack: opts out of the build for the wasm32-unknown-unknown and wasm-bindgen, which does the same aswasm-pack build.--target: changes the target passed to wasm-bindgenExtras added to the config file:
Tests are missing at the moment, awaiting a better way of doing integration testing.