Skip to content

Don't narrow generic on type specific assertions #168

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

Merged
merged 2 commits into from
Nov 20, 2021

Conversation

kanongil
Copy link
Contributor

This avoids resolving to never for ORed types like number | string. Closes #167.

This avoids narrowing the type that is forwarded to the type specific assertion by doing the conditional test on another TTest generic.

@kanongil kanongil added the types TypeScript type definitions label Nov 17, 2021
@kanongil
Copy link
Contributor Author

Anyone up for a review?

Copy link
Member

@Nargonath Nargonath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM but I don't understand how it works better than the previous typings. 😅 If you know some resources that I can refer to, that would be appreciated.

Thanks for the fix @kanongil.

@kanongil
Copy link
Contributor Author

kanongil commented Nov 19, 2021

Typescript will narrow types that are tested. We use conditional type checks to do our own narrowing. However, we passed the tested type T to the conditional type, which would then (incorrectly) be narrowed.

This means that number | undefined would be inferred to be NumberAssertion<number> | Assertion<undefined>. These contain incompatible declarations for the first arg to equal(), which then resolved to never.

With this fix, we pass an un-narrowed type in the condition check. This now resolves to NumberAssertion<number | undefined> | Assertion<number | undefined>, which means that T is now the same for both and the arg to equal() can be resolved.

@devinivy devinivy merged commit 4d61ad1 into hapijs:master Nov 20, 2021
@Nargonath
Copy link
Member

Thank you for your explanation @kanongil and for the linked resources. I got it now. 👍

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

Successfully merging this pull request may close these issues.

compilation errors for primitive type assertions
3 participants