-
Notifications
You must be signed in to change notification settings - Fork 26
Update: propEq to allow wider-typing for value in comparison #74
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
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
propEqneeds 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
valwithout yet knowing the type ofobjtl;dr is that when we know the type of
valbut 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:stringbut the prob type isstring | number| null(eg nullable)The downside to these changes is that we lose errors when the types at
obj.propdon't at all overlap with the type ofval, but it will returnneverin those cases, which is better than nothingSince the we know the type of
valandobjat 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
allPassandanyPassso they don't rely onpropEq. I don't want have to continuously update those as we updatepropEq. Those tests should stand on their own