Skip to content

Migrate scval.js to TS#926

Open
Ryang-21 wants to merge 5 commits intotypescript-migrationfrom
tsm-scval
Open

Migrate scval.js to TS#926
Ryang-21 wants to merge 5 commits intotypescript-migrationfrom
tsm-scval

Conversation

@Ryang-21
Copy link
Contributor

  • Migrated relevant scValToNative and nativeToScVal tests from tests/numbers/index.test.ts into isolated test file
  • Added additional tests for scValToNative and nativeToScVal
  • Created isolated unit tests for scValToBigInt
  • Removed tests/numbers/index.test.ts as all tests were migrated out

@Ryang-21 Ryang-21 requested a review from Copilot March 10, 2026 21:50
@Ryang-21 Ryang-21 linked an issue Mar 10, 2026 that may be closed by this pull request
5 tasks
@github-actions
Copy link

github-actions bot commented Mar 10, 2026

Size Change: +13.7 kB (+0.38%)

Total Size: 3.62 MB

Filename Size Change
dist/stellar-base.js 2.67 MB +10.2 kB (+0.38%)
dist/stellar-base.min.js 951 kB +3.54 kB (+0.37%)

compressed-size-action

Copy link

Copilot AI left a 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 migrates scval utilities to TypeScript and reorganizes/expands unit test coverage around nativeToScVal, scValToNative, and scValToBigInt.

Changes:

  • Migrates src/scval implementation to TypeScript with new exported typings and a standalone scvSortedMap export (while keeping the xdr.scvSortedMap monkey-patch for compatibility).
  • Replaces the prior “numbers” test aggregator with isolated Vitest test files, adding substantial new coverage for ScVal/native conversion.
  • Adds a dedicated scValToBigInt test suite and removes the old test/unit/numbers/index.test.ts.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/scval.ts TypeScript migration of core ScVal <-> native conversions and scvSortedMap implementation.
type_validation/scval.d.ts Generated declaration output for scval after TS migration.
test/unit/scval.test.ts New isolated Vitest suite for nativeToScVal, scValToNative, and scvSortedMap.
test/unit/numbers/scval_to_bigint.test.ts New isolated Vitest suite for scValToBigInt.
test/unit/numbers/index.test.ts Removed since tests were migrated into isolated files.
Comments suppressed due to low confidence (3)

src/scval.ts:418

  • Same issue as above for scvString: on decode failure, new Uint8Array((v as ArrayBufferView).buffer) can include bytes outside the view due to byteOffset/byteLength being ignored. Use a view that respects offsets/lengths (and copy if needed) so the raw-bytes fallback matches the actual XDR string payload only.
    src/scval.ts:266
  • In the number | bigint branch, the u32/i32 cases cast val to number but don't convert at runtime. When val is a bigint (e.g. 100n), this will pass a bigint into xdr.ScVal.scvU32/scvI32, which are typed to accept number and will likely throw at runtime. Convert explicitly (e.g. Number(val)) and consider range-checking to ensure the bigint fits in 32 bits before constructing the ScVal.
    src/scval.ts:405
  • The fallback path when TextDecoder fails uses new Uint8Array((v as ArrayBufferView).buffer), which ignores byteOffset/byteLength for Buffer/TypedArray views. For pooled Buffers this can return extra unrelated bytes beyond the actual string contents. Construct the Uint8Array using buffer, byteOffset, and byteLength (and copy if desired) so the returned bytes exactly match the original view.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate scval.js file to TS

2 participants