Skip to content

Commit 599defa

Browse files
authored
fix(xdr): widen equals() param so union-typed values compile (#1637)
1 parent 1a6b117 commit 599defa

7 files changed

Lines changed: 169 additions & 2 deletions

File tree

.github/workflows/tests.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@ jobs:
5656
if: matrix.node-version == 22
5757
run: pnpm run docs:snippets:check
5858

59+
# Compile-time regression tests (test/types/): code that must typecheck
60+
# against src/, e.g. equals() on union-typed XDR values (issue #1630).
61+
# tsc output does not vary by Node version, so run once.
62+
- name: Type Tests
63+
if: matrix.node-version == 22
64+
run: pnpm run test:types
65+
5966
# Browser tests get their own job per transport rather than two steps in
6067
# `build_and_test`. Each pass loads the SDK source graph — ~470 unbundled
6168
# `src/xdr` modules — once per test file, across 220 files and two browsers.

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
A breaking change will get clearly marked in this log.
44

5+
## Unreleased
6+
7+
### Fixed
8+
* `equals()` on XDR values is now callable from TypeScript on union types like `xdr.ScVal`, `xdr.TransactionEnvelope`, and `xdr.Memo` — which is what the SDK's accessors return ([#1630](https://github.com/stellar/js-stellar-sdk/issues/1630)). The parameter was typed as polymorphic `this`, which reduces to `never` on a union, so every call failed with TS2345 even though the runtime worked. The parameter is now `XdrValue`, so comparing two different XDR types compiles and returns `false`.
9+
510
## [v17.0.0-rc.1](https://github.com/stellar/js-stellar-sdk/compare/v16.2.0...v17.0.0-rc.1)
611

712
### Breaking Changes

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,11 @@
112112
"docs:site": "astro build && tsx scripts/build-md-siblings.ts",
113113
"docs:dev": "astro dev",
114114
"docs:preview": "astro preview",
115-
"test": "pnpm run docs:snippets:check && pnpm run test:node && pnpm run test:node:axios && pnpm run test:integration && pnpm run test:browser",
115+
"test": "pnpm run docs:snippets:check && pnpm run test:types && pnpm run test:node && pnpm run test:node:axios && pnpm run test:integration && pnpm run test:browser",
116116
"test:all": "pnpm run test:node && pnpm run test:node:axios && pnpm run test:integration && pnpm run test:guides && pnpm run test:browser && pnpm run test:e2e",
117117
"test:e2e": "./test/e2e/initialize.sh && vitest run --config config/vitest.config.e2e.ts --coverage",
118118
"test:node": "vitest run test/unit --config config/vitest.config.ts --coverage",
119+
"test:types": "tsc -p test/types/tsconfig.json",
119120
"test:node:axios": "cross-env TRANSPORT=axios vitest run test/unit --config config/vitest.config.ts",
120121
"test:e2e:noeval": "NODE_OPTIONS=--disallow-code-generation-from-strings pnpm run test:e2e",
121122
"test:integration": "vitest run test/integration --config config/vitest.config.ts --coverage",

src/xdr/values/xdr-value.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,10 @@ export abstract class XdrValue {
8383
return this.toJson();
8484
}
8585

86-
equals(other: this): boolean {
86+
// Widened to `XdrValue` (not `this`): polymorphic `this` in parameter
87+
// position intersects a union's arms to `never`, making equals uncallable
88+
// on union-typed values (ScVal, Memo, ...). Cross-type calls return false.
89+
equals(other: XdrValue): boolean {
8790
if (this === other) return true;
8891
if (other == null || !(other instanceof XdrValue)) return false;
8992
if (this.constructor !== other.constructor) return false;

test/types/tsconfig.json

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
{
2+
// Typechecks the compile-time regression tests in this directory against
3+
// the SDK source (`pnpm test:types`). These files contain code that must
4+
// compile — e.g. `equals()` on union-typed XDR values (issue #1630), which
5+
// the emitted declarations once rejected while runtime behavior was fine.
6+
// Extends the root build config so src typechecks under the same
7+
// strictness it builds with.
8+
"extends": "../../tsconfig.json",
9+
"compilerOptions": {
10+
"declaration": false,
11+
"emitDeclarationOnly": false,
12+
"declarationDir": null,
13+
"outDir": null,
14+
"noEmit": true,
15+
"allowImportingTsExtensions": true,
16+
"rootDir": "../../",
17+
// Mirrors the root config's `types`: the class-XDR layer resolves
18+
// `@stellar/js-xdr` types from the package itself, so listing the local
19+
// `stellar__js-xdr` stub here would shadow them.
20+
"types": ["node", "js-xdr-v4"],
21+
"typeRoots": [
22+
"../../types",
23+
"../../node_modules/@types",
24+
"../../node_modules"
25+
]
26+
},
27+
"include": ["**/*.ts"]
28+
}

test/types/xdr-equals.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Compile-time regression test for issue #1630: `XdrValue.equals` must be
2+
// callable on union-typed values. The signature was once `equals(other: this)`,
3+
// and polymorphic `this` in parameter position intersects a union's arms —
4+
// discriminated arms with conflicting `type` properties reduce to `never`, so
5+
// no argument compiled (TS2345) on any of the SDK's union types even though
6+
// the runtime was correct. This file only has to typecheck; it never runs.
7+
8+
import type {
9+
ScVal,
10+
ScValU32,
11+
TransactionEnvelope,
12+
LedgerEntry,
13+
LedgerEntryData,
14+
Memo,
15+
HostFunction,
16+
SorobanCredentials,
17+
InvokeContractArgs,
18+
} from "../../src/xdr/index.js";
19+
20+
declare const scValA: ScVal;
21+
declare const scValB: ScVal;
22+
declare const envelopeA: TransactionEnvelope;
23+
declare const envelopeB: TransactionEnvelope;
24+
declare const ledgerEntryDataA: LedgerEntryData;
25+
declare const ledgerEntryDataB: LedgerEntryData;
26+
declare const memoA: Memo;
27+
declare const memoB: Memo;
28+
declare const hostFunctionA: HostFunction;
29+
declare const hostFunctionB: HostFunction;
30+
declare const credentialsA: SorobanCredentials;
31+
declare const credentialsB: SorobanCredentials;
32+
declare const structA: InvokeContractArgs;
33+
declare const structB: InvokeContractArgs;
34+
declare const singleArm: ScValU32;
35+
declare const ledgerEntry: LedgerEntry;
36+
37+
export const results: boolean[] = [
38+
// Union receiver and union argument, for every union named in the issue.
39+
scValA.equals(scValB),
40+
envelopeA.equals(envelopeB),
41+
ledgerEntryDataA.equals(ledgerEntryDataB),
42+
memoA.equals(memoB),
43+
hostFunctionA.equals(hostFunctionB),
44+
credentialsA.equals(credentialsB),
45+
46+
// Structs and single union arms keep working.
47+
structA.equals(structB),
48+
singleArm.equals(singleArm),
49+
50+
// Single arm against the full union, both directions.
51+
singleArm.equals(scValA),
52+
scValA.equals(singleArm),
53+
54+
// Cross-type comparison compiles and returns false at runtime.
55+
scValA.equals(memoA),
56+
57+
// Union-typed property read — the real-world shape that surfaced the bug
58+
// (no `const` annotation to narrow the union back to one arm).
59+
ledgerEntry.data.equals(ledgerEntryDataA),
60+
];

test/unit/xdr/equals.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Runtime behavior of `XdrValue.equals`. The signature takes `XdrValue`
2+
// (not `this`) so it stays callable on union-typed values — see issue #1630
3+
// and test/types/xdr-equals.ts for the compile-time side. These tests pin the
4+
// runtime contract: structural comparison by encoded bytes, `false` for
5+
// mismatched types, and `false` (never a throw) for junk arguments.
6+
7+
import { describe, it, expect } from "vitest";
8+
9+
import {
10+
ScVal,
11+
Memo,
12+
InvokeContractArgs,
13+
ScAddress,
14+
ContractId,
15+
} from "../../../src/xdr/index.js";
16+
17+
describe("XdrValue.equals", () => {
18+
it("returns true for structurally equal values", () => {
19+
expect(ScVal.scvU32(7).equals(ScVal.scvU32(7))).toBe(true);
20+
});
21+
22+
it("returns true for self-comparison", () => {
23+
const value = ScVal.scvU32(7);
24+
expect(value.equals(value)).toBe(true);
25+
});
26+
27+
it("returns false for unequal values of the same type", () => {
28+
expect(ScVal.scvU32(7).equals(ScVal.scvU32(8))).toBe(false);
29+
});
30+
31+
it("returns false across arms of the same union", () => {
32+
expect(ScVal.scvU32(7).equals(ScVal.scvBool(true))).toBe(false);
33+
});
34+
35+
it("returns false across different XDR types", () => {
36+
expect(ScVal.scvVoid().equals(Memo.memoNone())).toBe(false);
37+
});
38+
39+
it("compares union-typed values (the issue #1630 shape)", () => {
40+
const a: ScVal = ScVal.scvU32(7);
41+
const b: ScVal = ScVal.scvU32(7);
42+
expect(a.equals(b)).toBe(true);
43+
});
44+
45+
it("works on structs, not just unions", () => {
46+
const make = () =>
47+
new InvokeContractArgs({
48+
contractAddress: ScAddress.scAddressTypeContract(
49+
new ContractId(new Uint8Array(32)),
50+
),
51+
functionName: "hello",
52+
args: [ScVal.scvU32(1)],
53+
});
54+
expect(make().equals(make())).toBe(true);
55+
});
56+
57+
it("returns false for null and non-XdrValue arguments", () => {
58+
const value = ScVal.scvU32(7);
59+
expect(value.equals(null as unknown as ScVal)).toBe(false);
60+
expect(value.equals(undefined as unknown as ScVal)).toBe(false);
61+
expect(value.equals({} as ScVal)).toBe(false);
62+
});
63+
});

0 commit comments

Comments
 (0)