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

fix: emit nominal tag in declaration #114

Merged
merged 2 commits into from
Jul 22, 2019
Merged

fix: emit nominal tag in declaration #114

merged 2 commits into from
Jul 22, 2019

Conversation

andnp
Copy link
Owner

@andnp andnp commented Jul 22, 2019

The Nominal type has not been working (possibly for a long time)
because the tag was not being emitted by the typescript compiler. This
didn't show up in testing because the tests are not run against the
emitted definitions.

Instead of emitting:

class Tagged<N extends string> { private __tagged__: N }

we were getting

class Tagged<N extends string> { __tagged__ }

thus nothing was tagged any longer.

Switching from private to protected fixes the emit issue while still
hiding the tag from the end user.

The `Nominal` type has not been working (possibly for a long time)
because the tag was not being emitted by the typescript compiler. This
didn't show up in testing because the tests are not run against the
emitted definitions.

Instead of emitting:
```typescript
class Tagged<N extends string> { private __tagged__: N }
```
we were getting
```typescript
class Tagged<N extends string> { __tagged__ }
```
thus nothing was tagged any longer.

Switching from `private` to `protected` fixes the emit issue while still
hiding the tag from the end user.
@andnp andnp requested a review from Retsam as a code owner July 22, 2019 02:16
@andnp andnp merged commit 7d922f5 into master Jul 22, 2019
@andnp andnp deleted the Fix-Nominal branch July 22, 2019 02:21
@andnp
Copy link
Owner Author

andnp commented Jul 22, 2019

🎉 This PR is included in version 3.2.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@andnp andnp added the released label Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant