Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Brand string types #185

Open
Akuukis opened this issue May 1, 2019 · 4 comments
Open

Brand string types #185

Akuukis opened this issue May 1, 2019 · 4 comments

Comments

@Akuukis
Copy link
Contributor

Akuukis commented May 1, 2019

Problem

Many data structures are represented as strings, and types don't help much because string matches string. E.g.

  • at some places it is confusing whenever you need an address of existing account, random publicKey or just a string.
  • when any typeguarded XDR is serialized and then parsed again, it again loses it's narrowed type
  • Typescript wont catch if I'm messing up any of the following: PublicKey, Address, AccountId, Secret, Transaction hash, serialized transaction, signature, and perhaps more.

Describe the solution you'd like

I propose to brand all those strings. That way TypeScript compiler can pick up all the problems. Something like this:

import { Brand } from 'utility-types';

type PublicKey = Brand<string, "PublicKey">
type Secret = Brand<string, "Secret">
// ...

class Keypair {
    static fromSecret(secret: Secret): PublicKey
    //...
}

const publicKey = Keypair.fromSecret(Secret)
Keypair.fromSecret(publicKey)  // <-- TypeScript will complain here.

This even has potential to track the narrowed types through serialization if needed.

Describe alternatives you've considered

Less strict alternative is to use simple aliases and excessive comments.

/**
 * Do not pass where Secret is expected, please!!!
 */
type PublicKey: string

Related: #176, DefinitelyTyped/DefinitelyTyped#32025

@morleyzhi
Copy link
Contributor

@abuiles I think this is a great task to consider, since the sooner we implement and commit to the branded types, the sooner devs can use them in their apps.

@Akuukis
Copy link
Contributor Author

Akuukis commented Jul 24, 2019

I gave a though how to proceed with this. IMO this is the best, least-breaking way:

  1. Implement
  2. compile & test
  3. write docs and comments
  4. replace all type Stuff = Brand<string, "Stuff"> with type Stuff = string, so effectively nothing changes.
  5. Release minor version
  6. (later) Put back all type Stuff = Brand<string, "Stuff">
  7. Release with next major version

Some cons:

  • I think the most breaking change, and thus the hardest part, will be to change the minds to start to bother about difference between PublicKey, Address and AccountId.

@abuiles
Copy link
Contributor

abuiles commented Jul 24, 2019

@Akuukis sounds like a good plan! Do you want to have a stab at it? Thanks!

@Akuukis
Copy link
Contributor Author

Akuukis commented Jul 25, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants