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

Should it be pipeline-expression with more operators [ "|>" | ":|>" | "?|>" | "?:|>" ] #210

Closed
muhammad-salem opened this issue Sep 14, 2021 · 22 comments

Comments

@muhammad-salem
Copy link

can we imagine that the ast of the new pipeline expression will be as

interface PipelineExpression <: Expression {
    type: "PipelineExpression";
    argument: Expression;
    pipes: [ PipeClause ]
}

interface PipeClause {
    operator: PipelineOperator;
    body: PipeBody | FunctionExpression;
}

enum PipelineOperator {
    "|>" | ":|>" | "?|>" | "?:|>"
}

this will give us the control to

  • add new operator
  • control execution of the pipeline step by step

:|> Bind Pipeline

  • mean that bind the FunctionExpression to the return value ^ (applied also on the first argument) and execute the body.
x :|> y
// equivalent to
y.apply(x, [])
`PipeBody had no 'this' it is like arrow function, will steal it from the nearest scope`

?|> Optional chaining Pipeline

  • mean that if the value of ^ is un defined or null, will stop execution the rest of PipeClause
    otherwise will continue the execution.
x |> ^  === 10 ? true : undefined
  ?|> y
  |> z;

// equivalent to
^ = x === 10 ? true : undefined;
if(^ === undefined || ^ === null){
	return ^;
}
^ = y(^);
return z(^);

?:|> Optional chaining Bind Pipeline

  • mean that cheek ^ if not nullish, bind it as this to the FunctionExpression.
x |> ^ * 10 
  ?:|> y;
// equivalent to
^ = x * 10;
if(^ === undefined || ^ === null){
	return void 0;
}
return y.apply(^, []);

** and could add more operators if needed.

@ljharb
Copy link
Member

ljharb commented Sep 14, 2021

More than one pipeline operator seems like it'd be very confusing.

@jonrimmer
Copy link

Some kind of short-circuiting operator is surely necessary at least. Otherwise aren't be back to the pre-optional-chaining bad old days, having to bog everything down with null/undefined checks, or otherwise end up with unsafe code?

And If there isn't going to be additional operators, why have an operator at all? Wouldn't it be less syntactically noisy to use some kind of pipe statement, where ^ refers to the last completion value?

do.pipe { 
  f();
  ^ + 1;
  console.log(^);
}

@tabatkins
Copy link
Collaborator

#159 already covers the possibility of an optional-pipe operator; let's leave discussion of that to that issue.

So that leaves this issue to talk about a bind-pipe operator. I lean against it, for two reasons.

  • as @ljharb says, multiple operators that are close in spelling and in meaning can be confusing. (The optional-pipe variant at least plays on the existing idiom of optional chaining/calling.)
  • Previous proposals for a bind operator have had multiple binding-related uses, such as being able to extract a method into a variable and retain the bindings automatically. That would be lost if we folded the functionality into pipe.

I'd be comfortable exploring this idea in the bind-operator repo as an option for spelling this particular binding use-case, but I don't want to push that proposal towards a particular solution ahead of time. No reason to have to decide this quickly; we can let pipeline mature in its own form and let binding decide to lean on this syntax or not in its own time.

@js-choi
Copy link
Collaborator

js-choi commented Sep 14, 2021

I'd be comfortable exploring this idea in the bind-operator repo as an option for spelling this particular binding use-case, but I don't want to push that proposal towards a particular solution ahead of time. No reason to have to decide this quickly; we can let pipeline mature in its own form and let binding decide to lean on this syntax or not in its own time.

(For what it’s worth, I am exploring presenting a resurrected bind-operator proposal to TC39 at the next plenary meeting. I’ve been discussing with @hax on how it would work with his Extensions proposal. It’s in flux.)

@muhammad-salem
Copy link
Author

muhammad-salem commented Sep 14, 2021

  • first of all, this is a suggestion to the estree structure of the Pipeline Expression.

  • this proposal is only for pipeline |> operator, let any other operator can has its own proposal.

  • the idea here is to wrapping this operator in a new expression, that have multi (or just one |> for now ) operators and can handle setting the value of placeholder ^ for the next pipe.

  • the need to break (stop executing ) the pipeline is still exist, the Optional chaining Pipeline ?|> operator is one of the ideas that can do that, it can rise a break signal to the Pipeline Expression to stop executing at this step.

  • in your proposal, the operator only know about the argument and the PipeBody, can't handle a break signal from the previous pipeline operator. the next pipe is know nothing about if it should be canceled or continue (continue is default), as simple we need the break statement of for loop to be exists in pipeline expression too.

enum PipelineOperator {
   "|>"
}

@tabatkins
Copy link
Collaborator

I apologize, but I don't understand what any of that comment is about.

@muhammad-salem
Copy link
Author

simply what is the effect of break statement on pipeline operator

x |> y
  ........
  |> if (^ == undefined) break; else return ^;
  .......
  • Not allow
  • we can have it

in case of not allow ==> we can ignore this issue

in case of it is a good thing and we can plane to have it

the current proposal AST i assume it can be represented as

interface PipelineOperator <: Expression {
    type: "PipelineOperator";
    left: Expression;
    right: Expression; // <<=== the pipe body
}

or in the best case

interface PipelineExpression <: Expression {
    type: "PipelineExpression";
    operator: "|>";
    left: Expression;
    right: Expression; // <<=== the pipe body
}

example

