effect
effect copied to clipboard
proposal for Effect.Tag conundrum
(I left out the changeset on purpose; what is the policy for changesets for fixes for unreleased changes)
🦋 Changeset detected
Latest commit: 9a00d023a01ba61da4cdf8f226e9f9def986d443
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 15 packages
| Name | Type |
|---|---|
| effect | Patch |
| @effect/cli | Patch |
| @effect/experimental | Patch |
| @effect/opentelemetry | Patch |
| @effect/platform-browser | Patch |
| @effect/platform-bun | Patch |
| @effect/platform-node-shared | Patch |
| @effect/platform-node | Patch |
| @effect/platform | Patch |
| @effect/printer-ansi | Patch |
| @effect/printer | Patch |
| @effect/rpc-http | Patch |
| @effect/rpc | Patch |
| @effect/schema | Patch |
| @effect/typeclass | Patch |
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
would be a patch, not sure I understand the fix though
not sure I understand the fix though
andThen & friends detect a function, while underlying it may be an Effect (aka the conundrum). it's going to call that function, which in turn can really check if the underlying service member is a function to be called, or an Effect to return. solving the disambiguity. once it has been able to check if its a function to call, or an Effect to return, it caches that knowledge for the next calls so that you only pay the price once.
the requirement for this to work consistently, is that at every place we want to deal with both functions and effects, we must first check if it's a function, only then if it's an Effect. as luck so has it (or well, it must be done that way anyway), that appears to be the case already.
would be a patch
isn't that weird, to have a changelog entry for an unreleased feature?
not sure I understand the fix though
andThen & friends detect a function, while underlying it may be an Effect (aka the conundrum). it's going to call that function, which in turn can really check if the underlying service member is a function to be called, or an Effect to return. solving the disambiguity. once it has been able to check if its a function to call, or an Effect to return, it caches that knowledge for the next calls so that you only pay the price once.
the requirement for this to work consistently, is that at every place we want to deal with both functions and effects, we must first check if it's a function, only then if it's an Effect. as luck so has it (or well, it must be done that way anyway), that appears to be the case already.
would be a patch
isn't that weird, to have a changelog entry for an unreleased feature?
Not really weird if we link changelogs to PRs. RE the fix, I am kind of unsure, there would be a mismatch between type and what happens behind the scenes, this would work with andThen but it feels like it's made as a special case for that.
cc @tim-smart
There is one other case in cli that needs addressing:
Also, instead of using a map you could put the cached props back on the class itself:
Hmm I think the only way to ensure functions are functions, and effects are effects is to separate the access like the previous revision.
Hmm I think the only way to ensure functions are functions, and effects are effects is to separate the access like the previous revision.
What’s wrong with my solution? esp. if we apply your suggestion to put it back on the class.
Ah yeah thinking about it some more your solution will work :)
Could also re-use the cn variable for the constant case.
Ah yeah thinking about it some more your solution will work :)
Could also re-use the
cnvariable for the constant case.
Awesome. Yea sure, who doesn’t like free stuff :)
The one case where it will still fail is if something expects a union of a function that doesn't return an Effect, or a plain Effect. I.e LazyArg<A> | Effect<A>
The one case where it will still fail is if something expects a union of a function that doesn't return an Effect, or a plain Effect. I.e
LazyArg<A> | Effect<A>
Got a failing test?
Hmm I think the only way to ensure functions are functions, and effects are effects is to separate the access like the previous revision.
The issue is that it's valid to have a function be an effect, it's a dsl that we used in the past too if I recall in platform, so this issue is kind of orthogonal to tag
Hmm I think the only way to ensure functions are functions, and effects are effects is to separate the access like the previous revision.
The issue is that it's valid to have a function be an effect, it's a dsl that we used in the past too if I recall in platform, so this issue is kind of orthogonal to tag
Only in implementation or also on the type level? If also on type level it should be fine as long as andThen and friends have the same bias type wise as implementation wise
Hmm I think the only way to ensure functions are functions, and effects are effects is to separate the access like the previous revision.
The issue is that it's valid to have a function be an effect, it's a dsl that we used in the past too if I recall in platform, so this issue is kind of orthogonal to tag
Only in implementation or also on the type level? If also on type level it should be fine as long as andThen and friends have the same bias type wise as implementation wise
No not necessarily, the function will return something different and expect potentially different arguments. It's not safe to assume andThen behaviour
No not necessarily, the function will return something different and expect potentially different arguments. It's not safe to assume andThen behaviour
How does such a case look like? can we add some failing tests?
No not necessarily, the function will return something different and expect potentially different arguments. It's not safe to assume andThen behaviour
How does such a case look like? can we add some failing tests?
Something like:
interface Wrapper<A> extends Effect.Effect<A> {
(arg: number): string
}
const wrapper = <A>(f: () => A): Wrapper<A> => {
function wrapped(n: number): string {
return `Hello: ${n}`
}
const base = Effect.sync(f)
Object.assign(wrapped, base)
Object.setPrototypeOf(wrapped, Object.getPrototypeOf(base))
return base as any
}
Effect.andThen(Effect.unit, wrapper(() => "OK"))
is currently prohibited by the NotFunction thing, I guess we have the choice: "it is not safe to discriminate between functions and effects but it can be safe to discriminate between functions and effects that are not functions"
in the case above we will be forced to rewrite to:
Effect.andThen(Effect.unit, () => wrapper(() => "OK"))
which will then work as expected
After a good amount of thinking it I think it is safe to discriminate between Function and NotFunction so this looks like it is working. The only troubles I can see are in tacit usage cases which we are advising against anyway