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
Implement Smart Pipeline proposal in @babel/parser #8289
Implement Smart Pipeline proposal in @babel/parser #8289
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/9434/ |
Could you move it to a separate PR? Just in case this one doesn't get merged before the Babel 7 RC because it is a huge PR. |
@nicolo-ribaudo Done! #8291 |
12da3b9
to
fbf62b4
Compare
Refreshed from #8291, so this is ready to review again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I haven’t fully reviewed the PR yet, I didn't mean to approve 😅)
Question: are |
Thanks for the review! As for the question
No; const _temp = await foo;
_temp(x); in every proposal. Smart pipelines enables |
If "smart" proposal is, we'll no longer say.
It's only used once.
Just pushed fixes or responses for everything except fixing the Pipeline types, which I have to dig into. |
Set whether we're in a pipeline in order to determine how to parse the hash. The error message changes as a result, since the `hash` never enters the block.
This ensures we clean up always if the callback throws.
This doesn't work because the `type` values are not compatible.
baa971f
to
39e7ee6
Compare
I addressed the comments so far, so this is ready for review again. |
No need to recheck it throughout then.
@xtuc Updated as per comments, responded to some of your comments inline. Made a note to update the spec.md. |
e6465ef
to
70318c9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from some minor comments, this PR looks good to me!
}; | ||
|
||
export type PipelineStyle = | ||
| "PipelineBareFunction" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the only non-node export of this file? I'm not sure that this is the right place where it should be put, but also I can't think of a better place 😕
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, the file is called types.js
so maybe it's ok?
It's not going to be anything else...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 awesome work!
@mAAdhaTTah: 🍾🍾🍾 |
Thanks so much! I think @xtuc is the last holdout. Any remaining changes you need? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LET'S MERGE THIS 🎉 💃
Also thanks for your contribution and sorry if that took so long on our side (or at least mine)! |
That's awesome, y'all! I can now get the PR for the transform going. |
Thanks everyone for the hard work with the update and review! |
Most of this work was implemented by the illustrious @js-choi, which some adjustments and refreshments from myself to use the new plugin options API. He's been fairly busy so I'm going to be working through the suggestions / changes here.
This is a big PR, but it enables
@babel/parser
to parse the Smart Pipeline proposal's topic reference. As discussed in babel/proposals#29, this is the first step towards enabling the pipeline plugin to parse and emit all three proposals.Next, I'll be implementing F#-style parsing, then we'll work on the pipeline plugin itself. I want to get them in piece by piece to avoid merge conflicts. This work had drifted quit a bit, and took me a few days to refresh @js-choi's work with the latest from
master
, so I'd like to land each piece one by one.This is a pretty major chunk of work, so definitely interested in getting as much feedback as possible.
🎉One step closer to the landing the pipeline proposals! 🎉
cc @littledan