-
-
Notifications
You must be signed in to change notification settings - Fork 95
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
Fantasy Land method aliases #257
Comments
How I see this is that we could just document normal non namespaced functions and state somewhere which FL interfaces the structure conforms to. About tests if
I understand that Sanctuary emphasis function composition, but I don't think we should make it painful to do chaining. Also even though composition is great, Sometimes it's easier to add
That's a good point but if we see If in a code we are working with some specific version of Maybe and want to do map over it, then we would just just write If we are writing some generic function like Also to note I'm not saying that for example Related: my comment on same discussion in folktale#39 |
Absolutely. In fact, I have
I agree that we should consider the prefixed methods private. There are still two ways to do it, though: It's worth noting that I was able to improve performance significantly in #216 by removing type checking from methods. Since Ramda does not do type checking, methods such as
I agree. I'm happy for the aliases to exist; I'm just not comfortable documenting their existence. If advanced users want to use
Absolutely. The point is, though, that we can have |
so |
That's the current situation in #216, yes. Paying the cost of type checking twice—once in Do you think the methods should be type checked? I'm not convinced that it was correct to remove the type checking. |
AliasingPros
👍
👎 Cons
👌 I recently started a new project and was evaluating which libraries to use for functional tools and types. I like Sanctuary because it is much more opinionated than Ramda in various areas (type safety and variadic functions to name two). For that reason I am all for deprecating the non-prefaced versions of the methods. Sanctuary makes it easy to write code in a certain way, and helps me out if I make a mistake in a way that Ramda doesn't. Especially once Sanctuary includes functions for all the FL methods, I see no reason to use the non-prefixed methods in practice. There are plenty of other libraries for ADTs that will support the non-prefixed versions and are good choices if you want to use method chaining. On the other hand, I don't support removing the non-prefixed versions for a little while. It would be nice to not break compatibility with existing code until libraries are caught up with FL 1.0. I would argue that both the prefaced and non-prefaced versions should be considered a private interface. OfIt would be nice to have an Filter and RejectI am all for creating Sanctuary versions of Ramda functions where it makes sense to do so, and I do think this is a case where it makes sense to do so. Typechecked methodsGiven what I've said above, I think it's fair to not typecheck the methods. However, the one down side is then if I write a generic function to operate on a ADT it will lose out on the type checking. On the other hand, I could always define the function using |
Thank you both for your thoughtful feedback. I intend to do the following:
It doesn't exist yet, but we can add it in #216 once we agree on the appropriate type of |
I'm reconsidering this issue. My long-term goal is to move the Maybe and Either types to their own packages. These packages will only provide the data constructors and the various Given this goal, what is the best transition? Initially I had thought it wise to keep the unprefixed methods around for the next six months, say, to making upgrading to I'm now in favour of removing all the aliases from #232. This will mean code which invokes Sanctuary methods directly will need to be updated when upgrading to v0.12.0, but this cost will need to be paid at some point regardless. |
I think given the work you've done on If one doesn't want to use the functions provided, they can make sure to update their calling code, or just not update Sanctuary. Eventually that would have to be the case anyway, might as well do it once you have the functions available to manage the transition. |
Thanks for the additional feedback, folks. :) |
There are several related questions which must be answered before we can merge #216. Discussing these in a dedicated thread seems wise, as that pull request is very large.
Currently we provide Maybe and Either types compatible with the Fantasy Land spec prior to the merging of fantasyland/fantasy-land#146. In #216 we're adding support for the prefixed method names. Should we continue to expose methods via the unprefixed names? Concretely, should we provide
Maybe#map
andMaybe#fantasy-land/map
or just the latter?Reasons to alias (:alien:):
Reasons not to alias (:no_entry_sign: :alien:):
python -c 'import this' | grep 'only one'
);If we decide to include these aliases, there's a second question to answer: should we also provide
Maybe.empty
,Maybe.of
, andEither.of
? These are already aliases (for() => Nothing
,Just
, andRight
respectively). There's value, of course, in providing the prefixed equivalents to facilitate the creation of polymorphic functions, but if one is dealing with the Maybe type, say, I see no reason to writeMaybe.empty()
rather thanNothing
. Am I missing anything?The final question is what to do with
Maybe#filter
. It's currently defined for compatibility withR.filter
andR.reject
. My plan, though, is to definefilter
in sanctuary-type-classes, since it can be derived from other FL functions:Thanks, @joneshf, for sharing this approach way back in #10!
With
Z.filter
in place, #216 could easily addS.filter
andS.reject
. At that pointMaybe#filter
would only be useful for Ramda compatibility. Is that important, do you think?I have views on these matters, but I'd love to hear from @safareli and @Bradcomp (and others) before weighing in.
The text was updated successfully, but these errors were encountered: