-
Notifications
You must be signed in to change notification settings - Fork 36
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
Apply new technique for strong heterogenous array types #281
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.
Consider using the newer readonly
syntax for read-only arrays.
I’m going to try to find some time to investigate this technique in Flow. I’ve been looking for a generic solution for this pattern for a while. |
I'd love to find at least an approximation in Flow. I tried (very) briefly to get something working with type-level functions (via $Call), but never managed to find something I was really happy with. It'll be great to have someone else give it a go! |
@unscriptable Here's the gist of what I was fiddling with. Maybe it'll be helpful. |
Update mergeArray docs to show variadic union. Improve mergeArray type for flow to allow variadic union, and add a type test to verify.
a4128cd
to
c6220d5
Compare
declare export function mergeArray <A> (ss: Array<Stream<A>>): Stream<A> | ||
|
||
type MergeArray = <A>(Array<Stream<A>>) => Stream<A> | ||
declare export function mergeArray <S> (ss: S): $Call<MergeArray, S> |
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.
@unscriptable This isn't quite the same as the ZipArray case (which I still don't know how to do in Flow), but it's a useful technique nonetheless. It basically turns an array type into a union of the array's value types (removing the extra Stream nesting in this case).
After looking at the docs, I'm inclined just to go with them as is, and let the Flow limitations manifest themselves as type errors for now. IOW, let's see if the current Flow arity limitations are problem in reality before we try to figure out what to do about it. |
This uses a variation of the technique from Simpler Variadic Function Types in Typescript to improve the types of zipArray and combineArray for TS, allowing them to scale to any arity.
It also improves the type of mergeArray, allowing it to be heterogeneous while remaining strongly typed. Essentially, this extends the union behavior of
merge
to arbitrary arrays of streams. This works by leveraging the fact that TS union types obey a distributive law.Potential Gotchas
In general, I see strong heterogeneous types as a plus. However, there could be a couple downsides:
Todos
ToStreamsArray
is duplicated, consider extracting it somewhere