x |> y
  |> map
  |> filter
  |> breakExecutionOnCondition // if (^ == undefined) break; else return ^;
  |> collect
  • the proposal ast for that will be:
{
	type: "PipelineOperator",
	left: {
		type: "PipelineOperator",
		left: {
			type: "PipelineOperator",
			left: {
				type: "PipelineOperator",
				left: {
					type: "PipelineOperator",
					left: x,
					right: y
				},
				right: map
			},
			right: filter
		},
		right: breakExecutionOnCondition
	},
	right: collect
}
  • the new issue ast will be:
{
	type: "PipelineExpression";
	argument: x;
	pipes: [
		{
			operator: '|>',
			body: map
		},
		{
			operator: '|>',
			body: filter
		},
		{
			operator: '|>',
			body: breakExecutionOnCondition
		},
		{
			operator: '|>',
			body: collect
		}
	]
}

the break point:

  • in case of the current proposal, only throw new Error('') can break Execution of a pipeline, and need to be handled with try-catch block

  • in case of the current issue, the breakExecutionOnCondition can rise a break signal to the PipelineExpression, then the PipelineExpression will not execute the next pipe collect and return the last value of ^.

@ljharb
Copy link
Member

ljharb commented Sep 15, 2021

@salemebo i'm also not sure what you're talking about; JS doesn't have an AST in the spec. break is a statement, not an expression, so it wouldn't work inside a pipeline unless there were do expressions - at which point, do expressions would govern the semantics, not pipeline.

@muhammad-salem
Copy link
Author

@ljharb yes, that is the point,

can we have a Pipeline Expression that can be considered as for and while statements that can handle break; statement.

@ljharb
Copy link
Member

ljharb commented Sep 15, 2021

ok so you're asking for break to work inside a pipeline (in addition to expressions) - to do what? The pipeline has to produce a value, even if it's short-circuited - what would it produce if you break-ed from it?

@muhammad-salem
Copy link
Author

the last value of placeholder ^

@ljharb
Copy link
Member

ljharb commented Sep 15, 2021

then this proposal would have to answer all the same questions do expressions will.

Why is, say, this:

a |> b(^) |> ^ ? break : c()

better than:

a |> b(^) |> ^ ? do { break } : c()

which would not require any special-casing of the break statement or pipeline?

@muhammad-salem
Copy link
Author

because the current proposal can't handle a break statement.

so in future if do expression accepted, the pipeline can't have a do expression with a break

@tabatkins
Copy link
Collaborator

I'm not sure why you think it couldn't. Function calls can't "handle a break statement" in their argument list either, but that doesn't mean you can't break inside of a do-expr in an arglist.

@muhammad-salem
Copy link
Author

I'm not sure why you think it couldn't. Function calls can't "handle a break statement" in their argument list either,

in this scenario, the break appear in the PipeBody expression, never had been in argument

but that doesn't mean you can't break inside of a do-expr in an arglist.

so, if we can break inside of a do-expr in an arglist, how the pipe line will handle that ??

x |> y
   |> z
   |> typeof ^ == 'string' ? do {break} : ^
   |> map <<== how function `map` will handle a break, or the pipeline that contain `map` as body handle `break` statement
   |> a
   |> b
   |> c

@ljharb
Copy link
Member

ljharb commented Sep 15, 2021

@salemebo it would break out of the loop, and the pipeline would never resolve to any value at all. In your case, the last 4 pipeline pieces would never execute.

@mmis1000
Copy link

mmis1000 commented Sep 16, 2021

I don't think the do proposal will help about the optional chaining.

  1. You need a block so you can break

So you would at least write it like

{
  const a = 
    x 
    |> y(^)
    |> y(^)
    |> do { break }
}

But it still won't work....

  1. Because the variable is in the wrong scope now.

And even it is in the same scope.

  1. The variable is not initiated properly and trying to access it will result in a ReferenceError

So a working one will looks like...

let a = null
{
  a =  x 
    |> y(^)
    |> y(^)
    |> do { break }
}

that looks a bit insane in my opinion.

@ljharb
Copy link
Member

ljharb commented Sep 16, 2021

@mmis1000 pretty sure you can’t break out of a block like that anyways.

@js-choi
Copy link
Collaborator

js-choi commented Sep 16, 2021

Yes, my apologies—I am quite confused by this issue in general and by some of the comments. What does break have to do with the pipe operator? break acts on for blocks, switch blocks, and labeled blocks. break has little to do with the pipe operator.

Are we talking merely about short-circuiting a chain of pipes, like with optional chaining ?.? (If you’re interested in that, there’s already a proposed |?> operator that would do that. See also #198.)

@mmis1000
Copy link

mmis1000 commented Sep 16, 2021

@mmis1000 pretty sure you can’t break out of a block like that anyways.

yes, it must be labeled, so the code will be actually even longer.

fail: {
  break fail;
  console.log('you never see me')
}

This already works today, but I doubt do anyone actually write like this?

@js-choi
Copy link
Collaborator

js-choi commented Sep 16, 2021

BLOCK: {
  a =  x 
  |> y(^)
  |> y(^)
  |> do { break BLOCK };
}

Note that this code will not compile. The last pipe expression is missing a topic. There must be a topic in every pipe step. You would want something like:

BLOCK: {
  a =  x 
  |> y(^)
  |> y(^)
  |> do { if (something) break BLOCK; else ^; };
}

But…I think this probably is off-topic from this issue. (Though I’m not really sure what the topic of this issue was in the first place…) If you have any more questions about how break works in do expressions in pipes, then please feel free to open a new issue about that. 😄

@tabatkins
Copy link
Collaborator

Yes, I think this issue can be closed now. The subtopic about an optional-chaining pipe is already being discussed in #198; the subtopic about a this-based pipe variant can be rejected at this point in the proposal (any piping variants will be addressed in followup proposals); and the subtopic about break is either about optional-chaining or unrelated to piping at all.

If there are any details in this thread that anyone still feels need to be addressed, please open a fresh issue focused on each.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants