Documentation refresh for monad transformers#4724

Open

reardonj wants to merge 3 commits intotypelevel:mainfrom

reardonj:documentation-refresh-monad-transformers

Contributor

reardonj commented Mar 4, 2025

This PR came out of a discussion in the Typelevel discord with @djspiewak and @armanbilge regarding unexpected interactions with monad transformers and Cats Effect.

This PR moves around monad transformer documentation to better reflect that it is a more advanced topic:

updates the monad transformer documentation
- to move the overview to a separate section instead of the jump start guide
- change uses of Future to Try to avoid suggesting developers use the deprecated Future instances
- add a Pitfalls section to warn users about unfortunate interactions with CE and direct users to Cats MTL as an alternative.
moves the Monad Transformer documentation pages out of Data Types to their own section.

I also made a few related changes for consistency:

remove the pointless Datatypes index pages that just listed all the datatype page links
move the Typeclass page to and Overview page under the Typeclasses section.
fix a broken link in guidelines.md

valencik reviewed

View reviewed changes

Member

valencik left a comment

NICE!!! 🎉
Thanks so much for putting in this effort, I'm really excited about it.

I think this is a good organizational change, I like the reduced Future usage, and less transformers in the return types.
I think we can go a little bit further calling out the pitfalls. Namely, let's repeat them on their respective transformer pages.
And let's call out the return types advice more prominently.

docs/monadtransformers/eithert.md

               `Either` can be used for error handling in most situations. However, when
-              `Either` is placed into effectful types such as `Option` or`Future`, a large
+              `Either` is placed into effectful types such as `Option`, a large

Member

valencik Mar 4, 2025

There's still an example using Future below this, but I think if we change that it should happen in another PR.

Contributor Author

reardonj Mar 5, 2025

Ya. Examples are awkward since one of the big motivations is IO, but we can't depend on CE here.

docs/monadtransformers/index.md Outdated Show resolved Hide resolved

docs/monadtransformers/index.md Outdated

+              The above `map` will return a value of type `OptionT[Try, String]`.
+              To get the underlying `Try[Option[String]]` value, simply call `.value` on the `OptionT` instance.
+              In practice, you should not include monad transformers in function signatures, and only use them as locally-scoped utilities to reduce boilerplate so that calling code is not required to care about the composition.

Member

valencik Mar 4, 2025

This sentence is kind of buried here, but I think it was a key point of the recent discussion about transformers.
Can we perhaps highlight this in a section of its own?

Contributor Author

reardonj Mar 5, 2025

docs/monadtransformers/index.md

+              class Account { /* ... */ }
+              class Money { /* ... */ }
+              def findUserById(userId: Long): Try[Option[User]] = { /* ... */ ??? }

Member

valencik Mar 4, 2025

Just want to note that these have changed from
OptionT[Future, User] to Try[Option[User]]

I think this is a good improvement if we're trying to recommend that folks don't use transformers in return types.

docs/monadtransformers/index.md Outdated

+              While powerful, this can lead to unwieldy type signatures and poor type inference.
+              The [Cats MTL](https://typelevel.org/cats-mtl/) project provides a capability based approach to extend effect types without requiring as much boilerplate.
+              Some monad transformers can also interact poorly with more powerful effect types that provide concurrent computation, such as Cats Effect's `IO` or fs2's `Stream`. You should avoid `StateT` and `WriterT` and use the concurrent data types provided by those libraries instead. Because `IO` already provides its own error channel, `EitherT[IO, Throwable]` can also lead to confusing behavior; prefer `IO`'s own error channel instead.

                
                    No newline at end of file

Member

valencik Mar 4, 2025

I think this warning should definitely be repeated on the EitherT, StateT, and WriterT, pages.

Contributor Author

reardonj Mar 5, 2025

Done

Member

valencik Mar 5, 2025

Thanks for the screenshots.
I think these are good.
The warning on EitherT is a little too up top front and center, but I don't think that's a blocker.

Contributor Author

reardonj Mar 5, 2025

I couldn't identify a nice place to put that one, EitherT doesn't have a convenient intro like the other ones.

docs/monadtransformers/index.md Outdated Show resolved Hide resolved

johnynek mentioned this pull request

Monad Transformer typeclass #4727

Open

reardonj added 2 commits

March 4, 2025 21:38


          extract monad transformers to their own section

c298cea


          Clean up index headers

9264f63

reardonj force-pushed the documentation-refresh-monad-transformers branch from 518bcd6 to 9264f63 Compare

March 5, 2025 03:10


          Address review comments

b8a8984

- add warnings to specific transformer pages
- call out recommended usage more specifically
- add CE link

Member

valencik commented Mar 8, 2025

To minimize these pitfalls, monad transformers should only be used as locally-scoped utilities to reduce boilerplate.
Do not include monad transformers in public method signatures.
Calling code can always use monad transformers themselves.

@typelevel/cats Anyone strongly disagree with this advice?

This PR has a somewhat large diff, but the focus of it is calling out this often repeated advice (and less examples with Future, and calling out more gotchas).

Member

valencik commented Mar 8, 2025

I should perhaps note, "strongly disagree" isn't the only option 😆
Adding more nuance is totally good too!

lenguyenthanh suggested changes

View reviewed changes

Member

lenguyenthanh left a comment

some nit about using * import instead of _ import

docs/imports.md

               import cats.syntax.all._
               ```
-              The `cats._` import brings in quite a few [type classes](typeclasses.md) (similar to interfaces) such as [Monad](typeclasses/monad.md), [Semigroup](typeclasses/semigroup.md), and [Foldable](typeclasses/foldable.md). Instead of the entire `cats` package, you can import only the types that you need, for example:

Member

lenguyenthanh Mar 9, 2025 •

edited

Loading

prefer to use cats.* as it is back-ported to scala 2, and underscore import is deprecated in scala 3. Similar in other places.

Contributor Author

reardonj Mar 9, 2025

There is one existing usage of a .* import in the docs. If we change it, I would much rather see it done as a separate PR doing the replacement everywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet