Break lines before binary operators by btmills · Pull Request #7111 · prettier/prettier

btmills · 2019-12-09T05:43:01Z

On Friday, @evilebottnawi requested a PR to resolve #3806 in v2.0 by breaking lines, where necessary, before binary operators rather than after them, putting the operators at the beginning of each line.

~~Previously, there was special case logic to break lines before pipe operators but after all other operators. I removed the special case handling so that line breaks occur before all operators.~~

I left the implementation, tests, and changelog as separate commits to help ease review. Happy to squash or make any other changes as necessary to help get this in!

fixes #3806

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/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
I’ve read the contributing guidelines.

✨Try the playground for this PR✨

alexander-akait · 2019-12-09T09:43:41Z

/cc @prettier/core will be great if you look at this

thorn0 · 2019-12-09T09:51:22Z

I thought there was a consensus that 2.0 would be focused on deprecating old Node versions etc. Updating to 2.0 should not lead to massive code reformatting. Otherwise too many people might prefer to keep using 1.x.

GrimzEcho · 2019-12-09T16:38:57Z

The main 2.0 thread(s) have mainly talked about Node versions, but there have also been many comments on not introducing changes that there is disagreement or lack of consensus on.

But there seems to be a very strong consensus on the proposed fix to #3806. The only argument offered against the change is that it would be breaking and shouldn't be implemented in a minor version. However, for the change itself, there is widespread agreement both in the issue thread and in the JavaScript community.

AirBnB Issue: airbnb/javascript#1281
AirBnB Style Guidelines: https://github.com/airbnb/javascript#comparison--nested-ternaries

StandardJS (based on ESLint, widely used): https://standardjs.com/rules.html

thorn0 · 2019-12-10T00:13:25Z

@BrianHVB You completely miss the point of why the ex-2.0 was renamed to 3.0. I'm not saying that there is no consensus about this change, only that first things first.

changelog_unreleased/javascript/pr-7111.md

fisker · 2020-01-31T16:01:54Z

Anyone considered break line both before and after?

const isEventTarget = value != null
  && 
    typeof value.addEventListener === "function"
  && 
    typeof value.removeEventListener === "function"
  && 
    typeof value.dispatchEvent === "function"
  && 
    typeof value.dispatchEvent === "function"
  -
    Also_Nice_With_A_Shorter_Operator_Inside
  ?
    SOME_OTHER_EXPRESSION
  ??
    SOME_OTHER_EXPRESSION
  instanceof
    Even_Longer_Operator

btmills · 2020-02-03T06:02:20Z

I've rebased this change on next to resolve merge conflicts, added tests verifying that this change only moves binary operators to the beginning of lines while leaving assignment operators on their previous lines, and corrected the changelog in 300b900.

VincentLanglet · 2020-02-29T12:02:46Z

@btmills There is still a conflict

phaux · 2020-02-29T14:15:24Z

tests/logical_expressions/__snapshots__/jsfmt.spec.js.snap

+  (isAnyValueSelected
+    && node.getAttribute(radioAttr.toLowerCase()) === radioValue)
+  || (!isAnyValueSelected && values[a].default === true)
+  || a === 0;


Would it be possible to also change this case where the multiline operators are nested? When I encounter such a case in my code, I usually refactor it so that prettier doesn't make it ugly like that. If I was writing it manually I would write something like:

( isAnyValueSelected && node.getAttribute(radioAttr.toLowerCase()) === radioValue ) || ( !isAnyValueSelected && values[a].default === true ) || a === 0;

I'm trying to keep this change as small as possible to improve the odds the team decides to merge it. Based on the volume of 👍s on #3806, putting binary operators at the beginning of lines has pretty widespread support. It helps that the logic change to do that exclusively deletes code. I'm concerned that expanding the scope could reduce the chance it gets merged at all.

Sure. I didn't mean this comment to show as a "review" of this PR. It was just a random question 😅 This needs a separate issue.

btmills · 2020-03-02T17:55:34Z

