-
Notifications
You must be signed in to change notification settings - Fork 11
Support function overloads #58
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
@aryaemami59, looking forward to this. Just in case there's anything breaking added in this, I would like hold off on v1 until this goes in, if you think it's close to being ready for review. But let's try as hard as possible to make sure it's non-breaking. FYI I added you as a collaborator - next PR you open you should be able to just push a branch against this origin rather than your fork. |
@mmkal Thank you! I'm gonna make sure this is non-breaking, if it turns out it has to be a breaking change, then I might just make a new assertion instead. |
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
BTW I haven't forgotten about this, I'm just in the middle of moving. As soon as I'm done I will make sure to finish this. |
…CallableWith-overload
Not sure about this one so far, gonna need some feedback on this. |
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 so far. I can't remember if we've already discussed whether this should also take over helpers like .parameters
and .returns
e.g.
function f(n: number): 1
function f(n: number, s: string): 2
function f(...) {
...
}
expectTypeOf(f).parameters.toEqualTypeOf<[number] | [number, string]>()
expectTypeOf(f).returns.toEqualTypeOf<1 | 2>()
What do you think?
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.
Couple more comments
…CallableWith-overload
Co-authored-by: Misha Kaletsky <15040698+mmkal@users.noreply.github.com>
Honestly, not too sure yet, it's up to you :) |
@aryaemami59 is there a reason the 10-overloads option needs to be the first ternary check rather than the last? I don't think this behaviour is great, even for an internal type: (playground, copy-pasting the Makes it awkward to use for things like |
@mmkal I wish present me could remember why past me decided to go with that approach :) I'll look at it to see if I can change it. |
Oh right, of course, if you put the 1-overload variant TypeScript will just pick the last one semi-arbitrarily. But I now don't see what value we're getting from the intermediate ones. It seems we might be stuck with a very long tuple as the output, but all functions will get a match from the first ternary. Simplified demo |
@mmkal right, I was trying to see if there is a way to find out exactly how many overloads a function has so we don't run into that issue. |
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
…CallableWith-overload
Related to #58 Related to #30 Improve support for overloaded functions (up to 10 overloads). So for an example function type: ```ts type Factorize = { (input: number): number[] (input: bigint): bigint[] } ``` --- `.parameters` gives you an ExpectTypeOf instance with a union of the parameter-tuple types, so: ```ts expectTypeOf<Factorize>().parameters.toEqualTypeOf<[number] | [bigint]>() ``` --- `.returns` gives you an ExpectTypeOf instance with a union of the return types, so ```ts expectTypeOf<Factorize>().returns.toEqualTypeOf<number[] | bigint[]>() ``` --- `.toBeCallableWith(...)` now accepts any overload input, not just the "last" like before. And you can now chain it via `.returns` which gives you the matching return type: ```ts expectTypeOf<Factorize>().toBeCallableWith(5).returns.toEqualTypeOf<number[]>() ``` --- ## Implementation - overload utilities were added - initially, I thought all that was needed was a utility that matches `F` typeargs against a single 10-overload type - but the `test-types` job caught that this _doesn't_ work for typescript <5.3 - for lower typescript versions, it seems we need an approach more like the one in #58 - there are some edge cases the tests found for generic functions and parameterless functions, so there is _sometimes_ some "useless" overload info that has to be explicitly excluded - there are a couple of intermediate utilities to check if the 10-overload type can be used, and to exclude "useless" overloads - equivalents added for constructor parameters too - used the overloaded versions in `DeepBrand` too <details> <summary>update: moved these changes to #93 to reduce diff</summary> - I felt the megafile `index.ts` was finally getting too big with the added overload utilties: - so those were put in a new `overloads.ts` - since overload utils they rely on some other existing utils, I created `utils.ts` to avoid circular references - `index.ts` remains as the home for the main `ExpectTypeOf` exports - added `export * from './utils'` since we have been exporting all the internal utils and I don't want to break people - update: moved these changes to #93 to reduce diff </details> --------- Co-authored-by: Misha Kaletsky <mmkal@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Arya Emami <aryaemami59@yahoo.com>
This PR:
toBeCallableWith()
This is a rough draft, I will probably add support for overloads through
.parameter(number)
and.parameters
as well.