Conversation
Access the keystore through Client's authenticator instead of storing it separately. Add authenticator() and store() getters to Client<AUTH>, refactor setup_client to accept store/keystore/rng as parameters, and extract create_rng helper for the seed-to-RpoRandomCoin logic.
The slotName→root difference is a base branch issue (next hasn't been merged into jmunoz-decouple-store-from-web yet). Reverting to keep our diff focused on the store generalization.
Move wrap_send to platform.rs as maybe_wrap_send (no-op on browser, AssertSend wrapper on nodejs), unify get_mut_inner() and keystore() into shared impl blocks, and extract build_error_chain from the duplicated js_error_with_context implementations.
…unctions Store export/import is not the client's responsibility. The store field (cfg-gated as Arc<dyn WebStore> for browser and Arc<dyn Store> for nodejs) was only needed for exportStore/forceImportStore methods and was stored-but-never-read on nodejs. exportStore and forceImportStore are now standalone wasm_bindgen functions that take a store_name parameter, keeping the functionality available without coupling it to the client. Tests updated accordingly.
debug_mode was only consumed once during setup_client and required a two-step setDebugMode() + createClient() pattern. Now it's an optional parameter on createClient directly, simplifying the struct.
Use Vec::from() with the existing From trait impls on array types instead of directly accessing the browser-specific __inner field. This eliminates ~20 cfg blocks across model files. Also unify settings remove/list methods, serialize_to_bytes, and bytes_to_js to work identically on both platforms. Fix rollup.config.js to include the browser feature alongside testing.
Five functions changed from sync to async in the wasm_bindgen -> js_export migration: newMintTransactionRequest, newSendTransactionRequest, newSwapTransactionRequest, createCodeBuilder, and accountReader. All test call sites now properly await these promises.
…ter name - Rpo256::hash_elements and AdviceMap::insert: change FeltArray param back to &FeltArray (borrow) to match next branch behavior. By-value params consume the JS object, causing null pointer when reused. - FetchedNote::get_note getter: add js_name = "note" so the JS property is "note" instead of "get_note" (on next, the method was fn note()).
…c-clean # Conflicts: # Cargo.lock # crates/idxdb-store/src/lib.rs # crates/rust-client/src/lib.rs # crates/web-client/Cargo.toml # crates/web-client/js/client.js # crates/web-client/js/types/api-types.d.ts # crates/web-client/src/account.rs # crates/web-client/src/export.rs # crates/web-client/src/import.rs # crates/web-client/src/lib.rs # crates/web-client/src/mock.rs # crates/web-client/src/models/account_component.rs # crates/web-client/src/models/account_reader.rs # crates/web-client/src/models/account_storage_requirements.rs # crates/web-client/src/models/auth_secret_key.rs # crates/web-client/src/models/components/auth_falcon512_rpo_multisig.rs # crates/web-client/src/models/storage_map.rs # crates/web-client/src/new_account.rs # crates/web-client/src/rpc_client/mod.rs # crates/web-client/src/sync.rs # crates/web-client/test/import_export.test.ts # crates/web-client/test/miden_client_api.test.ts # crates/web-client/test/new_transactions.test.ts # crates/web-client/yarn.lock
- Add createMockClient for Node.js (SqliteStore + FilesystemKeyStore) - Change mock_rpc_api/mock_note_transport_api fields to AsyncCell for &self access - Add Drop impl to prevent Tokio runtime panic on Node.js exit - Add node-adapter.ts: fake page, window.* globals, type normalization - Modify playwright setup to support nodejs project alongside browser - Add nodejs project to playwright config - Add Express demo API server (examples/node-demo/) - Add vitest-based test files (test/node/) and shared fixtures (test/shared/) - 148/190 existing browser tests pass on Node.js with zero test changes
Wraps AccountBuilder, AccountComponent, AuthSecretKey with arg normalization for static methods. Deduplicates getWasmOrThrow. Node.js tests: 160/190 passing (up from 148).
04e8ea0 to
caf38ec
Compare
|
Note that there may be some NodeJS specific things to improve, but if possible we should try to focus the review on the broader changes that may have an impact in the existing codebase. Any details of NodeJS can be addressed in follow-ups if they are not important. I just leave this comment because the PR is incredibly large and it's best to address the most relevant things now and if it's mergeable we can go for it and leave the rest of the points to tackle in a follow-up issue. |
There was a problem hiding this comment.
I like the js_export macro idea! It's a shame that it has to be shipped separately :(.
AFAIK, this is a pain point that other projects also encounter (stadiamaps/ferrostar#289 comes to mind).
I left a couple of comments, mainly regarding readability.
| if has_jsu64(&method) { | ||
| // Tag the method with its js_export args for later. | ||
| method.attrs.push(syn::parse_quote!(#[__js_export_args(#method_attr_tokens)])); | ||
| jsu64_methods.push(method); | ||
| } else { |
There was a problem hiding this comment.
Why are checking if the method has U64? Could we leave a comment explaining the rationale?
Edit: Found out why a bit below, maybe it'd be worth pointing to the make_platform_method in a comment
Edit 2: I also read that it's on top level doc!
| for member in item.items.drain(..) { | ||
| match member { |
There was a problem hiding this comment.
nit: Maybe to increase readability a bit I would a couple of comments stating what we're doing.
Mainly because syn's API can be a bit unintuitive from time to time.
| let output = match item { | ||
| Item::Struct(s) => handle_struct(&attr, s), | ||
| Item::Enum(e) => handle_enum(&attr, e), | ||
| Item::Impl(i) => handle_impl(&attr, i), |
There was a problem hiding this comment.
In the context of js bindings, functions can only be methods of structs, correct?
If that's the case, maybe I'd add a comment with a link to the docs. So that code that reflects this (e.g. [methods](So things like https://github.com/0xMiden/miden-client/pull/1908/changes#diff-2d13f93b3c68ba298247a3786c2780f5aea5b651ff05948645a918d6f8695b30R87
)) are also explained in the code.
| } | ||
|
|
||
| /// Extract the `#[__js_export_args(...)]` marker we stashed earlier. | ||
| fn extract_stashed_args(method: &mut ImplItemFn) -> TokenStream2 { |
There was a problem hiding this comment.
nit: I'm not sure I understand the rationale behind fn extract_stashed_args(method: &mut ImplItemFn) -> TokenStream2 {
It seems to me that we are adding the __js_export_args argument on line 95, just so we can remove it later on line 169, inside the make_platform_method.
Can't this flow be unified on one place?
Adds a Node.js build of the web client using napi-rs. The same Rust crate compiles to both wasm32 (browser, via wasm-bindgen) and native (Node.js, via napi-rs) using mutually exclusive feature flags (
browservsnodejs).#[js_export]macroA new proc macro replaces
#[wasm_bindgen]on exported items. It expands to#[cfg_attr(feature = "browser", wasm_bindgen)]+#[cfg_attr(feature = "nodejs", napi)]. When a method usesJsU64in its signature (u64/BigInt on browser, f64/number on Node.js), the macro splits it into two platform-specific impl blocks automatically.A
platform.rsmodule provides type aliases that switch per feature (JsErr,JsBytes,AsyncCell,ClientAuth, etc.), letting most methods be written once. Onlycreate_client,create_mock_client, andsettingsneed separate implementations due to fundamental platform differences (IndexedDB vs SQLite, WebKeyStore vs FilesystemKeyStore).Note on the macro: We could theoretically remove the macro but it might be worth keeping it in order to reduce the amount of lines and boilerplate we have in the rest of the code. It's just a tradeoff that we might want, but in case there's a strong reason for removing it we could do it.
Single npm package with conditional exports
The same
@miden-sdk/miden-sdkpackage now works in both browser and Node.js. Users write the same import in both environments:The
package.jsonuses conditional exports ("browser"and"node"conditions) so the runtime or bundler automatically selects the right entry point. The browser entry (dist/index.js) loads WASM + IndexedDB. The Node.js entry (js/node-index.js) loads the napi native binary + SQLite. TheMidenClientclass and all resource classes (accounts,transactions,notes, etc.) are shared between both backends.A Node.js normalization layer (
js/node/) handles API differences between napi and wasm-bindgen: BigInt to Number conversion, null to undefined for Option types, snake_case method aliases, and array type polyfills.Platform binary distribution
Native Node.js binaries are distributed via platform-specific npm packages (
@miden-sdk/node-darwin-arm64,@miden-sdk/node-darwin-x64,@miden-sdk/node-linux-x64-gnu). These are listed asoptionalDependencies(injected at publish time) so npm installs only the matching platform. A loader with fallback to the localtarget/directory supports development without prebuilt binaries.Note: We inject optional dependencies at publish time because if they were in the package.json then yarn install would fail because it tries to get those packages from npm and they don't exist yet.
Publishing changes
The existing publish workflows (
publish-web-client-next.ymlandpublish-web-client-release.yml) now include a matrix build step that compiles native binaries on 3 platforms (macOS ARM, macOS x64, Linux x64) and publishes the platform packages before the main package. Thejs-export-macrocrate is enabled for publishing to crates.io as users need it to compilemiden-client-web.Platform-agnostic tests
20 test files were rewritten to a platform-agnostic format using a
runfixture that abstracts the runtime:On Node.js,
runcalls the callback directly with a napi client (SQLite store). On browser, it serializes the callback and executes it inside a page viapage.evaluate()(WASM + IndexedDB). A normalization layer handles API differences between platforms (null to undefined, BigInt to Number, method name aliases).7 browser-only test files (
store_isolation,sync_lock,import_export, etc.) are skipped on Node.js.Additional Notes
Closes #1667