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

Opaque types are interchangeable #70

Closed
natesilva opened this issue Jan 14, 2020 · 8 comments · Fixed by #71
Closed

Opaque types are interchangeable #70

natesilva opened this issue Jan 14, 2020 · 8 comments · Fixed by #71

Comments

@natesilva
Copy link

natesilva commented Jan 14, 2020

Do you have any examples for Opaque? I thought Opaque types weren’t supposed to be interchangeable.

import { Opaque } from 'type-fest';

type AccountNumber = Opaque<number>;
type AccountBalance = Opaque<number>;

function getBalance(accountNumber: AccountNumber): AccountBalance {
  return 101 as AccountBalance;
}

// pass an AccountBalance instead of an AccountNumber
const ab = 42 as AccountBalance;
console.log(getBalance(ab)); 
// expected: should not compile
// actual: gets balance for account 42

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • The funding will be given to active contributors.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@natesilva natesilva changed the title Better documentation for Opaque? Opaque types are interchangeable Jan 14, 2020
@natesilva
Copy link
Author

The example shown in https://codemix.com/opaque-types-in-javascript/ works as expected. Their version of Opaque is defined as:

type Opaque<K, T> = T & { __TYPE__: K };

With that definition they are not interchangeable. So I think this is a bug?

@sindresorhus
Copy link
Owner

// @resynth1943

@resynth1943
Copy link
Contributor

resynth1943 commented Jan 14, 2020

The example shown in https://codemix.com/opaque-types-in-javascript/ works as expected. Their version of Opaque is defined as:

type Opaque<K, T> = T & { __TYPE__: K };

With that definition they are not interchangeable. So I think this is a bug?

That's not the same as the definition of the Opaque type in this library. To be precise, it's

export type Opaque<Type> = Type & {readonly __opaque__: unique symbol};

I think I know why TypeScript is doing this...

import { Opaque } from 'type-fest';

type AccountNumber = Opaque<number>;
type AccountBalance = Opaque<number>;

function getBalance(accountNumber: AccountNumber): AccountBalance {
 return 101 as AccountBalance;
}

Now, as you can see, AccountNumber is basically equal to AccountBalance... from TypeScript's viewpoint. They both have the same properties and are basically interchangeable.

After some minutes of exploration, I think I've found a solution:

type Opaque<Type, Marker extends string> = Type & Record<Marker, 'opaque'>;
type A = Opaque<number, 'A'>;
type B = Opaque<number, 'B'>;

function useA(a: A) { }

function getA(): A {
    return 2 as A;
}

const a: A = getA();
const b: B = getA(); // Compilation error.
useA(a);

In short, providing unique names to the Opaque type seems to be the only way to make an opaque type truly unique from the rest, preventing typing errors.

@sindresorhus How do you feel about this?

EDIT: Oops. They're not the same.

@resynth1943
Copy link
Contributor

If everyone's happy with my proposal above, I'd be happy to make a Pull Request. Following this, perhaps making the second generic parameter default to unknown would allow backwards compatibility?

@natesilva
Copy link
Author

I’m happy with that proposal, including defaulting the second parameter to unknown.

@resynth1943
Copy link
Contributor

Well if everyone's happy, I'm happy to blow the cobwebs off VSCodium and PR this in. ❤️

@sindresorhus
Copy link
Owner

👍

@natesilva
Copy link
Author

Thank you!

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

Successfully merging a pull request may close this issue.

3 participants