ts-sdk: coerce numeric inputs to bigint in id-like constructors#5041
Open
Ludv1gL wants to merge 2 commits into
Open
ts-sdk: coerce numeric inputs to bigint in id-like constructors#5041Ludv1gL wants to merge 2 commits into
Ludv1gL wants to merge 2 commits into
Conversation
The wrapper constructors for `Identity`, `ConnectionId`, `Timestamp`, `TimeDuration`, and `Uuid` all claim to take `bigint` (or `string | bigint` for `Identity`), but real-world callers can end up passing a `number` whenever data round-trips through JSON — `JSON.parse` has no way to produce a bigint, so HTTP responses, custom state caches, and JSON-format SDK endpoints can quietly downgrade a u128/u256 to a lossy number. Before this patch the constructors stored that number verbatim into a field typed as `bigint`, and the failure manifested several layers later in BSATN serialization as `TypeError: Cannot mix BigInt and other types`, with no indication of which call site was at fault. Wrap the non-string branch of each constructor in `BigInt(...)`: * `bigint` inputs are returned unchanged. * `number` inputs are coerced cleanly (and lose precision the same way `JSON.parse` already lost it — this just stops the crash). * `undefined` / objects throw a clear `TypeError` immediately, pointing at the constructor call instead of a deep serialization frame. No public type changes; strict-TS callers see no difference. Adds a small test file pinning the new behavior across all five classes.
The previous comment on Identity's constructor cited the HTTP `/sql` endpoint as a JSON path, but the TS SDK doesn't call `/sql` anywhere. The real number-input source for this constructor is caller-side code outside the SDK (custom state caches, hand-rolled HTTP clients, codegen drift, etc.). Update the comment to be honest about that. No code change.
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
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.
Description of Changes
The wrapper constructors for
Identity,ConnectionId,Timestamp,TimeDuration, andUuidall claim to takebigint(orstring | bigintforIdentity), but real-world callers can end up passing anumberwhenever data round-trips through JSON in caller-side code —JSON.parsehas no way to produce a bigint, so a u128/u256 that goes through any JSON pipeline (custom state caches, hand-rolled HTTP clients, codegen drift, etc.) comes back as a lossy number.Before this patch the constructors stored that
numberverbatim into a field typed asbigint. The failure then manifested several layers later in BSATN serialization, e.g. insideBinaryWriter.writeU256:with no indication of which call site was at fault.
Fix
Wrap the non-string branch of each constructor in
BigInt(...):numberinput — beforeIdentitylib/identity.tswriteU256ConnectionIdlib/connection_id.tswriteU128Timestamplib/timestamp.tstoDate()TimeDurationlib/time_duration.tsUuidlib/uuid.tsCannot mix BigInt and other typesfrom range checkbigintinputs are returned unchanged (BigInt(123n) === 123n).numberinputs are coerced cleanly. Precision was already lost upstream by whatever produced the number — this just stops the secondary crash.undefined/ objects throw a clearTypeErrorimmediately at the constructor, rather than several frames deep in serialization.No public type signatures change; strict-TS callers compile-check exactly as before.
Tests
Added
crates/bindings-typescript/tests/id_ctor_coercion.test.tspinning the new behavior across all five classes (9 cases): bigint pass-through, number coercion, hex-string parsing for Identity, range check preservation for Uuid, and clear errors onundefined.Full test suite (
pnpm run test) — 191 passed, 0 failed.pnpm run lint— clean.pnpm run build:types— clean.API and ABI breaking changes
None. Public type signatures unchanged. Behavior is strictly more permissive (previously-failing inputs now succeed or throw earlier with a clearer error).
Expected complexity level and risk
1 — narrow defensive change, six small constructors, isolated tests.