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

Recommend Cats Effect IO as a replacement for every use case of Future #4230

Merged
merged 2 commits into from
Jun 16, 2022

Conversation

armanbilge
Copy link
Member

Closes #4190.

This PR executes a soft deprecation of Future instances in Cats. The @deprecated annotation is not used. Instead, a @deprecated tag is used in the Scaladoc comment. This only affects rendering of API docs.

The compiler does not read Scaladoc comments, and therefore this cannot "junk" anyone's build. This addresses the concern from #4182 (comment).

Odds are, nobody will ever see these, because these are implicit instances that don't need to be imported or directly referenced in modern Cats. But it's better than nothing. I hope this is a reasonable compromise for everyone.

The language is taken verbatim from #4182 (comment). It also clarifies that these instances will not be removed from Cats and references the issue about the behavior change in traverse.

In addition, I propose we repeat this message in the Cats v2.8.0 release notes. Finally, we can also publish a scalafix lint via typelevel/typelevel-scalafix#2 as proposed in #4182 (comment).

Politely requesting reviews from Oscar and Ross since they were 👎 on hard-deprecation in #4182; thank you!

Screen Shot 2022-06-09 at 12 00 16

Screen Shot 2022-06-09 at 12 06 44

Screen Shot 2022-06-09 at 12 07 20

Co-authored-by: Patrick Oscar Boykin <[email protected]>
@armanbilge armanbilge added this to the 2.8.0 milestone Jun 9, 2022
@armanbilge armanbilge requested review from johnynek and rossabaker June 9, 2022 19:57
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize there was a @deprecated feature in Scaladoc. This seems fine to me.

Are we proposing Cats-Effect as the replacement, or specifically cats.effect.IO? Cats-Effect is a sprawling subject. IO is more directly the Future replacement.

@armanbilge
Copy link
Member Author

I agree that recommending IO specifically is probably more actionable.

@armanbilge armanbilge changed the title Recommend Cats Effect as a replacement for every use case of Future Recommend Cats Effect IO as a replacement for every use case of Future Jun 10, 2022
@armanbilge
Copy link
Member Author

@johnynek whenever you have a chance to look I'd appreciate your input here. This is one of the last things I'd like to get into Cats 2.8.0, thanks!

Copy link
Member

@danicheg danicheg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we had some module for unlawful things, it'd be nice to have instances for Future within there for, say, cats 3.0.

* @deprecated
* Any non-pure use of [[scala.concurrent.Future Future]] with Cats is error prone
* (particularly the semantics of [[cats.Traverse#traverse traverse]] with regard to execution order are unspecified).
* We recommend using [[https://typelevel.org/cats-effect/ Cats Effect `IO`]] as a replacement for ''every'' use case of [[scala.concurrent.Future Future]].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I endorse the advertisement of CE, but Monix is good too, maybe it's also worth mentioning?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point, sure! @danicheg what is the specific, referentially-transparent datatype we should recommend here, would it be Task? Is there any concern to recommend Monix before it has adopted CE3 since it cannot interop with maintained versions of the major libraries?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be fair, anyone who would have used Future would also have run into some interop problems 😂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could advise using some abstract IO datatype and refer to CE and Monix (to libraries, not their concrete datatypes). WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the kind of user who might be unintentionally using Future with Cats doesn't necessarily have the experience to appreciate what an "abstract IO datatype" is.

For the record, I have essentially no experience with Monix, but it seems to me there are an overwhelming number of datatypes in there, not all of which are RT. So I think it would be helpful to be more a little more specific.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a clear and direct recommendation for IO is best here. A user who is using Future in a way that's not aware of the RT implications isn't ready for writing tagless style code, so that's out. Monix isn't updated to CE3 and might not be a long time, so that's out (at least for now).

So, IO

Copy link
Member

@danicheg danicheg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a Monix user for years, I did my best at asking to promote it in these Scaladocs. So my conscience is clear.

@armanbilge
Copy link
Member Author

FWIW I did take a closer look at monix and indeed it seems that most/all of those datatypes are fundamentally RT, I think, just have more unsafe methods available than say IO. In any case, I understand a new version is in-development and I look forward to recommending it then :)

If there are no objections, I will merge this tomorrow and release Cats v2.8.0.

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

Successfully merging this pull request may close these issues.

Recommend using cats-effect as a replacement for _every_ use case of Future
4 participants