fix(xdr): widen equals() param so union-typed values compile - #1637
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Widens XDR equality typing so union-typed values compile while preserving runtime behavior.
Changes:
- Changes
equalsto accept anyXdrValue. - Adds runtime and compile-time regression coverage.
- Integrates type tests into CI and documents the fix.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/xdr/values/xdr-value.ts |
Widens the equality parameter type. |
test/unit/xdr/equals.test.ts |
Tests runtime equality behavior. |
test/types/xdr-equals.ts |
Covers union-type compilation. |
test/types/tsconfig.json |
Configures compile-time tests. |
package.json |
Adds the type-test command. |
.github/workflows/tests.yml |
Runs type tests in CI. |
CHANGELOG.md |
Records the fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
quietbits
approved these changes
Aug 12, 2026
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.
Fixes #1630.
What
Widens the parameter of
XdrValue.equalsfrom polymorphicthistoXdrValue, so the method is callable from TypeScript on union-typed values likexdr.ScVal,xdr.TransactionEnvelope, andxdr.Memo. The runtime body is unchanged: it already checksinstanceofand constructor identity, so comparing two different XDR types compiles and returnsfalse. Two kinds of tests guard the fix: a runtime suite (test/unit/xdr/equals.test.ts) pinning the structural-comparison contract, and a compile-time regression test (test/types/xdr-equals.ts) that exercisesequalson every union named in the issue, on a struct, on a single arm against the union in both directions, and via a union-typed property read. A newtest:typesscript (tsc -p test/types/tsconfig.json) runs it in CI next to the existing guide-snippet tsc gate, and the changelog gains a Fixed entry under Unreleased.Why
With
equals(other: this), polymorphicthisin parameter position distributes over a union's arms and intersects them; discriminated classes with conflictingtypeliterals reduce that intersection tonever, so no argument can satisfy the call (TS2345) on any of the 88 union types the SDK returns, even though the runtime was correct. This is worse than a normal type wart becausedocs/XDR_MIGRATION.mdprescribesequals()as the remedy for the v17 Buffer-to-Uint8Array assertion break, so the documented fix didn't compile on the types users actually hold. It shipped because nothing type-checked a call toequals: no test or internal caller touches it, and vitest strips types. The type test was verified to catch a regression by temporarily restoring the old signature, which reproduces the exact TS2345 from the issue.