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

feat: introduce UnwrapOpaque type #403

Merged
merged 5 commits into from Jun 24, 2022
Merged

Conversation

jmike
Copy link
Contributor

@jmike jmike commented Jun 2, 2022

This PR introduces a new UnwrapOpaque type to remove the [tag] from Opaque values.

Why is this necessary?

  1. Use an opaque type as object keys
  2. Prevent stupid TS4058 error, i.e. Return type of exported function has or is using name X from external module Y but cannot be named.

Fixes #165

@sindresorhus
Copy link
Owner

Any thoughts on the discussion in #165?

@jmike
Copy link
Contributor Author

jmike commented Jun 2, 2022

As far as naming goes, I do not have a preference. I can amend this PR to anything you decide.

Re: protected, I don't like classes but they seem to do the trick here. I cannot say - needs extensive testing.

I just thought this discussion is ongoing for quite some time, so why not do something about it since I am also affected by the same problems at work.

@jmike
Copy link
Contributor Author

jmike commented Jun 20, 2022

Hey, I don't want to push here or anything... if you don't want to merge this it's fine. Just kindly let me know because we need this functionality, so I might as well find/create another lib for branded types only.

source/opaque.d.ts Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

I'm fine with adding this, but it should be called UnwrapOpaque.

@jmike jmike changed the title feat: introduce DeOpaque type feat: introduce UnwrapOpaque type Jun 21, 2022
@jmike
Copy link
Contributor Author

jmike commented Jun 21, 2022

I have renamed the new type (as well as the PR title and description) to UnwrapOpaque per @sindresorhus suggestion.

@jmike jmike requested a review from sindresorhus June 21, 2022 18:52
source/opaque.d.ts Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

Looks good.

You need to add it to the readme.

@jmike
Copy link
Contributor Author

jmike commented Jun 23, 2022

Cool, the code has been updated accordingly.

readme.md Outdated Show resolved Hide resolved
@sindresorhus sindresorhus merged commit e904d8e into sindresorhus:main Jun 24, 2022
sindresorhus added a commit that referenced this pull request Jun 24, 2022
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
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 this pull request may close these issues.

Add Transparent to reverse Opaque
2 participants