fix(types): propagateTask; decouple value type from sink type #123
Conversation
|
This looks good to me 👍 |
| export function propagateTask<T>(run: PropagateTaskRun<T>, value: T): (sink: Sink<T>) => PropagateTask<T>; | ||
| export function propagateTask<T>(run: PropagateTaskRun<T>): (value: T, sink: Sink<T>) => PropagateTask<T>; | ||
| export function propagateTask<T>(run: PropagateTaskRun<T>): (value: T) => (sink: Sink<T>) => PropagateTask<T>; | ||
| export function propagateTask<A, B = A>(run: PropagateTaskRun<A, B>, value: A, sink: Sink<B>): PropagateTask<A, B>; |
There was a problem hiding this comment.
Can you help me understand this B = A constraint? If A and B are always the same type, then I'm not seeing how this is different from a single type parameter
There was a problem hiding this comment.
It's a type default. So if you only supply one type propagateTask<number>(...) it will assume B is of type number. Otherwise propagateTask<number, string>(..) will set A to number and B to string. It actually makes this a non-breaking change afaik
There was a problem hiding this comment.
@TylorS is correct; it's not a constraint, but rather allows omission of the second type in favour of the specified default. In this case, it preserves the default expectation while allowing for variance when needed.
There was a problem hiding this comment.
Ohhhhh, I thought it was a constraint! Thanks! I don't mind a breaking change, but it's pretty cool that this makes it backward compatible.
I don't think flow has a notion of a type default, but we should update the flow refs to match without a default. Could you include that in this PR as well, @axefrog?
There was a problem hiding this comment.
I could... I'd need to disclaim that I have no familiarity with flow, so you'd need to double check that I've done it correctly.
There was a problem hiding this comment.
No worries. We're happy to help/review/etc. The change should be basically equivalent here except that flow doesn't support default types (unless I've missed it, which is also possible!).
There was a problem hiding this comment.
Sorry for the delay. PR updated.
Without this change, propagateTask can't be used to pass a wrapper object as the value, with the internal values then unwrapped by the task runner for delivery to the sink.