Hi!
I have just found myself currying by hand yet another function like insert/remove from StrMap in fp-ts and Lens#modify in monocle-ts to use it with compose and pipe.
Here's the question therefore - are you going to include curried versions of these helpers? Or maybe replace current with curried and export uncurried with a postfix like it's done in sanctuary:
export const remove_ = <A>(k: string, d: StrMap<A>): StrMap<A> => { ... }
export const remove: (k: string) => <A>(d: StrMap<A>) => StrMap<A> = curry(remove_);
My policy so far is simple
Example
import { Applicative } from '../src/Applicative'
import { HKT } from '../src/HKT'
import { option, some } from '../src/Option'
import { right } from '../src/Either'
// curried version
declare function traverse1<F>(F: Applicative<F>): <A, B>(f: (a: A) => HKT<F, B>, ta: Array<A>) => HKT<F, Array<B>>
// x: HKT<"Option", number[]>
export const x1 = traverse1(option)(a => some(a), [1, 2, 3])
// export const x2 = traverse1(option)(a => right(a), [1, 2, 3]) // error :)
// uncurried version
declare function traverse2<F, A, B>(F: Applicative<F>, f: (a: A) => HKT<F, B>, ta: Array<A>): HKT<F, Array<B>>
// x3: HKT<"Option" | "Either", number[]>
export const x3 = traverse2(option, a => right(a), [1, 2, 3]) // no error :(
I'm definitely open to suggestions in order to change the policy (in general I would avoid API duplications though)
Well, at the beginning I was trying to change an object through lens multiple times and noticed that all modifying functions on every structure (Array, StrMap etc.) are uncurried as well as Lens#modify so at first the code looked like:
type Entity = {
id: string,
foo: string
};
type State = {
ids: string[],
entities: StrMap<Entity>
};
const ids = Lens.fromProp<State, 'ids'>('ids');
const entities = Lens.fromProp<State, 'entities'>('entities');
//signature is aligned with redux reducers (that's why state goes first)
const removeFromState = (state: State, id: string): State => compose(
state => ids.modify(ids => filter(id_ => id_ !== id, ids), state),
state => entities.modify(entities => remove(id, entities), state)
)(state);
But it could be:
const removeFromState = (state: State, id: string): State => compose(
ids.modify(filter(id_ => id_ !== id)), //curried modify from Lens and curried filter from Array
entities.modify(remove(id)) //curried modify from Lens and curried remove from StrMap
)(state);
Curried version looks much more cleaner. Also I don't think that performance penalty needed for currying is critical, but if it is, curried versions can be changed to uncurried with postfix anytime.
Also it looks like API is a bit inconsistent if some functions are uncurried to be aligned with javascript defaults and some are not to fix TS type inference. I think it would be better to preserve consistency throughout the codebase.
What do you think? If it looks like a massive breaking change then maybe you could export curried versions with prefix/postfix? I saw curriedConcat in Array.
Also it looks like API is a bit inconsistent if some functions are uncurried to be aligned with javascript defaults and some are not to fix TS type inference
I recently mentioned this as well and it breaks compatibility with Ramda too. I plan on looking into a solution, but until type inference gets better, it's a bit of a hairy issue.
I think it would be better to preserve consistency throughout the codebase
So do I, the point is consistency with respect to which policy? Current APIs are consistent to the base policy mentioned above
APIs are never curried unless type safety is compromised
(well except some mistakes like Functor.voidRight that I'd like to fix)
We can definitely change the policy, or change any API, name or module, if the ergonomics can be improved.
However I'll refuse any change which leads to lesser type safety (which is a key point of this library)
However I'll refuse any change which leads to lesser type safety (which is a key point of this library)
I understand this clearly and do agree but I don't get what kind of regression are you talking about mentioning curried functions. Your example with traversables shows that curried version is better then uncurried in type checking and emits compile-time error.
traverse1(option)(a => right(a), [1, 2, 3]) // error :)
traverse2(option, a => right(a), [1, 2, 3]) // no error :(
Do I get it wrong?
If you mean that functions with broken type inference should not be exported at all then the way to export uncurried versions with a postfix fits even better because such function can be dropped in a particular case. Or the lib can fully drop all uncurried versions for the sake of consistency. Though with a light perfomance penalty.
Your example with traversables shows that curried version is better then uncurried in type checking and emits compile-time error
Indeed! Sorry, I expressed myself poorly, my example was just to show and justify the rationale behind my choice, i.e. a mix of uncurried (the "standard") and curried functions (the exceptions) and why I find it consistent with the goal of this library (type safety first of all), not against your proposal :)
Ah I see :)
So the question's left: ~to be or not to be~ to curry or not to curry?
It is _extremely_ handy to use curried versions with compose/pipe but I know it's a massive breaking change. What do you think?
Well, the good thing about a type system is that refactorings are simpler. I don't mind to have breaking changes if they are well justified (by improved type safety or improved ergonomics)
We could put up a list of breaking changes we all agree upon. Perhaps some pratical examples would also help to focus on the new policy.
Also, if we are going to publish a breaking change release, then I'd love to fix some type safety issues (like in voidLeft / voidRight mentioned above)
In other words we can work on a "clean-up" release, which hopefully will fix all my arguable choices and bring us closer to v1.0.0
I'll start from the most important change with respect to type safety
Type constraint rule. If there is a type constraint then a function must be curried, at least in its type constraint arguments
Motivations
Type safety. For example the current uncurried version of Functor.voidLeft is unsafe
import { voidLeft } from '../src/Functor'
import { option } from '../src/Option'
import { right } from '../src/Either'
voidLeft(option, right(1), 2) // no error
Ergonomics
It will be easy to specialize a generic function like Functor.voidLeft or Functor.lift
import { lift } from '../src/Functor'
import { option } from '../src/Option'
const optionLift = lift(option)
What about Binary operations?
There are plenty of them: Setoid.equals, Semigroup.concat, Alt.alt etc..
Are the majority of usages saturated?
import { equals, some } from '../src/Option'
import { setoidNumber } from '../src/Setoid'
const optionEquals = equals(setoidNumber) // <= Type constraint rule
optionEquals(some(1), some(2)) // <= typical usage?
// or?
optionEquals(some(1))(some(2))
For reference, new v8 engine (Turbofan) performance characteristic (includes currying)
(https://www.nearform.com/blog/node-js-is-getting-a-new-v8-with-turbofan/)
@raveclassic Or we can tackle this issue the other way around: let's say we go full curried, i.e. the policy would be dead simple: all functions are completely curried, always. Any cons? For example awkwardness, e.g. myApi(a)(b)(c)(d)? Performances? Bad stack traces? Other?
one can derive a curried version from a unccurried, performant code.
one cannot derive a unccurried (performant) code from a curried version.
one can derive a type correct code from a type correct code.
one cannot derive a type correct code from a type incorrect code.
from my understanding, performant, uncurried code can sometimes be incorrect.
also we want to have correct, type safe code.
Therefore, by deduction, I would think that the best would be to have:
Is it correct or am I missing something?
@sledorze
new v8 engine (Turbofan) performance characteristic
Nice! Seems like currying doesn't affect performance notably.
Is it correct or am I missing something?
AFAICS that's correct and that's why I'd suggest keeping original perfomant uncurried functions with a postfix like it's done in sanctuary.js and provide handy curried versions with a name without a postfix.
Like uncurried remove_ and curried remove. Still if there's a type-checking regression in an uncurried function then only curried should be exported like in the case of voidLeft.
@gcanti IMO it's really that simple - if we curry, then we do it fully. Even binary functions. I don't see any reasonable cons except that you've mentioned. But the profit of currying outweighs them.
@sledorze That is correct, but I'm afraid you end up with a mess. I'm wary of duplicated APIs. And what about type classes and instances?
Type classes
export interface Functor<F> {
readonly URI: F
map<A, B>(f: (a: A) => B, fa: HKT<F, A>): HKT<F, B>
map<A, B>(f: (a: A) => B): (fa: HKT<F, A>) => HKT<F, B>
}
I'm not even sure it compiles...
(Ok, I tried.. it doesn't compile)
Instances
// Either.ts
export function getSetoid<L, A>(setoid: Setoid<A>): Setoid<Either<L, A>> {
return {
equals: (x, y) => fx.equals(setoid, fy) // uncurried version
}
}
export function getCurriedSetoid<L, A>(setoid: Setoid<A>): Setoid<Either<L, A>> {
return {
equals: x => y => fx.equals(setoid, fy) // curried version
}
}
@raveclassic I'm leaning towards the "curry all the things" policy, it's the only clean and consistent solution (with perhaps some notable exceptions, like compose or pipe)
I'd prefer to not clutter the codebase with two (o more) versions of everything (a priori).
I'd love to have one and only one well-typed version of each API.
Then, if you hit a (real, benchmarked) performance problem, you can always optimise that particular computation by hand. Or, in the face of a verified full-blown problem, we can even provide an additional uncurried API (a posteriori)
@gcanti Yep, sounds good! Actually I'm not getting hit by any performance problems so I'm ok with the only one (curried) version.
@gcanti that's good to me
Wow the first phase took longer than I thought.
Once https://github.com/gcanti/fp-ts/pull/212 is merged I think we could talk about the next steps, any ideas?
EDIT: FYI both fp-ts#master and fp-ts#FR/migrate_to_curried compile fine with typescript@rc (2.5)
@gcanti Sorry, I'm away from my laptop for the weekend. If you feel you can merge a PR - do it :) We only need to track what's left uncurried
Anyway until TS provides proper type inference I think we could go as is for now and pay more attention for doc generation of required quality.
All in all, thanks for the help, it's a large step towards improvement of the lib!
Great work guys!
So, as #202 is merged, we can close this!
@raveclassic I'm going to rebase https://github.com/gcanti/fp-ts/pull/199. Then I'll publish a fp-ts@next version in order to gather some early feedback
Released
@raveclassic how is going with fp-ts@next and monocle-ts@next? Any (negative) feedback?
@gcanti The flight is nominal :) No problems were found.
Actually I'm missing some methods on array like find :: Predicate a -> Array a -> Option a, just have no time to add them.
@raveclassic Thanks. Both officially released.