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

Swap Either type params #2102

Merged
merged 2 commits into from
Feb 14, 2024
Merged

Swap Either type params #2102

merged 2 commits into from
Feb 14, 2024

Conversation

mikearnaldi
Copy link
Member

@mikearnaldi mikearnaldi commented Feb 11, 2024

Swap type params of Either from Either<E, A> to Either<R, L = never>.

Along the same line of the other changes this allows to shorten the most common types such as:

import { Either } from "effect";

const right: Either.Either<string> = Either.right("ok");

While it was confusing to use Either<A, E> I don't find it particularly confusing to use Either<R, L>, the type reads like "it's either a right or a left"

Copy link

changeset-bot bot commented Feb 11, 2024

🦋 Changeset detected

Latest commit: e54a577

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages
Name Type
@effect/platform-node-shared Major
@effect/typeclass Major
effect Minor
@effect/schema Major
@effect/cli Major
@effect/platform-bun Major
@effect/platform-node Major
@effect/printer-ansi Major
@effect/printer Major
@effect/experimental Major
@effect/opentelemetry Major
@effect/platform-browser Major
@effect/platform Major
@effect/rpc-http Major
@effect/rpc Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot changed the base branch from main to next-minor February 11, 2024 17:30
@mikearnaldi mikearnaldi force-pushed the chore/swap-either branch 2 times, most recently from fdb21cb to fde0f40 Compare February 11, 2024 17:38
@ethanniser
Copy link
Contributor

ethanniser commented Feb 11, 2024

I think this a good change to keep it in line with all of the other changed types

But I am curious why the decision to only modify the type parameter order, and not the runtime semantics?

Surely the function named Either.right would result in a Either with the type of the right?

const right: Either.Either<string, never> = Either.right("ok"); // right = left
const left: Either.Either<never, number> = Either.left(5); // left = right

because Either is "just a tagged union" there shouldn't be any reason that 'right' belongs to the first type parameter any more than left

making them match just feels nicer?

of course it makes it a bit of a bigger migration, but its already a breaking change

@mikearnaldi
Copy link
Member Author

mikearnaldi commented Feb 11, 2024

I think this a good change to keep it in line with all of the other changed types

But I am curious why the decision to only modify the type parameter order, and not the runtime semantics?

Surely the function named Either.right would result in a Either with the type of the right?

const right: Either.Either<string, never> = Either.right("ok"); // right = left
const left: Either.Either<never, number> = Either.left(5); // left = right

because Either is "just a tagged union" there shouldn't be any reason that 'right' belongs to the first type parameter any more than left

making them match just feels nicer?

of course it makes it a bit of a bigger migration, but its already a breaking change

Users complained that "right" feels like "success" and "left" doesn't , also we couldn't write a codemod for that as it touches too many things that are contextual so the pain would probably be too big to migrate manually

@patroza
Copy link
Member

patroza commented Feb 11, 2024

gj for swapping to more semantic L and R. Use Effect/Exit when you need to model Error and Success.
there's still the obvious swapping in the generics: Right is on the Left and Left is on the Right.
but that's alright

@github-actions github-actions bot force-pushed the next-minor branch 4 times, most recently from 2cadab4 to 2255d9b Compare February 11, 2024 21:32
@github-actions github-actions bot force-pushed the next-minor branch 13 times, most recently from 6fb86a0 to ab10640 Compare February 13, 2024 06:53
@github-actions github-actions bot force-pushed the next-minor branch 6 times, most recently from fbdc28e to 5e84f40 Compare February 14, 2024 12:36
@datner
Copy link
Contributor

datner commented Feb 14, 2024

I think it should be noted in the docs, otherwise we will have fp devotees knocking at the door

@mikearnaldi
Copy link
Member Author

I think it should be noted in the docs, otherwise we will have fp devotees knocking at the door

they can keep believing their cult and use fp-ts v2 :) jokes aside, yes everything needs to be documented

@mikearnaldi mikearnaldi merged commit 5cb5d48 into next-minor Feb 14, 2024
12 checks passed
@mikearnaldi mikearnaldi deleted the chore/swap-either branch February 14, 2024 16:15
@github-actions github-actions bot mentioned this pull request Feb 14, 2024
github-actions bot pushed a commit that referenced this pull request Feb 14, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
github-actions bot pushed a commit that referenced this pull request Feb 14, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
github-actions bot pushed a commit that referenced this pull request Feb 14, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
tim-smart pushed a commit that referenced this pull request Feb 15, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
github-actions bot pushed a commit that referenced this pull request Feb 15, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
github-actions bot pushed a commit that referenced this pull request Feb 15, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
github-actions bot pushed a commit that referenced this pull request Feb 16, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
github-actions bot pushed a commit that referenced this pull request Feb 16, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
github-actions bot pushed a commit that referenced this pull request Feb 16, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
@samhh
Copy link

samhh commented Feb 16, 2024

I'm late to the party on this, but does this not make any prospective migration from fp-ts more difficult? If so, is it really worth it?

@mikearnaldi
Copy link
Member Author

I'm late to the party on this, but does this not make any prospective migration from fp-ts more difficult? If so, is it really worth it?

We are not optimizing for fp-ts migration, we are optimizing for newcomers and effect users.o

The migration from fp-ts is a much larger topic, in fp-ts Either was used as a Result, in Effect results are expressed as Exit and also 90% of the direct Either usages should probably instead be straight Effect usages.

mikearnaldi added a commit that referenced this pull request Feb 21, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
mikearnaldi added a commit that referenced this pull request Feb 21, 2024
Co-authored-by: Maxwell Brown <maxwellbrown1990@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

8 participants