Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Make
assert
,truthy
andfalsy
typeguards #3233Make
assert
,truthy
andfalsy
typeguards #3233Changes from 2 commits
f106e37
8c6696f
35083a9
04b0f49
58274c1
c9ec4b1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think
never
is correct here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can use anything besides
never
there and have it work correctly. The only things that typescript doesn't complain about using there arenever
andany
or a subset of theT
type. If you make itany
thenfalsy(true)
thinks it's falsy.This is also the exact opposite condition I setup in
truthy
andassert
:actual is Exclude<T,Falsy>
where Exclude isExclude<T,U> = T extends U ? never : T
becomesactual is T extends Falsy ? never : T
which is the opposite condition of thefalsy
assertion.I included the test-types with both the
if
case and theelse
case so you can see the types as they work. For further showcasing of the types, I setup a quick Playground example if you want to try changing what the assertions are.Playing around with it, the current conditions don't work well for number and string primitives. Typescript seems to not recognize that
0
is a subset of number unless you use anas const
expression with it. Similar results for an empty string (''
) but even more wacky as''
will make typescript think it istruthy
with a value of''
, but'' as const
makes typescript recognize it as falsy. Typescript also doesn't separate out0
or''
from their primitive values.So far, I think I'm having the most accurate approach using (playground link)
In which case the only remaining issue is that for a type of
number
orstring
, typescript doesn't think that they could ever be falsy.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I read the signature incorrectly.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With your suggestion of multiple overloads, that partially works, but runs into an issue where union types would fall back to the base behavior of the generic rather than the behavior of the overload.
I think I've cracked it for falsy, but I can't get the same to work for truthy as Typescript has no way to just invert a type. playground - As you can see from the screenshot, falsy works great, but truthy is missing
0
in theelse
clause, and I don't think there's really anything we can do about it...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would mention this in the doc comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work to have specific overloads?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.