Open gcanti opened 4 years ago
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.
@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)?
@gcanti
make A.sequence compatible with both kinds of type classes (data-first and data-last)?
How would you do that?
@raveclassic POC
EDIT: I'm temporary using Traversable1<URI>['sequence']
but we can't change it, we need another type there
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.
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.
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)
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 https://github.com/gcanti/fp-ts/issues/1237
(super-early-pre-alpha version)
branch: https://github.com/gcanti/fp-ts/tree/3.0.0
or npm i fp-ts@next
Pros (from https://github.com/gcanti/fp-ts/pull/942#issuecomment-524912701)
pipe
is an idiomatic way to usefp-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 callingpipeable
, destructuring, reexporting and so on. When they actually could be used as is.HKT
dictionary. This doesn't make much sense because the type is already "bundled" in the transformer instance (for example the result ofgetOptionM
already contains info aboutOption
and passedM
.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
Prior art
EDIT: I agree with the proposed change, so I'll add my vote as :+1: