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

TypeScript doesn't pick the correct overloading when mapping a ConcurrentFutureInstance (v12.0.0) #400

Closed
gcanti opened this issue Nov 18, 2019 · 3 comments · Fixed by #401

Comments

@gcanti
Copy link
Contributor

gcanti commented Nov 18, 2019

Repro

import * as F from 'fluture'

declare const x: F.FutureInstance<Error, number>
declare const y: F.ConcurrentFutureInstance<Error, number>

declare function double(n: number): number

F.map(double)(x) // ok

// v-- here is still picking the `FutureInstance` overloading
F.map(double)(y) // Argument of type 'ConcurrentFutureInstance<Error, number>' is not assignable to parameter of type 'FutureInstance<unknown, number>'

Possible solution (index.d.ts)

/** Map over the resolution value of the given Future / ConcurrentFuture. See https://github.com/fluture-js/Fluture#map */
export function map<RA, RB>(mapper: (value: RA) => RB): {
  <L>(source: FutureInstance<L, RA>): FutureInstance<L, RB>
  <L>(source: ConcurrentFutureInstance<L, RA>): ConcurrentFutureInstance<L, RB>
}

instead of

/** Map over the resolution value of the given Future. See https://github.com/fluture-js/Fluture#map */
export function map<RA, RB>(mapper: (value: RA) => RB): <L>(source: FutureInstance<L, RA>) => FutureInstance<L, RB>

/** Map over the resolution value of the given ConcurrentFuture. See https://github.com/fluture-js/Fluture#map */
export function map<RA, RB>(mapper: (value: RA) => RB): <L>(source: ConcurrentFutureInstance<L, RA>) => ConcurrentFutureInstance<L, RB>
@gcanti gcanti changed the title TypeScript doesn't pick the correct overloading when mapping a ConcurrentFutureInstance TypeScript doesn't pick the correct overloading when mapping a ConcurrentFutureInstance (v12.0.0) Nov 18, 2019
@Avaq
Copy link
Member

Avaq commented Nov 18, 2019

Thank you for the report! I imagine the same issue also affects other algebra's that use overloading in TypeScript, such as chain, ap, etc.

@gcanti
Copy link
Contributor Author

gcanti commented Nov 18, 2019

I imagine the same issue also affects other algebra's that use overloading in TypeScript

Actually no, they are fine, that's because TypeScript can use the first parameter (of alt / ap) to disambiguate

/** Logical or for Futures. See https://github.com/fluture-js/Fluture#alt */
export function alt<L, R>(left: FutureInstance<L, R>): (right: FutureInstance<L, R>) => FutureInstance<L, R>

/** Race two ConcurrentFutures. See https://github.com/fluture-js/Fluture#alt */
export function alt<L, R>(left: ConcurrentFutureInstance<L, R>): (right: ConcurrentFutureInstance<L, R>) => ConcurrentFutureInstance<L, R>

/** Apply the function in the left Future to the value in the right Future. See https://github.com/fluture-js/Fluture#ap */
export function ap<L, RA>(value: FutureInstance<L, RA>): <RB>(apply: FutureInstance<L, (value: RA) => RB>) => FutureInstance<L, RB>

/** Apply the function in the left ConcurrentFuture to the value in the right ConcurrentFuture. See https://github.com/fluture-js/Fluture#ap */
export function ap<L, RA>(value: ConcurrentFutureInstance<L, RA>): <RB>(apply: ConcurrentFutureInstance<L, (value: RA) => RB>) => ConcurrentFutureInstance<L, RB>

while in map

/** Map over the resolution value of the given Future. See https://github.com/fluture-js/Fluture#map */
export function map<RA, RB>(mapper: (value: RA) => RB): <L>(source: FutureInstance<L, RA>) => FutureInstance<L, RB>

/** Map over the resolution value of the given ConcurrentFuture. See https://github.com/fluture-js/Fluture#map */
export function map<RA, RB>(mapper: (value: RA) => RB): <L>(source: ConcurrentFutureInstance<L, RA>) => ConcurrentFutureInstance<L, RB>

can't do the same.

@Avaq
Copy link
Member

Avaq commented Nov 19, 2019

Released as 12.0.1.

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

Successfully merging a pull request may close this issue.

2 participants