-
Notifications
You must be signed in to change notification settings - Fork 46
Changed type definitions for functions with array parameters #69
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
Array parameters inside functions should now correctly return arrays in the type definition, just a small QoL change
Hi, I'm not that familiar with Types. @climaxmba could you review this PR? I see that basically you removed all the pipes |
Hi! Basically, these are not pipes, but rather discriminated unions or Union Types. The thing with these is that they help determine which types are supported as parameters by each function, but it's hard to specify which type the functions should return without rewriting the functions themselves, since typescript doesn't actually know which type should return what. For example:
In this example, TS knows all of the possible input types, but it will only know that it has to return a single Berry every time as it's currently defined. A possible solution to this is to write the following instead:
The problem with this is that now, TypeScript doesn't actually know when it will return a single berry, and when it will return multiple without rewriting the function itself in TS or making the consumer of the returned promise narrow the return type manually. To avoid these problems, we can instead duplicate the function signatures, basically stating that: in one case, the function will return only one element if only a string or number is given to it, and in another, if the function is given an array then TypeScript will know these functions will only return an array. This is also known as Function Overloading. Another plus with these changes is that now heterogeneous arrays are supported, which they weren't previously before (only arrays of numbers OR strings could be passed as parameters, not ones that contained both). |
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.
Sure @Naramsim, Eduardo has a point.
getBerryByName(name: string | number): Promise<Berry>; | ||
getBerryByName(names: Array<string | number>): Promise<Berry[]>; |
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.
For example, these lines tells TypeScript that a single object is returned in the Promise
, only if a string or number is given in getBerryByName
. And an array of objects, only if an array of strings or numbers is given.
Thanks so much for the explanation! Very informative! Now I see the benefits and we can merge! |
Functions with array parameters should now correctly return arrays in the type definition, before, all functions regardless of their parameters just used to return a single element, which doesn't reflect the intended behavior. Just a small QoL change to help work with TS.