-
Notifications
You must be signed in to change notification settings - Fork 22
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
Update: propEq to allow wider-typing for value in comparison #74
Conversation
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 is a really great solution to the problem. It looks like it can fix almost all of the problems I face during development.
I agree about clearing up allPass
and anyPass
tests since they are unit tests and should not depend on any other function. However, it may be a better idea to make integrational tests somewhere, to ensure that functions can work okay together since sometimes typings updates can break the co-usage of the functions.
…args or return types
dcbe597
to
aab5114
Compare
See this playground for full tests: https://tsplay.dev/mMlMdN
propEq
needs a lot of updating. This MR is one of 3 towards that effort.This first effort is just to update the typings to allow for a more correct range of typing when passing a
val
without yet knowing the type ofobj
tl;dr is that when we know the type of
val
but not the type ofobj
, we're allowing for a wider type range for better support. These additions fix the following issues that the current typings don't support:string
but the prob type isstring | number
| null
(eg nullable)The downside to these changes is that we lose errors when the types at
obj.prop
don't at all overlap with the type ofval
, but it will returnnever
in those cases, which is better than nothingSince the we know the type of
val
andobj
at the same time with the full function signaturepropEq(val, name, obj)
, that signature can remain strict, since the above cases are not an issue with the full signatureTODO: update tests for
allPass
andanyPass
so they don't rely onpropEq
. I don't want have to continuously update those as we updatepropEq
. Those tests should stand on their own