feat!: use target_arch instead of feature flags for wasm vs uniffi #210
+124
−132
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR removes FFI feature flags in favor of using target_arch for selecting between WASM and UniFFI code paths, updates build and CI scripts accordingly, and refactors the mutex and macro crates to align with the new approach.
- Switch from
ffi_uniffi/ffi_wasmfeatures to#[cfg(target_arch = "wasm32")]and#[cfg(not(target_arch = "wasm32"))] - Update cargo manifests and dependencies to target-specific sections
- Refactor
FfiMutex, client manager, macros, docs, and CI to remove explicit feature flags
Reviewed Changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/build_pkgs/src/typescript.rs | Removed explicit --no-default-features/--features ffi_wasm flags |
| scripts/sanity.sh | Changed to per-crate cargo check including WASM target |
| docs/book/crates/algokit_transact_ffi.md | Updated build instructions to use --target wasm32-unknown-unknown |
| crates/ffi_mutex/src/lib.rs | Replaced feature flags with target_arch cfg and updated FfiMutex |
| crates/ffi_mutex/Cargo.toml | Converted feature-based dependencies to target-specific dependencies |
| crates/ffi_macros/src/lib.rs | Updated macro cfg_attr annotations to use target_arch |
| crates/ffi_macros/Cargo.toml | Enabled full feature on syn dependency |
| crates/algokit_utils/src/clients/client_manager.rs | Switched from RwLock to FfiMutex and updated lock calls |
| crates/algokit_utils/Cargo.toml | Added ffi_mutex and WASM-specific getrandom dependency |
| crates/algokit_transact_ffi/src/transactions/keyreg.rs | Switched cfg(feature) to cfg(target_arch) for Tsify import |
| crates/algokit_transact_ffi/src/transactions/application_call.rs | Updated cfg_attr on enum to use target_arch |
| crates/algokit_transact_ffi/src/lib.rs | Refactored FFI functions, enums, and custom types to use target_arch |
| crates/algokit_transact_ffi/Cargo.toml | Removed FFI feature definitions, added target-specific deps |
| crates/algokit_http_client/src/lib.rs | Refactored scaffolding, exports, and attributes to target_arch |
| crates/algokit_http_client/Cargo.toml | Updated dependencies into cfg(target_arch) and non-WASM sections |
| crates/algod_client/Cargo.toml | Removed ffi_uniffi feature, added WASM getrandom dep |
| api/oas_generator/rust_oas_generator/templates/base/Cargo.toml.j2 | Aligned template with new algokit_http_client dependency pattern |
| .github/workflows/ci_cd.yml | Replaced individual checks with calling scripts/sanity.sh |
Comments suppressed due to low confidence (1)
crates/ffi_mutex/src/lib.rs:7
- [nitpick] Update this doc comment to reflect that the selection now uses
target_archcfg attributes instead offfi_uniffi/ffi_wasmfeature flags.
/// A wrapper around `tokio::sync::Mutex` (for uniffi) or `std::cell::RefCell` (for WASM)
joe-p
force-pushed
the
feat/use_target_arch
branch
2 times, most recently
from
095f191 to
1ce3fb6
Compare
neilcampbell
approved these changes
Jul 18, 2025
joe-p
force-pushed
the
feat/use_target_arch
branch
from
d1b995b to
4bcdf6a
Compare
Rust features are intended to be purely additive, so this changes aligns with that philosophy. This also makes it a bit more ergonomic to compile the FFI packages since we no longer need to specify features. It should be noted that the sanity script now iterates over each crate individually to run cargo check. Otherwise, feature unification at the workspace level causes problems. I've also temporary switched the usage of RwLock in utils to FfiMutex so it compiles when targeting WASM. We can come back to this later and optimize down the road Use quotes around dir Co-authored-by: Copilot <[email protected]>
joe-p
force-pushed
the
feat/use_target_arch
branch
from
4bcdf6a to
b452343
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rust features are intended to be purely additive, so this changes aligns with that philosophy. This also makes it a bit more ergonomic to compile the FFI packages since we no longer need to specify features. It should be noted that the sanity script now iterates over each crate individually to run cargo check. Otherwise, feature unification at the workspace level causes problems. I've also temporary switched the usage of RwLock in utils to FfiMutex so it compiles when targeting WASM. We can come back to this later and optimize down the road