@btmills There is still a conflict

@VincentLanglet I'm not surprised one of the snapshots has been updated since I last rebased. Rebasing the branch takes several minutes, so I haven't been actively monitoring this PR to keep it up to date. If the team indicates they're interested in merging, I'll certainly do another rebase as soon as possible!

btmills · 2020-03-23T03:55:31Z

I’ve rebased this on top of master now that 2.0 has been released.

tattali · 2020-07-13T09:40:26Z

Hi, it looks like the conflicting files are just snapshots and this for years.
Are there any plans to merge this pull request into a milestone?

There has been a lot of discussion around this PR I think it is a quite important one.

Thank you!

storm85049 · 2024-11-18T09:16:05Z

Any chance of merging this ? 🥹

Refs prettier#3806 These cover a few edge cases I encountered while maintaining `@btmills/prettier` over the last few years. I'm adding them up front to demonstrate that the `experimentalOperatorPosition` changes in a later commit have no effect on this formatting when the flag is not enabled.

Refs prettier#3806 This merely defines the flag. A future commit will use it in updated logic.

Right now, this flag isn't implemented, so these tests duplicate the existing behavior. I'm adding them now so that when I implement the flag's logic in a future commit, the changed formatting will be obvious in the diff and easier to review.

Fixes prettier#3806 as requested in prettier#7111 (comment) The flag's tests were already added in a previous commit, so the test changes you see in this commit are specifically due to the new flag's implementation.

In response to prettier#7111 (comment)

This clones TypeScript's type union operator tests in `union-parens.ts` and swaps the operators (& <-> |) to test the the type interesection operator. It also runs intersection tests with the new `experimentalOperatorPosition: "start"` flag. This does _not_ yet update the implementation to consider the flag; to ease review, I'll make that change in the next commit to isolate the impact of the change.

The type union operator was already formatted at the beginning of lines by default, so that logic didn't need to be updated.

I rebased from 1dfe28f to c706678 to fix test failures related to prettier#16601. The rebase applied cleanly, and this was the only change to tests. Since the PR has already been approved, I'll keep this as a separate commit rather than squashing into the prior commits.

btmills · 2024-11-24T22:41:21Z

I rebased from 1dfe28f to c706678 (main as of writing) to fix test failures in the most recent CI run related to #16601/#16642. The rebase applied without conflicts, and 32de8ac was the only change from yarn test -u. Since this PR has already been approved, I'll keep 32de8ac as a separate commit rather than squashing into the prior commits.

pkg-pr-new · 2025-01-03T16:12:25Z

Open in Stackblitz

npm i https://pkg.pr.new/prettier@7111

commit: 8c6ed25

sosukesuzuki · 2025-01-03T16:23:12Z

CI is failing due to causes unrelated to this PR.

btmills mentioned this pull request Dec 9, 2019

Placing operators at the beginning of lines #3806

Closed

alexander-akait added this to the 2.0 milestone Dec 9, 2019

thorn0 reviewed Dec 10, 2019

View reviewed changes

changelog_unreleased/javascript/pr-7111.md Outdated Show resolved Hide resolved

VincentLanglet mentioned this pull request Jan 9, 2020

Prettier 2.0 – 2.0 2020 #6888

Closed

thorn0 modified the milestones: 2.0, 3.0 Jan 9, 2020

thorn0 force-pushed the next branch 2 times, most recently from e16f758 to 7d88fb8 Compare January 10, 2020 00:39

btmills force-pushed the issue3806 branch 3 times, most recently from ba9d939 to 300b900 Compare February 3, 2020 06:00

phaux reviewed Feb 29, 2020

View reviewed changes

thorn0 force-pushed the next branch from 55247de to 262423a Compare March 21, 2020 19:03

thorn0 closed this Mar 22, 2020

thorn0 reopened this Mar 22, 2020

thorn0 changed the base branch from next to master March 22, 2020 21:11

btmills force-pushed the issue3806 branch 2 times, most recently from f5f93ae to 8723d6b Compare March 22, 2020 23:12

btmills mentioned this pull request Aug 24, 2020

option to set && || at new line btmills/prettier#2

Closed

btmills force-pushed the issue3806 branch from 8723d6b to bec3247 Compare August 26, 2020 06:32

btmills mentioned this pull request Sep 15, 2024

TypeScript type union and intersection operators still at end of line btmills/prettier#21

Open

fisker approved these changes Sep 27, 2024

View reviewed changes

fisker mentioned this pull request Nov 24, 2024

Release 3.4 #16850

Closed

11 tasks

fisker approved these changes Nov 24, 2024

View reviewed changes

fisker changed the title ~~JS: Break lines before binary operators (fixes #3806)~~ JS: Break lines before binary operators Nov 24, 2024

fisker changed the title ~~JS: Break lines before binary operators~~ Break lines before binary operators Nov 24, 2024

sosukesuzuki modified the milestones: 4.0, 3.5 Nov 24, 2024

btmills and others added 10 commits November 24, 2024 16:27

Define experimentalOperatorPosition flag

ef9dd44

Refs prettier#3806 This merely defines the flag. A future commit will use it in updated logic.

Implement experimentalOperatorPosition flag

77b63d9

Fixes prettier#3806 as requested in prettier#7111 (comment) The flag's tests were already added in a previous commit, so the test changes you see in this commit are specifically due to the new flag's implementation.

Document experimentalOperatorPosition flag

c5546a8

Change operator position option start|end

a727d66

In response to prettier#7111 (comment)

Apply experimentalOperatorPosition: start to type intersections

33050fe

The type union operator was already formatted at the beginning of lines by default, so that logic didn't need to be updated.

Update 7111.md

6b4bf3f

btmills force-pushed the issue3806 branch from a7bb07d to 32de8ac Compare November 24, 2024 22:33

sosukesuzuki approved these changes Jan 3, 2025

View reviewed changes

Merge branch 'main' into issue3806

8c6ed25

sosukesuzuki merged commit eed53fc into prettier:main Jan 3, 2025

DayKev mentioned this pull request May 4, 2025

📎 Break lines before binary operators biomejs/biome#5879

Closed

AA-Turner added a commit to AA-Turner/sphinx that referenced this pull request May 21, 2025

TMP: prettier/prettier#7111 binary ops

1318caa

bavalpey mentioned this pull request May 28, 2025

feat(formatter): add option to split binary expressions before operators biomejs/biome#6159

Merged

AlCalzone mentioned this pull request Feb 25, 2026

feat(oxfmt): support experimentalOperatorPosition option with default "start" oxc-project/oxc#19707

Closed

Uh oh!

Conversation

btmills commented Dec 9, 2019 • edited by fisker Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexander-akait commented Dec 9, 2019

Uh oh!

thorn0 commented Dec 9, 2019

Uh oh!

GrimzEcho commented Dec 9, 2019

Uh oh!

thorn0 commented Dec 10, 2019

Uh oh!

Uh oh!

fisker commented Jan 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

btmills commented Feb 3, 2020

Uh oh!

VincentLanglet commented Feb 29, 2020

Uh oh!

phaux Feb 29, 2020

Choose a reason for hiding this comment

Uh oh!

btmills Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

phaux Mar 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

btmills commented Mar 2, 2020

Uh oh!

btmills commented Mar 23, 2020

Uh oh!

tattali commented Jul 13, 2020

Uh oh!

storm85049 commented Nov 18, 2024

Uh oh!

btmills commented Nov 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pkg-pr-new bot commented Jan 3, 2025

Uh oh!

sosukesuzuki commented Jan 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

18 participants

btmills commented Dec 9, 2019 •

edited by fisker

Loading

fisker commented Jan 31, 2020 •

edited

Loading

phaux Mar 2, 2020 •

edited

Loading

btmills commented Nov 24, 2024 •

edited

Loading