Deprecate jsxBracketSameLine option in favour of multi-language brack…

…etSameLine option (#11006) * deprecate jsxBracketSameLine in favour of multi-language angleBracketSameLine option * update docs and snapshots * update tests to to make them more reviewable * Rewrite angular tests * Rewrite html tests * Rewrite vue tests * Restore unnecessary changes in jsx * Fix file location * Test both `true` and `false` * Revert changes in markdown * Run tests on all js parsers * Improve html test * fix changelog comment * move bracketSameLine to common options * mark jsxBracketSameLine as deprecated * Update changelog_unreleased/html/11006.md Co-authored-by: fisker Cheung <lionkay@gmail.com> * Update 11006.md * fix tests by removing `jsxBracketSameLine` `default` property * fix style * Revert docs under website dir * Add tests for deprecated options * Fix lint problems Co-authored-by: fisker Cheung <lionkay@gmail.com> Co-authored-by: sosukesuzuki <aosukeke@gmail.com>
prettier · Aug 31, 2021 · 4992d97 · 4992d97
1 parent af19dd1
commit 4992d97
Show file tree

Hide file tree

Showing 37 changed files with 1,207 additions and 70 deletions.
diff --git a/changelog_unreleased/html/11006.md b/changelog_unreleased/html/11006.md
@@ -0,0 +1,35 @@
+#### Replace `jsxBracketSameLine` option with `bracketSameLine` option (#11006 by @kurtztech)
+
+Deprecate the `jsxBracketSameLine` option in favour of new `bracketSameLine` option that will work for HTML, Angular, Vue, and JSX.
+
+<!-- prettier-ignore -->
+```html
+<!-- Input -->
+<div id="foo-bar-baz"
+  class="bar-foo-baz"
+  title="a sample title"
+  data-foo="bar"
+  data-bar="baz">lorem ipsum dolor sit amet</div>
+
+<!-- Prettier stable -->
+<div
+  id="foo-bar-baz"
+  class="bar-foo-baz"
+  title="a sample title"
+  data-foo="bar"
+  data-bar="baz"
+>
+  lorem ipsum dolor sit amet
+</div>
+
+<!-- Prettier main -->
+<!-- Options: `{bracketSameLine: true}` -->
+<div
+  id="foo-bar-baz"
+  class="bar-foo-baz"
+  title="a sample title"
+  data-foo="bar"
+  data-bar="baz">
+  lorem ipsum dolor sit amet
+</div>
+```
diff --git a/docs/option-philosophy.md b/docs/option-philosophy.md
@@ -29,7 +29,7 @@ Options that are easier to motivate include:
 - `--end-of-line` makes it easier for teams to keep CRLFs out of their git repositories.
 - `--quote-props` is important for advanced usage of the Google Closure Compiler.
 
-But other options are harder to motivate in hindsight: `--arrow-parens`, `--jsx-single-quote`, `--jsx-bracket-same-line` and `--no-bracket-spacing` are not the type of options we’re happy to have. They cause a lot of [bike-shedding](https://en.wikipedia.org/wiki/Law_of_triviality) in teams, and we’re sorry for that. Difficult to remove now, these options exist as a historical artifact and should not motivate adding more options (“If _those_ options exist, why can’t this one?”).
+But other options are harder to motivate in hindsight: `--arrow-parens`, `--jsx-single-quote`, `--bracket-same-line` and `--no-bracket-spacing` are not the type of options we’re happy to have. They cause a lot of [bike-shedding](https://en.wikipedia.org/wiki/Law_of_triviality) in teams, and we’re sorry for that. Difficult to remove now, these options exist as a historical artifact and should not motivate adding more options (“If _those_ options exist, why can’t this one?”).
 
 For a long time, we left option requests open in order to let discussions play out and collect feedback. What we’ve learned during those years is that it’s really hard to measure demand. Prettier has grown a lot in usage. What was “great demand” back in the day is not as much today. GitHub reactions and Twitter polls became unrepresentative. What about all silent users? It looked easy to add “just one more” option. But where should we have stopped? When is one too many? Even after adding “that one final option”, there would always be a “top issue” in the issue tracker.
 

diff --git a/docs/options.md b/docs/options.md
@@ -135,7 +135,44 @@ Valid options:
 | ------- | ---------------------- | ------------------------ |
 | `true`  | `--no-bracket-spacing` | `bracketSpacing: <bool>` |
 
-## JSX Brackets
+## Bracket Line
+
+Put the `>` of a multi-line HTML (HTML, JSX, Vue, Angular) element at the end of the last line instead of being alone on the next line (does not apply to self closing elements).
+
+Valid options:
+
+- `true` - Example:
+
+<!-- prettier-ignore -->
+```html
+<button
+  className="prettier-class"
+  id="prettier-id"
+  onClick={this.handleClick}>
+  Click Here
+</button>
+```
+
+- `false` - Example:
+
+<!-- prettier-ignore -->
+```html
+<button
+  className="prettier-class"
+  id="prettier-id"
+  onClick={this.handleClick}
+>
+  Click Here
+</button>
+```
+
+| Default | CLI Override          | API Override              |
+| ------- | --------------------- | ------------------------- |
+| `false` | `--bracket-same-line` | `bracketSameLine: <bool>` |
+
+## [Deprecated] JSX Brackets
+
+_This option has been deprecated in v2.4.0, use --bracket-same-line instead_
 
 Put the `>` of a multi-line JSX element at the end of the last line instead of being alone on the next line (does not apply to self closing elements).
 

diff --git a/src/common/common-options.js b/src/common/common-options.js
@@ -46,4 +46,12 @@ module.exports = {
       },
     ],
   },
+  bracketSameLine: {
+    since: "2.4.0",
+    category: CATEGORY_COMMON,
+    type: "boolean",
+    default: false,
+    description:
+      "Put > of opening tags on the last line instead of on a new line.",
+  },
 };
