sanctuary-js / sanctuary

:see_no_evil: Refuge from unsafe JavaScript
https://sanctuary.js.org
MIT License
3.04k stars 95 forks source link

reinstate S.meld #303

Closed davidchambers closed 7 years ago

davidchambers commented 7 years ago

@ackerdev, @MikaAK, and @JAForbes recently mentioned on Gitter that they find meld useful. The function was removed in #280 after failing to receive support in #278.

At least a handful of people find this function useful, so I'm happy to keep it around. We haven't published since #280 was merged, so it would appear that the function was never removed. ;)

Please respond with :thumbsup: if you'd like to see meld reinstated.

rjmk commented 7 years ago

Should we :-1: if we don't like it or is just critical mass with :+1:s?

I missed the gitter conversation, I'm afraid. Do you have a link handy?

EDIT: Look here

davidchambers commented 7 years ago

The conversation was fairly short. Someone said the function is useful and will be missed, and a couple of people quickly replied in agreement.

The function was removed because its type cannot be expressed in Hindley–Milner, and because I was under the impression that it simply isn't a useful function. If people do in fact find it useful, I'm happy to keep it around.

rjmk commented 7 years ago

Fair enough. I suppose it's a fairly natural function for JS developers writing functionally

safareli commented 7 years ago

personally, i think codeNameOneOf1 is better than codeNameOneOf2

const codeNameOneOf1 = R.curry((as, x) => R.contains(x.codeName, as))
const codeNameOneOf2 = R.flip(S.meld([R.prop('codeName'), R.contains]))

codeNameOneOf([‘test’, ’test2])({codeName: ‘test’})
R.filter(codeNameOneOf([’test2’, ’test3’), […items])

we even didn't had description which clearly expressed what the meld was doing, one could not even take a look at tests to understand because it's way to mysterious

also using curryN in it's implementation we are limited to only passing 10 arguments, so if you have 4 functions taking 4, 4, 3 and 3 arguments we will get error as curryN only accepts max 10 as it's input.

rjmk commented 7 years ago

I do agree with @safareli. I understand the frustration with constructing more complicated pipelines of functions and the desire to do so pointfree, but I don't much like meld. I am also in general pro fewer functions in Sanctuary, so if something can be removed while keeping Sanctuary usable, I'd like to. I understand opinions may differ here though

svozza commented 7 years ago

That we needed a diagram to describe what this function did makes me think were possibly going overboard by including it.

MikaAK commented 7 years ago

I feel like meld is actually quite useful and there's more than one time where I could of used it in the past and ended up having to have something that instead looks like const fn = curry((a, b, c) => lastFn(otherFn(someFn(a))(b))(c)) vs const fn = meld([someFn, otherFn, lastFn])

ackerdev commented 7 years ago

So, I somewhat frequently find myself wanting to create a function like (x, y) => f(g(x), y), and being able to create a pointfree version of this would be nice. I find myself doing this somewhat frequently, though perhaps it's a symptom of using functional JS constructs in imperative code.

Before sanctuary, I was using R.useWith(f, [g, identity]) which does work but doesn't read so well and is a little wonky to understand.

S.meld([f, g]) was definitely a little more difficult to grok at first, but created a nicer result that was easier to read after meld was grokked.

I'm not attached specifically to meld, and while I like it I'd also settle for having a good suggestion of an alternative to these if one existed.

davidchambers commented 7 years ago

I think you mean S.meld([g, f]) rather than S.meld([f, g]), @ackerdev.

Have those of you with meld experience run into problems with higher-order functions? Consider this example:

> S.meld([S.add, R.map]).length
3

> S.meld([x => y => S.add(x, y), R.map]).length
2

> S.meld([x => y => S.add(x, y), R.map])(10, [1, 2, 3])
[11, 12, 13]

Having to manually curry S.add in this case is a bit of a nuisance, especially since it works correctly when applied to its arguments one at a time. The problem is that S.add and R.map are both binary according to their length properties, and meld has no way to know that we'd like to treat S.add as a unary function while treating R.map as a binary function.

As soon as I found myself manually currying functions to achieve the desired results, meld lost much of its lustre.

ackerdev commented 7 years ago

@davidchambers Yep, you're right. Wasn't paying full attention.

I haven't used meld enough to run into that case yet, but I can see why that would be confusing and inconvenient (and cases like this are why I'm not too attached to meld in particular). useWith works better there, like R.useWith(R.map, [S.add]).

I'd be pretty ecstatic to have a clearer representation of that, obsoleting both of my primary usages for meld and useWith. Unfortunately I'm still dipping my toes in functional and don't really know if any constructs exist for this outside of the JS FP ecosystem yet.

davidchambers commented 7 years ago

I'm on the fence here. There's more enthusiasm for this function than I realized initially, but there are also people who believe it does not belong in the library.

Would you consider a pull request to add meld to Ramda, @CrossEye? It's a better fit for Ramda than it is for Sanctuary.

davidchambers commented 7 years ago

We've decided that this function does not fit with Sanctuary's philosophy (which we should write about and publish on the website). If you're a fan of this function you could express your support for it in ramda/ramda#2076.