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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Either altW 'left' is too wide #1488

Closed
wmaurer opened this issue Apr 20, 2021 · 2 comments 路 Fixed by #1489
Closed

Either altW 'left' is too wide #1488

wmaurer opened this issue Apr 20, 2021 · 2 comments 路 Fixed by #1489
Labels

Comments

@wmaurer
Copy link
Contributor

wmaurer commented Apr 20, 2021

馃悰 Bug report

The function signature for Either altW is as follows:

export declare const altW: <E2, B>(that: Lazy<Either<E2, B>>) => <E1, A>(fa: Either<E1, A>) => Either<E1 | E2, A | B>

From my understanding of alt, Left<E1> can never be produced, so I believe the signature should be as follows:

export declare const altW: <E2, B>(that: Lazy<Either<E2, B>>) => <E1, A>(fa: Either<E1, A>) => Either<E2, A | B>

Current Behavior

const foo = pipe(
    E.left<number, number>(1),
    E.altW(() => E.right<string, string>('a')),
);

In this case, foo's type is Either<string | number, string | number>.

Expected behavior

I believe foo's type should be Either<string, string | number>.

Reproducible example

See above.

Suggested solution(s)

Remove E1 from the return type of altW:

export const altW: <E2, B>(
    that: Lazy<Either<E2, B>>,
) => <E1, A>(fa: Either<E1, A>) => Either<E2, A | B> = that => fa => (isLeft(fa) ? that() : fa);

I could make a PR if you agree.

Your environment

Software Version(s)
fp-ts 2.10.2
TypeScript 4.2.4
@gcanti gcanti added the bug label Apr 20, 2021
@gcanti
Copy link
Owner

gcanti commented Apr 20, 2021

I could make a PR if you agree.

@wmaurer thanks, you are right the current error type is overspecified. Looks like the following data types suffer from the same issue:

  • Either
  • IOEither
  • ReaderEither
  • ReaderTaskEither
  • StateReaderTaskEither
  • TaskEither

@wmaurer
Copy link
Contributor Author

wmaurer commented Apr 20, 2021

Great, thanks for the confirmation. I'll put together a PR soon.

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

Successfully merging a pull request may close this issue.

2 participants