refactor(sdk)!: fix type inconsistencies across wasm-sdk and js-evo-sdk#3047
refactor(sdk)!: fix type inconsistencies across wasm-sdk and js-evo-sdk#3047QuantumExplorer merged 2 commits intov3.1-devfrom
Conversation
- Unify optional return types: | null → | undefined in unchecked_return_type - Replace unknown return types with actual wasm types in facades - Use named array type aliases (IdentifierLikeArray, PlatformAddressLikeArray, ProTxHashLikeArray) - Fix TokenLastClaim → RewardDistributionMoment in unchecked_return_type - Use ProTxHashLike | undefined instead of string | Uint8Array | null - Remove unnecessary ?? null coercions - Define *LikeArray types using *Like alias - Fix missing | undefined in proof return types - Remove unused once_cell dependency from rs-platform-version - Fix eslint max-len warnings in facade files Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the
📝 WalkthroughWalkthroughType/interface updates across JS SDK facades and WASM bindings unify array aliases and shift optional results from null to undefined. DocumentsFacade adds six mutation methods delegating to the WASM SDK. Some return types are narrowed to non-optional. Tests switch to testnet SDK. Minor Rust/Cargo and lint changes included. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
packages/wasm-sdk/src/dpns.rs (1)
458-507:⚠️ Potential issue | 🟠 MajorUse
undefinedinstead ofnullfor proof optional data.Both proof helper methods declare
| undefinedin their return type annotation but returnJsValue::NULLon missing data. This violates the type contract—when data is absent, line 506 and line 577 should useJsValue::UNDEFINED.Suggested fix
} else { - JsValue::NULL + JsValue::UNDEFINED };Apply to both locations (lines 506 and 577).
packages/wasm-sdk/src/queries/token.rs (1)
610-638:⚠️ Potential issue | 🟠 MajorFix NULL vs UNDEFINED mismatch in proof response type.
get_token_total_supply_with_proof_infodeclaresunchecked_return_type = "ProofMetadataResponseTyped<TokenTotalSupply | undefined>"but returnsJsValue::NULLinstead ofJsValue::UNDEFINED. This breaks the contract with TypeScript consumers, wherenullandundefinedare semantically distinct types. Other similar functions in this file (get_token_contract_info_with_proof_info,get_token_perpetual_distribution_last_claim_with_proof_info) correctly useJsValue::UNDEFINED.Suggested fix
- let data = supply_result - .map(|supply| JsValue::from(TokenTotalSupplyWasm::new(supply.token_supply as u64))) - .unwrap_or(JsValue::NULL); + let data = supply_result + .map(|supply| JsValue::from(TokenTotalSupplyWasm::new(supply.token_supply as u64))) + .unwrap_or(JsValue::UNDEFINED);packages/wasm-sdk/src/queries/identity.rs (8)
553-579:⚠️ Potential issue | 🟠 MajorSame null vs undefined inconsistency.
Line 555 declares
bigint | undefinedbut line 573 returnsJsValue::NULL.
603-638:⚠️ Potential issue | 🟠 MajorSame null vs undefined inconsistency.
Line 605 declares
bigint | undefinedbut line 631 returnsJsValue::NULL.
657-687:⚠️ Potential issue | 🟠 MajorSame null vs undefined inconsistency.
Line 659 declares
Map<Identifier, bigint | undefined>but line 681 returnsJsValue::NULLfor missing balances.
911-938:⚠️ Potential issue | 🟠 MajorSame null vs undefined inconsistency.
Line 913 declares
bigint | undefinedbut line 932 returnsJsValue::NULL.
940-981:⚠️ Potential issue | 🟠 MajorSame null vs undefined inconsistency.
Line 942 declares
Map<Identifier, bigint | undefined>but line 971 returnsJsValue::NULL.
983-1012:⚠️ Potential issue | 🟠 MajorSame null vs undefined inconsistency.
Line 985 declares
IdentityBalanceAndRevision | undefinedbut line 1006 returnsJsValue::NULL.
1014-1045:⚠️ Potential issue | 🟠 MajorSame null vs undefined inconsistency.
Line 1016 declares
Identity | undefinedbut line 1039 returnsJsValue::NULL.
325-348:⚠️ Potential issue | 🟠 MajorType mismatch: TypeScript declares
undefinedbut Rust returnsnull.The
unchecked_return_typeon line 327 declaresIdentity | undefined, but line 342 returnsJsValue::NULLwhen the identity isNone. In JavaScript,JsValue::NULLis the literalnullvalue, notundefined. This creates a type-safety issue for TypeScript consumers checkingresult === undefined, which will be false when the value isnull.Change
JsValue::NULLtoJsValue::UNDEFINEDto match the declared type:Diff
let data: JsValue = match identity { Some(identity) => IdentityWasm::from(identity).into(), - None => JsValue::NULL, + None => JsValue::UNDEFINED, };This same pattern applies to other methods in this file where
JsValue::NULLis used but the type declares| undefined.
🧹 Nitpick comments (3)
packages/js-evo-sdk/tests/unit/facades/identities.spec.ts (3)
35-36: Consider adding test coverage forgetIdentitiesContractKeysWithProofInfoStub.The ESLint suppression indicates this stub is declared but unused. There's a
contractKeys()test but no correspondingcontractKeysWithProof()test to exercise this stub.Would you like me to generate a test case for
contractKeysWithProof()to match the pattern of other*WithProofmethods?
79-84: Inconsistent indentation in multi-line stub chain.The object properties on lines 81-83 are indented at 6 spaces, but similar patterns elsewhere (e.g., lines 103-105) use 8 spaces for proper alignment under the
.resolves({call.🔧 Suggested fix
getIdentityContractNonceWithProofInfoStub = this.sinon .stub(wasmSdk, 'getIdentityContractNonceWithProofInfo').resolves({ - data: BigInt(0), - proof: {}, - metadata: {}, - }); + data: BigInt(0), + proof: {}, + metadata: {}, + });
130-135: Same indentation inconsistency as noted above.Apply the same fix pattern here for consistency.
🔧 Suggested fix
getIdentityTokenBalancesWithProofInfoStub = this.sinon .stub(wasmSdk, 'getIdentityTokenBalancesWithProofInfo').resolves({ - data: new Map(), - proof: {}, - metadata: {}, - }); + data: new Map(), + proof: {}, + metadata: {}, + });
Issue being fixed or feature implemented
Type inconsistencies between the wasm-sdk Rust layer and the js-evo-sdk TypeScript facades resulted in incorrect nullability annotations, unknown return types, and inconsistent parameter types that hurt developer experience and could mask runtime errors.
What was done?
| nullto| undefinedinunchecked_return_typemacro to match JavaScript/TypeScript conventions where WASM returnsundefined(notnull) for absent valuesunknownreturn types with actual wasm types in all js-evo-sdk facade methods (e.g.,Promise<unknown>→Promise<wasm.TokenMintResult>)IdentifierLikeArray,PlatformAddressLikeArray,ProTxHashLikeArrayfor cleaner function signatures instead of rawIdentifierLike[]TokenLastClaim→RewardDistributionMomentmapping inunchecked_return_typeto match the actual Rust type exportProTxHashLikenullability: UseProTxHashLike | undefinedinstead ofstring | Uint8Array | null?? nullcoercions in facade methods where the wasm layer already handles optional parameters| undefinedin proof return types that incorrectly declared non-optional resultsonce_celldependency fromrs-platform-versionHow Has This Been Tested?
js-evo-sdkupdated and passing (e.g.,identities.spec.tsmock adjustments)Breaking Changes
No breaking changes at runtime. TypeScript return types are now more precise (e.g.,
unknown→ specific type,| null→| undefined), which may surface compile-time errors in downstream code that was relying on the looser types.Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Refactor
Tests
Chores