Skip to content
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

Normative: Permit arrow functions in pipeline #70

Closed
wants to merge 3 commits into from

Conversation

littledan
Copy link
Member

Putting AssignmentExpression in the right side of a pipeline would
create ambiguities; this PR just permits arrow functions specifically.

Addresses #60 . cc @jridgewell

Putting AssignmentExpression in the right side of a pipeline would
create ambiguities; this PR just permits arrow functions specifically.
@dallonf
Copy link

dallonf commented Nov 10, 2017

Out of curiosity, would classic ES5-style anonymous functions work in the same way?

Another question: does this combine with #71 to allow |> await async _ => {/*some aysync stuff*/} ?

@littledan
Copy link
Member Author

Out of curiosity, would classic ES5-style anonymous functions work in the same way?

That should already work with the current grammar, and it should keep working with this PR. FunctionExpression is part of PrimaryExpression, which you can get to from LogicalORExpression.

Another question: does this combine with #71 to allow |> await async _ => {/some aysync stuff/} ?

There's a little bit of text that would need to be added at the intersection, but it should work out just fine, I think. I'm not sure if it's the cleanest style, though...

@littledan
Copy link
Member Author

littledan commented Dec 16, 2017

We discussed this proposal at TC39, but several committee members said they needed some more time to think about whether the grammar was reasonable enough. I'm interested in collecting more feedback here if possible; cc @tschneidereit @ajklein @msaboff @bterlson @allenwb @anba @waldemarhorwat

@michaelficarra
Copy link
Member

The body of an arrow is an AssignmentExpression, meaning this will lead to an ambiguous parse. See a |> b => c |> d, which may be either (a |> (b => c)) |> d or a |> (b => (c |> d)).

@littledan
Copy link
Member Author

@michaelficarra Good point! At a high level, this is not all that ambiguous since the two versions should have mostly the same semantics (except for some edge cases with leaking a var declaration out of an eval; there may be other things but I haven't thought of them). For that reason, I was OK with either parse. I was picturing the second parse, which is what I believe the Babel implementation would take. Do you think there's any tweaks we could make to the grammar to make it unambiguous? cc @anba

@charmander
Copy link

charmander commented Jan 10, 2018

a |> (b => c) |> b and a |> (b => c |> b) are very different. It’d be nice to require parentheses.

@littledan
Copy link
Member Author

@charmander What about the difference are you concerned about?

@charmander
Copy link

That it’s not obvious at first glance which one a |> b => c |> b means and that writing a |> (b => c) |> b isn’t painful enough to require any exceptions allowing its parentheses to be dropped.

@gilbert
Copy link
Collaborator

gilbert commented Jan 10, 2018

@charmander Can you find a case where the two parses are semantically different? I'm having trouble finding one myself. For example, these two are equivalent:

// Original
10 |> x => double(x) |> y => inc(y) |> double

// Parse Option 1
10 |> (x => double(x)) |> (y => inc(y)) |> double
//=> 42

// Parse Option 2
10 |> x => (double(x) |> y => (inc(y) |> double))
//=> 42

@michaelficarra
Copy link
Member

@gilbert @littledan Scope.

a |> b => c |> b

Is that second b a reference error? Depends on how it's parsed.

Regardless, I would be opposed to introducing a grammar ambiguity unless it was in somewhere entirely inconsequential like a cover grammar or a grammar only used for the purposes of rejecting a program.

@littledan
Copy link
Member Author

Oh! Somehow I was thinking about leaked vars and didn't even think about the binding introduced by the arrow itself.

Anyway, I agree that we shouldn't introduce an ambiguity. Assuming we want to resolve it by taking the longer arrow function body interpretation, does anyone here have an idea of how to phrase the grammar?

@zenparsing
Copy link
Member

zenparsing commented Jan 22, 2018

I would find both of these parsing options highly surprising.

The longer arrow function would be surprising because the intention of this shortcut is to make small arrows inside of the pipeline more ergonomic. If I go from:

val
  |> (_ => _.x())
  |> (_ => y(_, 1));

to

val
  |> _ => _.x()
  |> _ => y(_, 1);

in order to make things look nicer, I would be extremely surprised that this resulted in a different parse tree.

On the other hand, it would also be surprising to take the shorter arrow function, since that would break my normal precedence expectations with respect to arrow functions.

For these reasons, I don't think we can go forward with permitting arrows without parentheses.

@mAAdhaTTah
Copy link
Collaborator

Given that this conclusion has an impact on the conversation in #84, I'd like to ask if we have a conclusion as to how arrow functions are going to... function in the pipeline. Is the intention to just have arrows be a SyntaxError, e.g.:

1 |> y => y + 1

would be invalid?

@littledan
Copy link
Member Author

Without this patch, this is a SyntaxError.

I still can't think of a solution to the ambiguity issue. It seems to me that if we're likely to need some form of pipeline placeholders, then this arrow function syntax is also not as necessary.

@mAAdhaTTah
Copy link
Collaborator

Reopening this conversation in light of what I'm pushing for in #104:


@zenparsing:

On the other hand, it would also be surprising to take the shorter arrow function, since that would break my normal precedence expectations with respect to arrow functions.

Do we have expectations as to the precedence between arrow & the pipeline at this point? The shorthand rule expressed in #104 is "pipeline operator terminates an arrow function"; is that a strong enough shorthand to override whatever default expectation we may have?


@charmander:

...and that writing a |> (b => c) |> b isn’t painful enough to require any exceptions allowing its parentheses to be dropped.

IIRC, concerns have been raised about the ergonomics of paren'd arrow functions, specifically by @gilbert, as he's a proponent of a placeholders-based approach. I think arrow functions would have to step in to fill the gaps until a partial application proposal gets accepted, so I'd prefer to optimize its ergonomics as much as possible.


@michaelficarra: Just so I'm clear, would the proposal in #104 clear up the grammar ambiguity you reference? I believe so, but just covering all my bases.


Thanks everyone!

@charmander
Copy link

@mAAdhaTTah The parentheses required around arrow functions seem straightforward enough. They don’t get in the way. I think making pipeline operator expressions convenient to use as the body of an arrow function (without parentheses split across multiple lines) and without introducing strange precedence rules and surprising behaviour (when someone tries to use x => x |> f |> g to mean x => (x |> f |> g), compared to x |> x => y |> f being an easier to diagnose syntax error) is more worthwhile.

@js-choi js-choi deleted the arrow-pipeline branch September 14, 2021 22:00
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants