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

Consider changing typeclasses signatures to be always data-last #1216

Open
gcanti opened this issue May 19, 2020 · 9 comments
Open

Consider changing typeclasses signatures to be always data-last #1216

gcanti opened this issue May 19, 2020 · 9 comments
Milestone

Comments

@gcanti
Copy link
Owner

gcanti commented May 19, 2020

Pros (from #942 (comment))

  • API inconsistency: pipe is an idiomatic way to use fp-ts however its abstract core - the typeclasses - have signatures not suitable for this idiomatic way out of the box and require some messy (redundant IMO) steps like calling pipeable, destructuring, reexporting and so on. When they actually could be used as is.
  • transformers: if I need a "pipeable" version (an idiomatic version actually) of some transformer I need to register its type as a separate monad on the HKT dictionary. This doesn't make much sense because the type is already "bundled" in the transformer instance (for example the result of getOptionM already contains info about Option and passed M.
  • pipeable module itself is a mess with no sensible profit - it just fixes data-first typeclasses when they could be data-last by design completely eliminating the need for this module.

Cons

  • breaking change
  • performance implications?

Prior art

EDIT: I agree with the proposed change, so I'll add my vote as 👍

@mikearnaldi
Copy link
Contributor

I did some further testing, it doesn't look so bad performance wise (roughly 30% loss but case dependent), in case of critical places there are still means of optimization by exposing specific data-first functions outside of the instances so I'll also vote for it.

@gcanti
Copy link
Owner Author

gcanti commented Jun 9, 2020

@raveclassic I have a proposal to make this change kind of "backward compatible". Switching to data-last type class is indubitably a breaking change for "power users" (e.g. library authors). However "normal" users always interact with type class instances only as a way to configure constrained functions, for example A.sequence

import * as A from 'fp-ts/lib/Array'
import * as E from 'fp-ts/lib/Either'
import { pipe } from 'fp-ts/lib/pipeable'

pipe(
  [E.right(1), E.right(2), E.right(3)],
  A.sequence(E.either), // <= as a user I really don't care what `E.either` actually is
  E.bimap(
    () => 'error',
    () => 'ok'
  )
)

Now E.either is pretty bad from a tree-shakeablity point of view so I'm thinking of splitting this kind of mega instances into specialized ones. For example an applyEither instance (which is now internal, i.e. not yet ufficually exported), an applicativeEither instance, an monadEither instance, etc...

Those internal instances are data first now.

Here's come the trick, what if we export a data-last applicativeEither instance instead, and then make A.sequence compatible with both kinds of type classes (data-first and data-last)?

@raveclassic
Copy link
Collaborator

@gcanti

make A.sequence compatible with both kinds of type classes (data-first and data-last)?

How would you do that?

@gcanti
Copy link
Owner Author

gcanti commented Jun 9, 2020

@raveclassic POC

EDIT: I'm temporary using Traversable1<URI>['sequence'] but we can't change it, we need another type there

@raveclassic
Copy link
Collaborator

Well... The trick with _tag: 'data-last' is awesome but it turns out we need to maintain both data-first and data-last interfaces for all typeclasses in the lib (sic!) as well as write 2 convertors (data-first <-> data-last) per each typeclass in the lib (sic!) as well.

I'm not sure it's worth the maintenance effort. I'd rather spend a couple of days migrating to a data-last version of the lib.

@gcanti
Copy link
Owner Author

gcanti commented Jun 9, 2020

Yeah, actually I'm not fond of this solution neither, but since I'm going to split the mega instances, it's kind of a unique occasion and I wanted to hear your feedback and to make sure all paths were explored before proceeding.

@raveclassic
Copy link
Collaborator

raveclassic commented Jun 9, 2020

I'm totally ok to split the instances if they will still contain each other

const functorEither = {
  URI,
  map: ...
}
const applyEither = {
  ...functorEither,
  ap: ...
}

Even more, I'd suggest to change name to instanceFunctor, instanceApply, etc. since on practice we don't need typeclass name in the instace. This would also address several issues about import ergonomics (including famous array.array, either.either etc)

@gcanti
Copy link
Owner Author

gcanti commented Jun 10, 2020

since on practice we don't need typeclass name in the instace

@raveclassic do you mean "data type name in the instance"? instanceFunctor, instanceApply actually contain the type class name.

Another question: if all Functor instances are named instanceFunctor (instead of functorOption, functorEither) isn't more difficult to import them automatically?

Note. Please do not answer here, since now the argument is no more strictly related to data-last type classes I opened #1237

@gcanti
Copy link
Owner Author

gcanti commented Jan 2, 2021

(super-early-pre-alpha version)

branch: https://github.com/gcanti/fp-ts/tree/3.0.0

or npm i fp-ts@next

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

No branches or pull requests

3 participants