effect icon indicating copy to clipboard operation
effect copied to clipboard

[experiment] Improve Effects assignability after adding `Symbol.iterator` by replacing `void`s with `unknown`

Open Andarist opened this issue 1 year ago • 10 comments

This is not for merging, I just put it together since I was curious if I could make it work this way. This work was started based on the initial commit from https://github.com/Effect-TS/effect/pull/2602

It doesn't (yet?) allow you to remove the adapter thing from this branch but perhaps this is just missing some minor thing. I'm not sure if there were any extra changes done by @tim-smart to allow that. This PR focuses solely on making this to typecheck with the added Symbol.iterator method.

There are a few minor TODO comments here but all of them (but one!) are really about the same thing - adding Symbol.iterator properly to your error classes.

Andarist avatar Apr 27 '24 08:04 Andarist

⚠️ No Changeset found

Latest commit: 7da71f8ea12bbc31e3517837c6f3bd7c85f4f539

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

changeset-bot[bot] avatar Apr 27 '24 08:04 changeset-bot[bot]

Nice!

mikearnaldi avatar Apr 27 '24 09:04 mikearnaldi

This is part of a larger discussion about using "void" instead of "unknown" when we don't care about the value of an effect, in theory it would make sense

mikearnaldi avatar Apr 27 '24 09:04 mikearnaldi

Thanks for taking the time to look at this :)

Replacing void with unknown does feel weird, feels like we are losing some semantics by making the return types too generic.

tim-smart avatar Apr 28 '24 22:04 tim-smart

What kind of semantics you'd lose? I find void to be quite weird and problematic at times. Take a look at this:

declare const test: string | void | undefined;

if (test === undefined) {
  test; // void | undefined
} else {
  test; // string
}

Andarist avatar Apr 29 '24 07:04 Andarist

Mainly you lose some meaning / intent. This function returns nothing (void) vs this function could return anything but we don't care what (unknown).

tim-smart avatar Apr 29 '24 09:04 tim-smart

Mainly you lose some meaning / intent. This function returns nothing (void) vs this function could return anything but we don't care what (unknown).

Void doesn't mean that this function returns nothing, it means that we don't care about the return

mikearnaldi avatar Apr 29 '24 09:04 mikearnaldi

Yeah, so in that sense usually either unknown or undefined is (IMHO) a better more-predictable choice (depending on your needs).

This is perfectly valid:

const test: () => void = () => 42;

Andarist avatar Apr 29 '24 10:04 Andarist

But in terms of familiarity for the majority of developers, most of them know that

const myFn = () => {
  // No explicit return
}

Will infer a return type of void. I just think that familiarity is worth something.

tim-smart avatar Apr 29 '24 10:04 tim-smart

But in terms of familiarity for the majority of developers, most of them know that

const myFn = () => {
  // No explicit return
}

Will infer a return type of void. I just think that familiarity is worth something.

yes it will infer void, but you can also type void and return, the meaning really is "whatever" and given it is sometimes treated as undefined it creates potential issues

mikearnaldi avatar Apr 30 '24 21:04 mikearnaldi