-
Notifications
You must be signed in to change notification settings - Fork 107
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
New, optimized version of Pipeline Operator #20
Comments
I kinda liked calling the right hand side as a unary function since it was obvious how it was getting called, however the performance implications and the fact that it fits in nicely with partial function application-centric libraries is good enough reason for me. |
@mindeavor let add = x => y => x + y
100 |> add(10) Here I'm expecting |
Anything with syntactic transformations is unfriendly to patterns like dynamically generating functions, or grabbing the function you want to pipeline beforehand and storing it in a variable. This makes it much less useful. The previous version that was merely an alternate In particular, syntax like:
no longer works - it'll try to insert the argument into the loggify() call, rather than the resulting function. This makes it harder to refactor code - you can't start with The fact that this plays badly with non-pipelining functional code is not worth the 5-character savings of being able to omit the wrapper arrow function. |
@seanstrom Under the new proposal you can accomplish that by using parenthesis: let add = x => y => x + y
100 |> (add(10)) //=> add(10)(100) @tabatkins I think a general log function will solve that issue: let log = (x) => (console.log(x) || x);
64 |> sqrt(); // Original
64 |> sqrt() |> log(); // Logged version
64
|> sqrt()
|> log() // <-- Easy to comment out
; |
@mindeavor would @tabatkins log function also work with the defined syntax you used in my example? |
@seanstrom Certainly: let log = (x) => (console.log(x) || x);
let add = (x) => (y) => x + y;
100 |> (add(10)) |> log() //=> log( add(10)(100) )
// Or even:
100 |> (add(10)) |> log() |> (add(20)) //=> add(20)( log( add(10)(100) ) ) |
@mindeavor oh sorry I mean to ask that @tabatkins could wrap the function call in parens too. let loggify = f => x => {
let r = f(x)
console.log(r)
return r
}
64 |> (loggify(sqrt)) |
@mindeavor I feel hesitant that users will accept the additional syntax for calling a function inlined, instead of additional syntax to annotate argument list pushing. |
Have we considered using a syntax for the placeholder arguments like so: 100 |> _.reduce([1,2,3], &1, fn) This syntax is borrowed from how I believe Elixir allows for lambda expressions with argument placeholders, just slightly re-purposed. |
@seanstrom I understand where you're coming from, but I do believe non-curried functions are the vast majority of JavaScript. In fact I think that the main reason there is a small presence of currying in JavaScirpt at all is because of ES5's verbose functions. If arrow functions had always existed, I doubt most anyone would be inclined to manually curry their functions. Re placeholders: This has been discussed in #4. The consensus is adding additional syntax for |
@mindeavor I'm less enthusiastic about this version, it seems much more implicit than the previous proposal and I find it more difficult to read. You've got to keep in your head that |
I agree n-ary are more popular than unary functions at the moment, but piping composes very well with unary functions by default. It's only when a community is already used to n-ary functions that we have to design a shorthand around what can be seen as building a curried function that will eventually call the n-ary function. That's why I suggested place holder arguments since they seem like a good way of modeling that. |
Arrow functions over Placeholder Arguments but |
@eplawless Yes, that is a valid downside to this version. On the other hand, you could make a similar argument against @seanstrom I think we're going to have to agree to disagree on this point. As much as I love FP, I don't see the JS community ever moving towards curried functions by default. As for "un-rewarding", to me the motivating examples speak otherwise :) |
@mindeavor I feel like you're missing the point. I'm not saying currying is the only way or the best way. I'm suggesting that when we add support for multi-parameter functions, we do so by building on top of the syntax instead of overloading syntax if possible. Again val |> (partialFn(otherVal)) // partial application
val |> unaryFn() // unary function |
I liked your initial proposal better, for its simplicity. It mixes very well with the new |
@mindeavor with the old version, there was a way to reduce the cognitive load in targeted cases where the higher order functions weren't obvious, using arrow functions: x |> x => f(10)(x) We lose that in the new proposal. It also allowed very clear "placeholder" syntax. x |> _ => f(1, _, 3) The new proposal is a behavior of this operator that we haven't seen before, and I think it has some real downsides. I'd urge you to really consider whether this is the right direction for it. |
@seanstrom I think I understand what you're saying, but the only hinderance I can see is the usage of curried functions. All other functions seem to be perfectly fluent. Here's my stance on the issue. I love FP, and often curry my own functions. However, there are tradeoffs to be made. The new pipeline version effectively suffers the need to type curry-parenthesis, but gains the ability to integrate with prototype methods while still being useful for normal functions. This is an important point; you don't want to switch to a different style of chaining and not be able to go back. Of course, it very well may be that the original is better. Which I would have no problem with; I would just like to exhaust all practical possibilities before a decision is made. |
@eplawless That's a really nice placeholder syntax! And don't worry, "considering" is all I am doing at the moment :) |
Before I forget, there is one issue with the original proposal: associativity, especially with arrow functions. arg |> x => f(x, 10) |> y => g(y, 20)
// Parsed as:
( arg |> x => f(x, 10) ) |> y => g(y, 20)
// or:
arg |> x => ( f(x, 10) |> y => g(y, 20) )
// ? Both of the above constructs are useful. I've seen people make comments with an example like this: array.map( x => x |> f |> g ) ...which would either work or break depending on operator precedence. This new proposal avoids this by requiring parenthesis around arrow functions. Should the same be required in the original proposal? |
@mindeavor I don't understand the trade-off of prototype methods and curry-parens. By that I mean to say I understand why you'd want to be able to access prototype methods but not how this design meets that goal better. I'm assuming by the example you linked you're referring to: val
|> Lazy() // Lazy(val)
.map( p => p.name )
.take(5) In my mind this would work the same as: let collection = val |> Lazy
collection
.map( p => p.name )
.take(5) Or with an extra rule for newlines val
|> Lazy
.map( p => p.name )
.take(5) which would be equivalent to (val |> Lazy)
.map( p => p.name )
.take(5) and if you piped again it would be ((val |> Lazy).map(p => p.name).take(5)) |> otherFn Was that what you were referring to? In general I appreciate the balancing between to the two styles that you're trying to go for. I hope what I'm doing is at least voicing these concerns properly. |
@mindeavor good point on the arrows functions. I assumed users would need to wrap their functions in parens in order to separate the expressions in a single line. Else the first functions body would be the rest of the operations. At least that's how the syntax reads to me. |
Another -1 here.
I disagree; here let add = x => y => x + y
100 |> (add(10)) //=> add(10)(100) This similarly feels un-Javascript-y. I have no reason to expect |
@seanstrom Yup, that's what I'm referring to. One advantage to piping to the first invocation is that you can start chaining right away, and even pipe to something else afterwards without hinderance. JS isn't whitespace sensitive in general, so a newline rule would probably not be a good idea.
Can you provide an example for illustration? |
This is a fair point :) |
@mindeavor yeah I agree say that whitespace rules wouldn't be much better. val
|> Lazy(&1).map( p => p.name ).take(5)
// equivalent to
val
|> x => Lazy(x).map( p => p.name ).take(5) Which is just more of the same JS we know, but we add the sugar syntax for convenience. Again I know you're very aware of this, just explaining my line of thinking. Here's what I meant for wrapping the arrow functions: arg |> x => f(x, 10) |> y => g(y, 20)
// could be two expressions
arg |> (x => f(x, 10)) |> (y => g(y, 20))
// or one expression with piping in its definition
arg |> (x => f(x, 10) |> y => g(y, 20)) |
@seanstrom In an ideal world we would have placeholder args, but I don't think it will happen taking responses like this into consideration. I just realized my concerns associativity may be unfounded. From what I can see – and someone correct me if I'm wrong – the following two lines are actually equivalent: (arg |> x => f(x)) |> y => g(y)
arg |> x => (f(x) |> y => g(y)) |
This new proposal makes the javascript syntax ambiguous, a function call stops being a function call if next to the pipe operator. The parenthesis around the function call to bring back the normal behavior also looks like trying to solve a problem that should not be there, and adds even more ambiguity. If currying functionality is something desirable as part of the syntax it should be a separate proposal. |
@mindeavor just finished reading the critique to this whole proposal and the comment you specifically linked. I apologize for being too pushy, you've already gone through enough stress. It's moments after reading that kind of push back that makes me want to use at least SweetJS. Also, I believe the associativity you showed checks out. This of course assumes f and g are pure and stuff. I would also mention that if users didn't wrap their arrow functions, they may have done so to intentionally create a closure. |
@seanstrom It's no problem, thank you for being considerate. When you "assume f and g are pure", can you think of a case where non-pure would make a difference? Both lines of that example I wrote translate to the same AST, so I think in either case using non-pure functions would retain the same behavior. |
@mindeavor you're absolutely right about the purity not mattering. I meant that strictly for the example being associative in the law sense. But the way functions will eventually pipe to each other is equivalent as you've shown it. |
It seems the consensus is that the original proposal is much better, due to its simplicity and understandability. With regard to the original proposal, there is one more thing to discuss: operator precedence (#23) Closing now :) |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
After much mulling and consideration, I think I have managed to combine everyone's ideas to create a version of the pipeline operator that is optimal for JavaScript. In short, the rule is:
In other words,
x |> f(10, 20)
is equivalent tof(10, 20, x)
.Why not the first argument like Elixir? A couple reasons:
x |> f(10)
, I expect10
to be the first argument tof
. Elixir's style changes this tof(x, 10)
.let g = f.bind(null, 10); x |> g(20)
Syntactically, this also lets us code really useful flows that still solve the prototype extension problem. Check out the full README of the new proposal version for details.
I would love everyone's feedback on this, both negative and positive. Thanks for reading.
P.S. The pull request for this version is here.
The text was updated successfully, but these errors were encountered: