Support right precedence hack-style pipeline #11557

sosukesuzuki · 2021-09-20T12:01:39Z

Description

Hack-style pipeline operators are right-associative. babel/parser@7.15.5 treats it as left-associative, babel/parser@7.15.6 treats it as right-associative.

ref: babel/babel#13668
ref: babel/babel#13668 (comment)

Clsoes #11540

Checklist

I’ve added tests to confirm my change works.
(If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
(If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
I’ve read the contributing guidelines.

✨Try the playground for this PR✨

sosukesuzuki · 2021-09-22T14:33:08Z

tests/format/js/pipeline-operator/__snapshots__/jsfmt.spec.js.snap

+|> (await %)
+|> % || throw new Error(\`foo \${bar1}\`)
+|> bar2(%, ", ")
+|> bar3(%)
+|> % + "!"
+|> new Bar.Foo(%)
+|> (await bar.bar(%))
+|> console.log(%);


I'm not sure the best way, but following official explainer, we don't add indent on here.

It's clear for me to indent.

This can't convince me. I'd like to hear opinions from other maintainers.

Okay, I have no strong opinion for this. So for now, let's use indentation version, same as current Prettier.

fisker · 2021-09-23T02:15:41Z

Let me understand the problem here

We can't know the ast is a hack-style pipeline, even if we search for TopicReference, because TopicReference is not required in pipeline. So we have to store the plugins, right?
This ast doesn't print right because we are trying to flat the BinaryExpression. What if we don't try to flat |>? (To be honest: I don't like prettier flat my operators, even simple logical operators) We already have ** , will the output ugly?

sosukesuzuki · 2021-09-23T18:20:06Z

@fisker

We can't know the ast is a hack-style pipeline, even if we search for TopicReference, because TopicReference is not required in pipeline. So we have to store the plugins, right?

No! TopicReference is required in hack pipeline. So we can detect it in the way you said. I had no such idea.

This ast doesn't print right because we are trying to flat the BinaryExpression.

Why? Hack pipeline is right-associative operators, so it is no problem to flat it?

What if we don't try to flat |>? (To be honest: I don't like prettier flat my operators, even simple logical operators) We already have ** , will the output ugly?

When we don't flat it...

input:

const searchResults$ = fromEvent(document.querySelector('input'), 'input')
  |> map(%, event => event.target.value)
  |> filter(%, searchText => searchText.length > 2)
  |> debounce(%, 300)
  |> distinctUntilChanged(%)
  |> switchMap(%, searchText => queryApi(searchText) |> retry(%, 3))
  |> share(%);

output:

const searchResults$ =
  fromEvent(document.querySelector("input"), "input")
  |> (map(%, (event) => event.target.value)
    |> (filter(%, (searchText) => searchText.length > 2)
      |> (debounce(%, 300)
        |> (distinctUntilChanged(%)
          |> (switchMap(%, (searchText) => queryApi(searchText) |> retry(%, 3))
            |> share(%))))));

fisker · 2021-09-24T10:51:18Z

This ast doesn't print right because we are trying to flat the BinaryExpression.
Why? Hack pipeline is right-associative operators, so it is no problem to flat it?

I don't know, my guess.

When we don't flat it...

~~That's ugly, but maybe not caused by flat? If we can know it's right-associative, why not just print differently, like not add parens and intentions?~~ Never mind, your solution seems good.

Bumps [@babel/parser](https://github.com/babel/babel/tree/HEAD/packages/babel-parser) from 7.15.5 to 7.15.7. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.15.7/packages/babel-parser) --- updated-dependencies: - dependency-name: "@babel/parser" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps [@babel/parser](https://github.com/babel/babel/tree/HEAD/packages/babel-parser) from 7.15.5 to 7.15.6. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.15.6/packages/babel-parser) --- updated-dependencies: - dependency-name: "@babel/parser" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>

sosukesuzuki · 2021-09-24T18:00:32Z

src/language-js/parse/babel.js

@@ -149,7 +149,8 @@ function createParse(parseMethod, ...optionsCombinations) {
      throw createBabelParseError(error);
    }

-    return postprocess(ast, { ...opts, originalText: text });


out of scope: opts is big object, so maybe it is good that we don't copy it for other parsers too.

sosukesuzuki · 2021-09-27T08:44:57Z

@fisker Can you review again?

fisker · 2021-09-27T09:21:44Z

1 + 100 |> a(%) means a(1 + 100), not 1 + a(100)?

sosukesuzuki · 2021-09-28T23:31:46Z

1 + 100 |> a(%) means a(1 + 100)

yes.

(Babel)

Input:

1 + 100 |> a(%)

Output:

"use strict";

var _ref;

_ref = 1 + 100, a(_ref);

sosukesuzuki · 2021-09-28T23:42:03Z

tests/format/js/pipeline-operator/__snapshots__/jsfmt.spec.js.snap

+      |> (yield %)
+      |> (await %)
+      |> y(%)
+      |> a.b(%)
+      |> a.b(%)
+      |> a.b(%)
+      |> a.b?.(%)
+      |> a.b?.(%)


@fisker I've updated snapshots with indentation version (#11557 (comment)). What do you think this change? ( After seeing this change, I personally reconsidered and decided that it would be better to be consistent without indentation. )

sosukesuzuki · 2021-10-02T02:12:14Z

I don't like that this PR is blocker to update babel/parser. So I'll merge this with the unindent version for now. Let's discuss in another issue. Since this syntax is not used by most users, there is no pain to change the behavior.

sosukesuzuki force-pushed the support-right-preceding-hack branch 2 times, most recently from b0184cd to 9fa6e27 Compare September 22, 2021 14:30

sosukesuzuki commented Sep 22, 2021

View reviewed changes

sosukesuzuki changed the base branch from main to dependabot/npm_and_yarn/babel/parser-7.15.7 September 22, 2021 16:19

sosukesuzuki changed the base branch from dependabot/npm_and_yarn/babel/parser-7.15.7 to main September 22, 2021 16:21

sosukesuzuki force-pushed the support-right-preceding-hack branch from d86a003 to 38e5972 Compare September 22, 2021 16:21

sosukesuzuki marked this pull request as ready for review September 22, 2021 16:55

sosukesuzuki requested a review from fisker September 22, 2021 16:55

dependabot bot and others added 14 commits September 25, 2021 02:09

tmp

ebc7bfa

paren

a914db2

tmp

4c8b9ef

Return parts simply

50a36d2

Recursive for hack pipe

776fa55

Update tests

2af2250

Revert shouldFlatten

16789d2

Revert needs-parens

482957d

Fix

69431f8

Fix lint problems

8cf47d6

Update error message

6ac889a

Fix isEnabledHackPipeline logic

634138a

sosukesuzuki force-pushed the support-right-preceding-hack branch from 38e5972 to 634138a Compare September 24, 2021 17:25

sosukesuzuki commented Sep 24, 2021

View reviewed changes

sosukesuzuki commented Sep 28, 2021

View reviewed changes

sosukesuzuki force-pushed the support-right-preceding-hack branch from e441842 to 634138a Compare October 2, 2021 02:03

sosukesuzuki merged commit fb7451a into prettier:main Oct 2, 2021

sosukesuzuki deleted the support-right-preceding-hack branch October 2, 2021 02:34

sosukesuzuki mentioned this pull request Oct 2, 2021

Remove unnecessary copying options for parsers #11602

Merged

1 task

github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support right precedence hack-style pipeline #11557

Support right precedence hack-style pipeline #11557

sosukesuzuki commented Sep 20, 2021 •

edited

sosukesuzuki Sep 22, 2021

fisker Sep 24, 2021

sosukesuzuki Sep 24, 2021 •

edited

fisker Sep 27, 2021

sosukesuzuki Sep 28, 2021

fisker commented Sep 23, 2021 •

edited

sosukesuzuki commented Sep 23, 2021

fisker commented Sep 24, 2021 •

edited

sosukesuzuki Sep 24, 2021

sosukesuzuki commented Sep 27, 2021

fisker commented Sep 27, 2021

sosukesuzuki commented Sep 28, 2021

sosukesuzuki Sep 28, 2021 •

edited

sosukesuzuki commented Oct 2, 2021

Support right precedence hack-style pipeline #11557

Support right precedence hack-style pipeline #11557

Conversation

sosukesuzuki commented Sep 20, 2021 • edited

Description

Checklist

sosukesuzuki Sep 22, 2021

Choose a reason for hiding this comment

fisker Sep 24, 2021

Choose a reason for hiding this comment

sosukesuzuki Sep 24, 2021 • edited

Choose a reason for hiding this comment

Elixir (by mix format)

F# (by Fantomas)

fisker Sep 27, 2021

Choose a reason for hiding this comment

sosukesuzuki Sep 28, 2021

Choose a reason for hiding this comment

fisker commented Sep 23, 2021 • edited

sosukesuzuki commented Sep 23, 2021

fisker commented Sep 24, 2021 • edited

sosukesuzuki Sep 24, 2021

Choose a reason for hiding this comment

sosukesuzuki commented Sep 27, 2021

fisker commented Sep 27, 2021

sosukesuzuki commented Sep 28, 2021

sosukesuzuki Sep 28, 2021 • edited

Choose a reason for hiding this comment

sosukesuzuki commented Oct 2, 2021

sosukesuzuki commented Sep 20, 2021 •

edited

sosukesuzuki Sep 24, 2021 •

edited

Elixir (by `mix format`)

fisker commented Sep 23, 2021 •

edited

fisker commented Sep 24, 2021 •

edited

sosukesuzuki Sep 28, 2021 •

edited