diff --git a/src/language-html/options.js b/src/language-html/options.js
@@ -1,9 +1,12 @@
 "use strict";
 
+const commonOptions = require("../common/common-options.js");
+
 const CATEGORY_HTML = "HTML";
 
 // format based on https://github.com/prettier/prettier/blob/main/src/main/core-options.js
 module.exports = {
+  bracketSameLine: commonOptions.bracketSameLine,
   htmlWhitespaceSensitivity: {
     since: "1.15.0",
     category: CATEGORY_HTML,

diff --git a/src/language-html/printer-html.js b/src/language-html/printer-html.js
@@ -706,7 +706,15 @@ function printAttributes(path, options, print) {
   ) {
     parts.push(node.isSelfClosing ? " " : "");
   } else {
-    parts.push(node.isSelfClosing ? line : softline);
+    parts.push(
+      options.bracketSameLine
+        ? node.isSelfClosing
+          ? " "
+          : ""
+        : node.isSelfClosing
+        ? line
+        : softline
+    );
   }
 
   return parts;

diff --git a/src/language-js/options.js b/src/language-js/options.js
@@ -26,13 +26,14 @@ module.exports = {
       },
     ],
   },
+  bracketSameLine: commonOptions.bracketSameLine,
   bracketSpacing: commonOptions.bracketSpacing,
   jsxBracketSameLine: {
     since: "0.17.0",
     category: CATEGORY_JAVASCRIPT,
     type: "boolean",
-    default: false,
     description: "Put > on the last line instead of at a new line.",
+    deprecated: "2.4.0",
   },
   semi: {
     since: "1.0.0",

diff --git a/src/language-js/print/jsx.js b/src/language-js/print/jsx.js
@@ -576,9 +576,11 @@ function printJsxOpeningElement(path, options, print) {
 
   const bracketSameLine =
     // Simple tags (no attributes and no comment in tag name) should be
-    // kept unbroken regardless of `jsxBracketSameLine`
+    // kept unbroken regardless of `bracketSameLine`.
+    // jsxBracketSameLine is deprecated in favour of bracketSameLine,
+    // but is still needed for backwards compatibility.
     (node.attributes.length === 0 && !nameHasComments) ||
-    (options.jsxBracketSameLine &&
+    ((options.bracketSameLine || options.jsxBracketSameLine) &&
       // We should print the bracket in a new line for the following cases:
       // <div
       //   // comment

diff --git a/tests/format/angular/bracket-same-line/__snapshots__/jsfmt.spec.js.snap b/tests/format/angular/bracket-same-line/__snapshots__/jsfmt.spec.js.snap
@@ -0,0 +1,128 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`angularjs.html - {"bracketSameLine":false} format 1`] = `
+====================================options=====================================
+bracketSameLine: false
+parsers: ["angular"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+<div ng-if="$ctrl .shouldShowWarning&&!$ctrl.loading"
+    bindon-target=" a | b : c "
+    (event)="  foo == $event  "
+    *ngIf=" something?true:false    " [(ngModel)]="canSave">Warning!</div>
+<span ng-if="$ctrl .shouldShowWarning&&!$ctrl.loading"
+    bindon-target=" a | b : c "
+    (event)="  foo == $event  "
+    *ngIf=" something?true:false    " [(ngModel)]="canSave">Warning!</span>
+<img ng-if="$ctrl .shouldShowWarning&&!$ctrl.loading"
+    bindon-target=" a | b : c "
+    (event)="  foo == $event  "
+    *ngIf=" something?true:false    " [(ngModel)]="canSave"/>
+<script long-attribute="long_long_long_long_long_long_long_long_long_long_long_long_long_value">alert(1)</script>
+<div (event)="  foo == $event  ">Warning!</div>
+<span (event)="  foo == $event  ">Warning!</span>
+<img (event)="  foo == $event  "/>
+<script>alert(1)</script>
+
+=====================================output=====================================
+<div
+  ng-if="$ctrl.shouldShowWarning && !$ctrl.loading"
+  bindon-target="a | b: c"
+  (event)="(foo == $event)"
+  *ngIf="something ? true : false"
+  [(ngModel)]="canSave"
+>
+  Warning!
+</div>
+<span
+  ng-if="$ctrl.shouldShowWarning && !$ctrl.loading"
+  bindon-target="a | b: c"
+  (event)="(foo == $event)"
+  *ngIf="something ? true : false"
+  [(ngModel)]="canSave"
+  >Warning!</span
+>
+<img
+  ng-if="$ctrl.shouldShowWarning && !$ctrl.loading"
+  bindon-target="a | b: c"
+  (event)="(foo == $event)"
+  *ngIf="something ? true : false"
+  [(ngModel)]="canSave"
+/>
+<script
+  long-attribute="long_long_long_long_long_long_long_long_long_long_long_long_long_value"
+>
+  alert(1);
+</script>
+<div (event)="(foo == $event)">Warning!</div>
+<span (event)="(foo == $event)">Warning!</span>
+<img (event)="(foo == $event)" />
+<script>
+  alert(1);
+</script>
+
+================================================================================
+`;
+
+exports[`angularjs.html - {"bracketSameLine":true} format 1`] = `
+====================================options=====================================
+bracketSameLine: true
+parsers: ["angular"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+<div ng-if="$ctrl .shouldShowWarning&&!$ctrl.loading"
+    bindon-target=" a | b : c "
+    (event)="  foo == $event  "
+    *ngIf=" something?true:false    " [(ngModel)]="canSave">Warning!</div>
+<span ng-if="$ctrl .shouldShowWarning&&!$ctrl.loading"
+    bindon-target=" a | b : c "
+    (event)="  foo == $event  "
+    *ngIf=" something?true:false    " [(ngModel)]="canSave">Warning!</span>
+<img ng-if="$ctrl .shouldShowWarning&&!$ctrl.loading"
+    bindon-target=" a | b : c "
+    (event)="  foo == $event  "
+    *ngIf=" something?true:false    " [(ngModel)]="canSave"/>
+<script long-attribute="long_long_long_long_long_long_long_long_long_long_long_long_long_value">alert(1)</script>
+<div (event)="  foo == $event  ">Warning!</div>
+<span (event)="  foo == $event  ">Warning!</span>
+<img (event)="  foo == $event  "/>
+<script>alert(1)</script>
+
+=====================================output=====================================
+<div
+  ng-if="$ctrl.shouldShowWarning && !$ctrl.loading"
+  bindon-target="a | b: c"
+  (event)="(foo == $event)"
+  *ngIf="something ? true : false"
+  [(ngModel)]="canSave">
+  Warning!
+</div>
+<span
+  ng-if="$ctrl.shouldShowWarning && !$ctrl.loading"
+  bindon-target="a | b: c"
+  (event)="(foo == $event)"
+  *ngIf="something ? true : false"
+  [(ngModel)]="canSave"
+  >Warning!</span
+>
+<img
+  ng-if="$ctrl.shouldShowWarning && !$ctrl.loading"
+  bindon-target="a | b: c"
+  (event)="(foo == $event)"
+  *ngIf="something ? true : false"
+  [(ngModel)]="canSave" />
+<script
+  long-attribute="long_long_long_long_long_long_long_long_long_long_long_long_long_value">
+  alert(1);
+</script>
+<div (event)="(foo == $event)">Warning!</div>
+<span (event)="(foo == $event)">Warning!</span>
+<img (event)="(foo == $event)" />
+<script>
+  alert(1);
+</script>
+
+================================================================================
+`;
diff --git a/tests/format/angular/bracket-same-line/angularjs.html b/tests/format/angular/bracket-same-line/angularjs.html
@@ -0,0 +1,17 @@
+<div ng-if="$ctrl .shouldShowWarning&&!$ctrl.loading"
+    bindon-target=" a | b : c "
+    (event)="  foo == $event  "
+    *ngIf=" something?true:false    " [(ngModel)]="canSave">Warning!</div>
+<span ng-if="$ctrl .shouldShowWarning&&!$ctrl.loading"
+    bindon-target=" a | b : c "
+    (event)="  foo == $event  "
+    *ngIf=" something?true:false    " [(ngModel)]="canSave">Warning!</span>
+<img ng-if="$ctrl .shouldShowWarning&&!$ctrl.loading"
+    bindon-target=" a | b : c "
+    (event)="  foo == $event  "
+    *ngIf=" something?true:false    " [(ngModel)]="canSave"/>
+<script long-attribute="long_long_long_long_long_long_long_long_long_long_long_long_long_value">alert(1)</script>
+<div (event)="  foo == $event  ">Warning!</div>
+<span (event)="  foo == $event  ">Warning!</span>
+<img (event)="  foo == $event  "/>
+<script>alert(1)</script>
diff --git a/tests/format/angular/bracket-same-line/jsfmt.spec.js b/tests/format/angular/bracket-same-line/jsfmt.spec.js
@@ -0,0 +1,2 @@
+run_spec(__dirname, ["angular"], { bracketSameLine: true });
+run_spec(__dirname, ["angular"], { bracketSameLine: false });