Make AccountResponse conform to the StellarBase.Account interface. #655
Conversation
This just changes Account -> AccountResponse, the rest is just wrapping the docstrings @ 80 chars for aesthetics.
Shaptic
force-pushed
the
conform-account-response
branch
from
ab2b451 to
f059a03
Compare
| public createSubaccount(id: string): MuxedAccount { | ||
| return this._baseAccount.createSubaccount(id); | ||
| } |
There was a problem hiding this comment.
The Account.createSubaccount() method doesn't have a type annotation:
https://stellar.github.io/js-stellar-sdk/js-stellar-base_src_account.js.html
My guess is that TypeScript will assume id is of type Any, in which case Account would still be incompatible with AccountResponse because createSubaccount()'s function signature doesn't match.
There was a problem hiding this comment.
stellar-base is written in pure JavaScript, so its source will never have true type annotations (aside from in the jsdoc). They're provided after-the-fact in index.d.ts, where id is in fact specified to be a string. I've also made sure of this in a test script (based on #653).
Aside: The true long-term fix here is to make Account an interface just like in the Go SDK, with AccountResponse (at the SDK level) and another Account plus MuxedAccount (at the base level) being three implementations of it, but that's a bigger investment (and a major version bump) for Later:tm:.
Shaptic
force-pushed
the
conform-account-response
branch
from
f059a03 to
7928a44
Compare
This resolves an unintentional breaking change introduced in 8.2.0 in which
StellarSdk.AccountResponsewas no longer interchangeable withStellarBase.Accountin things likeTransactionBuilder. This also closes #653.Limitations: Since the test suite is written in JavaScript rather than TypeScript, this is impossible to catch there. More work needs to be done to avoid this happening in the future.