Entity Decoder - Remove EitherT #6875
Replies: 2 comments 2 replies
-
All breaking changes should be run through a cost-benefit analysis.
In a vacuum, I see a small benefit.
The change could be made cheaper which would make this more appealing, but it's one more hurdle to the adoption of 1.0. At $WORK, many projects are stuck on 0.21, and some further behind that, because breaking changes are really expensive, and the more an upgrade costs, the less likely it is to ever happen. This increases developer drag: I spent part of my day off yesterday preparing a security fix for four versions. I think the bar here is not "makes things a little nicer," but rather, "is clearly worth the hassle of developer attention to upgrade and maintain." I don't see that the benefits clearly outweigh the costs here. If we were to make a grander change, like EntityCodecs as data (i.e., explicit, like Skunk) rather than as a type class instance (i.e., implicit, like Circe), then I could imagine this being part of it: that change is much more expensive and beneficial. But that proposal is a whole other discussion. |
Beta Was this translation helpful? Give feedback.
-
Actually, both this proposal and that are part of my grander idea: to finally support content negotiation. I will try to find time for a sketch of that. |
Beta Was this translation helpful? Give feedback.
-
The PR #6846 carries the changes to the
main
branch needed to remove the use ofEitherT
fromThe PR changes the external API of
http4s
in these two ways.DecodeResult
changes from referring to theEitherT
monad transformer to anEither
of a failure or the result type. This makes it similar to the nearbyParseResult
alias, or to theDecoder.Result
alias in circe.The interface for an
EntityDecoder
changes from returning the previousEitherT
alias, to the unwrappedF[Either[DecodeFailure, A]]
.The motivation for this change is to remove the use of the
EitherT
in this part of the public API.From what I have seen in the PR, the only disadvantage is loosing some of the methods from the
EitherT
class, but these are not widely used and their non-transformed replacement do not take that many lines in comparison.Discussion or motivations
An
EitherT[F[_], L, R]
is a wrapper around anF[Either[L, R]]
. Choosing to use one or not comes down to whether it helps to understanding, makes it easier to use the value, and fits the context where it's used. Since no one can understand the wrapper without its contents, the current way is at least one step more difficult. The only way a transformer help is when used in MTL-style code, which generalizes over other notions of monad. However, this is not commonly used for HTTP routes, and when used it is better left as an opt-in than an opt-out.The purpose of an
EntityDecoder
is to decode the entities that accompany a request in a server, or a response in a server. In both the server and client definitions, there are other forms of error raising and error handling that are not done throughEitherT
, so this wrapper does not fit in the context where it is often used.Beta Was this translation helpful? Give feedback.
All reactions