WIP: feat(typings): brand Secret, PublicKey, AccounId #214
Conversation
There was a problem hiding this comment.
Ok, did steps 1 & 2 here #214 and stellar/js-stellar-sdk#384 for Secret, AccountId and PublicKey.
For step 3 please see what makes common sense for you and what's not, so I can have a checklist to describe.
Looking good! For this stage let's move ahead as you suggest with making this an alias only. What I've been thinking is that we can introduce the breaking change with the move to TS for this library too.
What are you thought if we proceed as follows:
- Add typings using aliases
- Start TS migration (which would require a major bump)
- Include brands as part of the TS migration
| * | ||
| * See also `AccountId`, `PublicKey`. | ||
| */ | ||
| export type NonExistingAccountId = Brand<string, 'NonExistingAccountId'>; |
There was a problem hiding this comment.
@Akuukis do we need to make this assertion here? I think we can only AccountId regardless if they exist in the network or not.
| } | ||
|
|
||
| interface AccountMerge extends BaseOperation<OperationType.AccountMerge> { | ||
| destination: string; | ||
| destination: NonExistingAccountId; |
There was a problem hiding this comment.
this should probably be AccountId - when you merge the other account exists.
|
|
||
| /** | ||
| * 56-character long string, starts with G, passes `StrKey.isValidEd25519PublicKey()` | ||
| * and account does not exist on the given Network.. |
There was a problem hiding this comment.
Typo: Extra . --- Network..
|
Closing this for now since we should not mess with types which are going to be auto-generated. If we want to introduce this, we should do it on top of the work done in #342 |
See also PR at
stellar-sdk: stellar/js-stellar-sdk#384Closes #185