search

origamitower / folktale

[not actively maintained!] A standard library for functional programming in JavaScript
https://folktale.origamitower.com/
MIT License
2.04k stars 102 forks source link

Move to prefixed Fantasy Land methods #39

Closed robotlolita closed 7 years ago

robotlolita commented 8 years ago

As of 0.3 the Fantasy-Land specification now uses prefixed names (i.e.: type['fantasy-land/map'](fn) rather than type.map(fn)). Folktale 2 should support this accordingly.

Because we're still supporting the same set of features older versions of Folktale do, so people can transition their codebase without as many problems, we'll be using compatibility methods to let people call these namespaced functions. That is, each type will look like this:

Type.prototype['fantasy-land/map'] = function(fn) {
  return Type(fn(this.value));
};

Type.prototype.map = function(fn) {
  return this['fantasy-land/map'](fn);
}

We should also encourage people to use the Core.FantasyLand module instead of calling the methods directly when they're writing generic functions (e.g.: a function that works with any functor, rather than just Folktale's Maybe). This direction is documented here: https://github.com/origamitower/folktale/blob/master/ROADMAP.hs#L106-L297

Types that need fixing

See https://github.com/fantasyland/fantasy-land/pull/146 for details on the Fantasy-Land decision.

abuseofnotation commented 8 years ago

I am all for supporting Fantasy Land, but are we sure we want to remove the old names?

They will still be useful for playing in the REPL, or for code examples (even the Fantasy Land Spec uses them in that context).

Plus, I imagine that not all folktale users care about Fantasy Land (some probably just want a safer alternative of promises, for instance) and for them the prefix will be just an annoyance.

robotlolita commented 8 years ago

That's a good question.

What I want here is to nudge people into moving from type.map(f) and friends to Folktale's map(type, f) (or type::map(f), but this one still depends on https://github.com/tc39/proposal-bind-operator/ being accepted. The proposal has a new champion, but things in tc39 move slowly…), for their production apps.

This is important for a few reasons:

But you're right about things like code examples and REPL. So perhaps instead of outright deprecating it, we could have an optional warning that asks people to use the function version for their production code, and then you could turn that warning on with something like FOLKTALE_RECOMMENDATIONS=on node myprogram.js.

abuseofnotation commented 8 years ago

I can definitely agree that having a unified map chain etc.) functions would bring many advantages, when combining several Fantasy Land libs. At the same time, for me personally, it's sometimes a bit off-putting to have to explicitly require such functions one by one, instead of getting them with my data.

The warnings are a good idea. When I read about them I immediately thought of that instanceof check that we do and that fails if the Folktale versions are different. But I now see that you added a separate option for that one.

UPDATE: An alternative is to just use low log levels for these messages.

safareli commented 8 years ago

My thoughts about namespaced methods:

All libs which require structure to implement some FL interface should depend on FL namespaced methods. this way it would be easy to change one type with other and libs will work together well. so basically I see FL namespaced methods as communication mechanism between libs.

But this doesn't means that those libs should not have normal, non namespaced methods. Maybe some of us prefer functions over methods but that's just our opinion, others like methods as they read better for them.

What we should do is make those libs as accessible to all kinde of developers as we can. i think we should just state for each structure which FL interfaces it conforms to, but list only normal non namespaced methods so it's easy to play with and use.

I dont see any reason to deprecate or log anything when someone use .map(f) instead of calling fantasy-land/map as @boris-marinov mentioned some users don't care about FL at all thay just want better promise, so it would be best combination to use FL between libs but provide user friendly api to users too.

Also this way maybe we would chiang name of chain to endThen (as elm has) or something like that, but still conform to FL Chain spec. and if someone uses ramda for example it's chain will delegate to fantasy-land/chain (if exists and if not then to chain) and everything will work seamlessly.

robotlolita commented 8 years ago

I've been thinking about this in the past weeks (work's been eating all my time lately :x), and yeah, there isn't really any reason we couldn't have the methods.

It also just struck me that all functions in Fantasy Land use consistent types anyway, so someValidation.ap(x) requires x to be the same type as someValidation. So, wherever users are writing non-generic functions (e.g.: a function that works with a Maybe, rather than a Functor), it's always safe to invoke the short method (maybe.map(f) is guaranteed to always work).

The problem only happens when users are writing generic functions (e.g.: a function that works with any Functor), in those cases, functor.map(f) is not safe, because different libraries might have defined the methods in different places.

It would be nice if we could warn people of this, but we can't really detect the cases where it matters, so I suppose our best option is (rather than outputting warnings) describing this in the documentation, and recommending people to use the Core.FantasyLand module's functions when writing generic operations.

Thanks @boris-marinov and @safareli for the input :)

safareli commented 8 years ago

Nice 👍

We should state somewhere in doc that if user is writing generic code over any Functor, then FL namespaced methods should be used. Logging might be sort of annoying as it requires configuration, I think just a word in doc will be enough, but it's not that big issue.