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

Update: propEq to allow wider-typing for value in comparison #74

Merged
merged 9 commits into from
Feb 13, 2024

Conversation

Harris-Miller
Copy link
Collaborator

@Harris-Miller Harris-Miller commented Oct 15, 2023

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 of obj

tl;dr is that when we know the type of val but not the type of obj, we're allowing for a wider type range for better support. These additions fix the following issues that the current typings don't support:

  • val is string but the prob type is string | number
  • prop type is | null (eg nullable)
  • prop type is optional

The downside to these changes is that we lose errors when the types at obj.prop don't at all overlap with the type of val, but it will return never in those cases, which is better than nothing

Since the we know the type of val and obj at the same time with the full function signature propEq(val, name, obj), that signature can remain strict, since the above cases are not an issue with the full signature

TODO: update tests for allPass and anyPass so they don't rely on propEq. I don't want have to continuously update those as we update propEq. Those tests should stand on their own

@Harris-Miller Harris-Miller changed the title Draft: Update PropEq to allow wider-typing for value in comparison Draft: Update: propEq to allow wider-typing for value in comparison Oct 15, 2023
Copy link
Contributor

@Nemo108 Nemo108 left a 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.

types/propEq.d.ts Show resolved Hide resolved
@Harris-Miller Harris-Miller changed the title Draft: Update: propEq to allow wider-typing for value in comparison Update: propEq to allow wider-typing for value in comparison Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants