Skip to content
This repository was archived by the owner on May 19, 2018. It is now read-only.

Pipeline Operator proposal#742

Merged
hzoo merged 1 commit into
masterfrom
pipeline
Sep 29, 2017
Merged

Pipeline Operator proposal#742
hzoo merged 1 commit into
masterfrom
pipeline

Conversation

@hzoo

@hzoo hzoo commented Sep 28, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Ref babel/proposals#29
Taken from babel/babel#3159 by @gilbert
Spec: https://github.com/tc39/proposal-pipeline-operator/pull/51/files

need more tests

  • precedence
  • with arrow function 10 |> x => x + 1;
  • chaining a |> b |> c
  • multline
let result = "hello"
  |> doubleSay
  |> capitalize
  |> exclaim;
let newScore = person.score
  |> double
  |> _ => add(7, _)
  |> _ => boundScore(0, 100, _);

@hzoo hzoo changed the title pipelineOperator proposal Pipeline Operator proposal Sep 28, 2017
@hzoo
hzoo requested a review from littledan September 28, 2017 13:02
@hzoo hzoo mentioned this pull request Sep 28, 2017
Open
2 tasks
jridgewell
jridgewell suggested changes Sep 28, 2017
View reviewed changes
Comment thread src/tokenizer/index.js Outdated
if (code === 124) {
// '|>'
if (next === 62) {
this.expectPlugin("pipelineOperator");

@jridgewell jridgewell Sep 28, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we usually expect in the not-tokenizer part?

Comment thread src/tokenizer/index.js
@@ -411,9 +411,17 @@ export default class Tokenizer extends LocationParser {
const next = this.input.charCodeAt(this.state.pos + 1);

@jridgewell jridgewell Sep 28, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename the function.

@hzoo hzoo Sep 28, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is ok now

Comment thread test/fixtures/experimental/pipeline-operator/base/actual.js
@@ -0,0 +1 @@
a |> b

@jridgewell jridgewell Sep 28, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More examples, especially arrows.

@hzoo hzoo Sep 28, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@hzoo
hzoo merged commit d2e06a9 into master Sep 29, 2017
@jridgewell
jridgewell deleted the pipeline branch September 29, 2017 03:25
littledan
littledan reviewed Sep 30, 2017
View reviewed changes
Comment thread src/tokenizer/types.js
this.prefix = !!conf.prefix;
this.postfix = !!conf.postfix;
this.binop = conf.binop || null;
this.binop = conf.binop === 0 ? 0 : conf.binop || null;

@littledan littledan Sep 30, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be nice with ?? :)

@hzoo hzoo Sep 30, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, should be easy to add/use here (would be good to find this kind of code on github/codebase and then do a codemod)

Comment thread src/parser/expression.js
if (node.operator === "|>") {
this.expectPlugin("pipelineOperator");
// Support syntax such as 10 |> x => x + 1
this.state.potentialArrowAt = startPos;

@littledan littledan Sep 30, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the current draft spec has this logic. I couldn't figure out how to put it into the grammar. Right now, x |> y => z would be a syntax error. But it's clearly useful--i'll think more about the spec.

@hzoo hzoo Sep 30, 2017 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know tc39/proposal-pipeline-operator#60

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@littledan littledan littledan left review comments
@jridgewell jridgewell jridgewell approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Spec: Pipeline Operator Tag: New Feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hzoo @jridgewell @littledan