-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Add support for Hack(#) -style Pipeline Operator #43617
base: main
Are you sure you want to change the base?
Conversation
@typescript-bot pack this |
Hey @sandersn, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build and an npm module you can use via |
Personal opinion: The hack-style pipeline operator is bad because it introduces a new context variable, like 'Pipelines!'
|> #.length
|> Math.pow(#, 2)
|> console.log(#); // 100 as var $$;
($$ = 'Pipelines!',
$$ = $$.length,
$$ = Math.pow($$, 2),
$$ = console.log($$)) // 100 without loss of functionality with very little comparable overhead. (If you use a 1 character variable name, it's a fixed 5 character overhead plus 2 characters per pipeline phase (extra whitespace and comma). The functional pipeline operator, on the other hand, exists to deliver a point-free style that otherwise is impossible in JS - while some people may hate this style, that is what the true functional pipeline operator truly enables. (Plus, in the common functional style, each pipeline phase saves at least 4 characters, which is a much greater savings than the hack style). Moreover, the functionality of the "hack style" is replicatable (without implicit variables) using the functional style by embedding arrow functions into the pipeline, at a cost of 3-5 (depending on whitespace preferences) characters per pipeline phase which required adapted arguments. (Which shouldn't be too often, as if you find yourself adapting the same function's argument's repeatedly, in the case of either operator form, you should probably wrap it with an adapted form for clarity. For example, if Thanks for coming to my rant. I'm obviously not on the TC39, so my opinion will have absolutely no bearing on which proposal advances. |
Update: I've been refactoring the TS PRs for pipelines (both F# #38305 and hack #43617 ) during this week and the commits are a lot cleaner now. The current commit history for hack-style pipelines is just a mess since it builds upon the F# implementation. The newer one will use a common commit. One big idea I came upon is to have different PipelineExpression types for both a method that itself calls a unary function (F#) and another (hack) that just replaces tokens with the "argument". After all, the way those two pipelines work is different despite the similarities. This will allow for easier support of both F#, hack and even smart pipeline styles at the same time. I believe that whichever style of F# or hack pipelines will be the default, the next shop would be to add another pipelineish operator for the other one. This is relatively straightforward to implement in TypeScript, so I'm considering adding Tacit Unary Function Application syntax for this Hack-style PR to enable I've been rebasing my own repos against master, but I still haven't had time to fix the |
055dbe5
to
e1c9d1f
Compare
@typescript-bot pack this |
Hey @orta, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build and an npm module you can use via |
After giving this PR a small trial run, I noticed a pretty big roadblock toward your adoption goal. The little bits of the feature I could try felt natural and worked correctly in a sandbox but, to get a proper feel for it, I think I would really need to use it in an actual project. Unfortunately, while I would have a great VueJs candidate to test this on, I had to give up on eslint entirely for it to work. With linting enabled in the project, vue cli basically explodes during compilation. For me, until we can get some experimental eslint support for TS 4.3 and the pipeline operator syntax itself (via @typescript-eslint/parser), I don't think there will be much adoption outside of sandbox testing. There is just too much value in a tool like eslint to warrant ditching it to test a proposal feature in a real world scenario. I will definitely be back on board the second a version that doesn't crash with this PR gets published, even if it means it only ignores everything in pipelines. |
I've been using this PR for a little while on a Discord bot refactor/TypeScript migration project that will be open-source once completed. I greatly enjoy using it to avoid nesting parentheses or avoid declaring variables where function names already communicate intent. I miss it immediately when going back to company code. Here's a file using small pipelines from my project, it's a Discord message event handler. There're pipelines on lines 30, 42 and 47. It's my first project using TypeScript, so there might be weird stuff. 😖 I've been mostly using the Minor feedback on the implementation: the non-null assertion |
Now at Stage 2 |
@Lonli-Lokli The tweet makes it sound like the proposal already advanced but I don't think it did yet, it was "just" presented today and seeking stage 2 (which is big news too of course). I'm making assumptions based on very little data here but for example "BigInt Math" was scheduled to be presented after pipelines and its advancement has already been updated, unlike pipeline's. Edit: I guess I was wrong 🙃 tc39/proposals@f410ced |
Hey everyone. Let's not rush this. |
@Pokute It looks like there is a transform bug: var pipelineHackPlaceholder_1, pipelineHackPlaceholder_2;
(pipelineHackPlaceholder_1 = 1, ((pipelineHackPlaceholder_2 = pipelineHackPlaceholder_1, pipelineHackPlaceholder_2))); and not as var pipelineHackPlaceholder_1, pipelineHackPlaceholder_2;
(pipelineHackPlaceholder_1 = 1, ((pipelineHackPlaceholder_2 = #, pipelineHackPlaceholder_2))); |
I don't think posting Hack vs. F# arguments to these TypeScript pull requests has any productive outcome. There's no good reason why we should fragment the that discussion further by having parallel discussions here, where vast majority of people invested into pipeline operator will not see it. These kinds of comments, or listing community advocates for one proposal or another have no impact whatsoever on whether this specific implementation of pipeline operator for TypeScript is correct or not. |
I made an issue: |
@stken2050 can you please stop spamming this and the F#-style PR threads? |
@micheal-hill Thanks for your insight, and I've deleted the previous 4 comments by myself. |
Can we make this PR up to date with 4.4? The spec is settled, so this PR could move forward. |
I have a new version updated against new main ready, that fixes bug reported by nicolo-ribaudo and a couple of other bugs too. Unfortunately there's no good point in pushing it currently as the master is broken. I'll push it when #46930 is fixed. |
Many thanks. Can't wait to test it out! |
e1c9d1f
to
a0fdb2a
Compare
@typescript-bot pack this |
Hey @orta, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build and an npm module you can use via |
Can this be packed with [email protected] ? |
Ergonomically, that would require force pushing a version that's rebased on an older version. That would be practically undoing some work and it the repo would most probaly not be mergeable to current main branch. It's likely that packing is not possible unless I change the target branch to the 4.5 tag and for that I would have to create a new PR etc. A ton of work and more noise. @VictorGaiva, what are the reasons you want one for version 4.5 instead of the current main branch? I don't quite understand the reasonings and with them I might be able to help more. |
|
I don't think that there are established hack vs. F# styles yet. The pipeline's main attraction is normalising the direction of data flow which is present in your code. Only through long-term experimentation can we build some best-practices.
Thank you! I appreciate testing pipelines out and the feedback!
Interesting. Do extensions and |
Experiment PR adding Hack-style pipeline operator support to TypeScript. The Pipeline proposal is currently at stage 1 for EcmaScript. Pipeline operator proposal "introduces a new operator |> similar to F#, OCaml, Elixir, Elm, Julia, Hack, and LiveScript, as well as UNIX pipes and Haskell's &. It's a backwards-compatible way of streamlining chained function calls in a readable, functional manner, and provides a practical alternative to extending built-in prototypes."
There's additional Tacit Unary Function Application support through the |>> -operator which behaves exactly like minimal pipelines implementation taking the pipeline input as a sole parameter for the function.
There's also a PR for minimal implementation pipeline operator.
Code status
This works, but might have some missing features. The TS transformer generates valid JS and there's working type inference. There might be some inconsistencies with specs, insufficient error messages and there's almost no tests. Most of failing tests are not included in this PR since Playground won't get generated with any failures and having faulty baselines is shady. I'm eagerly awaiting feedback, testing and even help with this PR. Even using and talking of these features will help advance them through TC39 proposal process!
Testing
Playground
To test it in your project, add following to your package.json:
and then run
npm install
.PR state
I used @rbuckton 's work on pipelineStage1 branch as a guide.
Backlog
milestone (required)master
branchgulp runtests
locallyWhy a draft PR for stage 1 proposal?
The TypeScript officially follows EcmaScript / stage 3 proposals. However, I'm opening this draft pull request despite it being only stage 1 so that we get a functional TypeScript environment to gather feedback from developers without having to build typescript itself. Having a playground and npm package will encourage more people to try this fun new feature out and will raise awareness.
Of course, I'm not expecting merging until the proposal reaches stage 3.