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

Bug: isValid wrong typing #222

Closed
cpiber opened this issue Sep 21, 2021 · 3 comments · Fixed by #224
Closed

Bug: isValid wrong typing #222

cpiber opened this issue Sep 21, 2021 · 3 comments · Fixed by #224

Comments

@cpiber
Copy link
Contributor

cpiber commented Sep 21, 2021

isValid always assumes the type of the value, because the first argument should be unknown (like it is for ow itself):

isValid: <T>(value: T, predicate: BasePredicate<T>) => value is T;

Looking over the rest of the file, it seems this mistake was made in several places; ReusableValidator should probably also not use T for the value (and assert?)

@sindresorhus
Copy link
Owner

Good catch. Would you be interested in doing a pull request to fix it?

@cpiber
Copy link
Contributor Author

cpiber commented Sep 22, 2021

Sure, give me a bit of time though please

@cpiber
Copy link
Contributor Author

cpiber commented Sep 22, 2021

So, I just got to it and I have a problem: Ideally ReusableValidator<T> would also assert value is T, however typescript complains Assertions require every name in the call target to be declared with an explicit type annotation.. Do you happen to have a solution for that? I don't think it's elegant to have to explicitly them, and it's a breaking change...

Maybe one option would be not to include assert, but introduce a convenience type that extracts the type of ReusableValidator and provides another type that actually asserts. That way we don't have a breaking change but also a way for the user to easily get the assertion.

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

Successfully merging a pull request may close this issue.

2 participants