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

Decide how public-facing instance traits should be #3008

Open
travisbrown opened this issue Aug 26, 2019 · 4 comments
Open

Decide how public-facing instance traits should be #3008

travisbrown opened this issue Aug 26, 2019 · 4 comments

Comments

@travisbrown
Copy link
Contributor

Right now we have AllInstances and AllSyntax traits that are in the public API and are sometimes used outside of Cats itself—for example in Circe we have a CirceSuite helper class that extends these traits so that we don't have to import cats.implicits._ in every test file.

The problem with this kind of usage is that AllInstances and AllSyntax don't actually include all of anything, ever since we started stacking on BinCompatN layers to maintain binary compatibility on Scala 2.11. For example, want a Defer for Function0? You won't get it from AllInstances—you also need AllInstancesBinCompat0.

This means anyone using AllInstances or AllSyntax has to be careful to add the latest AllSyntaxBinCompat37 or whatever traits included in each new Cats release if they actually want everything.

This is something we haven't managed to stay on track on even within Cats itself—e.g. see #3007, which is due to the fact that cats.implicits is lagging two AllSyntaxBinCompats behind, or SyntaxSuite, which is four AllInstanceBinCompats out of date (but happens to still be working), etc.

To make things even more confusing, there's a AllSyntaxBinCompat (note the missing number) abstract class that bundles up all the syntax bin-compat traits, but no equivalent AllInstancesBinCompat.

I've also recently proposed (and merged) making the bin-compat traits package-private (#3003), which on second thoughts I think we'll have to undo, at least until 2.1.

So there's a lot of miscellaneous little brokenness and inconsistency and mildly bad UX here, some of which I'm about to open a PR to address, but there's also a bigger question of whether we actually intend the AllInstances and AllSyntax traits to be used outside of the implementation of Cats itself.

travisbrown added a commit to travisbrown/cats that referenced this issue Aug 26, 2019
kailuowang pushed a commit that referenced this issue Aug 26, 2019
* Add AllInstancesBinCompat to match AllSyntaxBinCompat

* Add bin-compat traits to implicits import, fixing #3007

* Bring test object and trait up to date on BinCompat levels

* Make AllXBinCompat traits public again (#3008)
@Daenyth
Copy link
Contributor

Daenyth commented May 15, 2020

Just as a data point, I've used those in third-party code so that I could make one combined import for cats implicits + my library implicits + another library implicits, to help reduce brain effort on imports

@djspiewak
Copy link
Member

As another random data point… I originally structured djspiewak/shims to hide the "All" traits, so the only way to actually get at anything was via the shims._ import. The problem with this is it made it impossible for any third parties to participate in the implicit prioritization, which created unavoidable conflicts. Given Cats' nature, I would suspect similar problems if we privatized the traits.

@joroKr21
Copy link
Member

But this is only relevant for syntax now, isn't it?

@djspiewak
Copy link
Member

But this is only relevant for syntax now, isn't it?

Yes, which significantly decreases (though does not entirely remove) the relevancy of interaction with prioritization.

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

No branches or pull requests

4 participants