fix(javascript): inline property decorator should stay inline (part 2)

docs/rationale.md

@@ -99,24 +99,6 @@ class HeroButtonComponent {
 }
 ```
 
-There's one exception: classes. We don't think it ever makes sense to inline the decorators for them, so they are always moved to their own line.
-
-<!-- prettier-ignore -->
-```js
-// Before running Prettier:
-@observer class OrderLine {
-  @observable price: number = 0;
-}
-```
-
-```js
-// After running Prettier:
-@observer
-class OrderLine {
-  @observable price: number = 0;
-}
-```
-
 Note: Prettier 1.14.x and older tried to automatically move your decorators, so if you've run an older Prettier version on your code you might need to manually join up some decorators here and there to avoid inconsistencies:
 
 ```js

src/language-js/printer-estree.js

@@ -98,8 +98,14 @@ function genericPrint(path, options, printPath, args) {
 
   const parentExportDecl = getParentExportDeclaration(path);
   const decorators = [];
-  let shouldBreakDecorators = false;
   if (
+    node.type === "ClassMethod" ||
+    node.type === "ClassProperty" ||
+    node.type === "TSAbstractClassProperty" ||
+    node.type === "ClassPrivateProperty"
+  ) {
+    // their decorators are handled themselves
+  } else if (
     node.decorators &&
     node.decorators.length > 0 &&
     // If the parent node is an export declaration and the decorator
@@ -111,20 +117,9 @@ function genericPrint(path, options, printPath, args) {
         options.locStart(node.decorators[0])
     )
   ) {
-    if (
-      node.type === "ClassExpression" ||
-      node.type === "ClassDeclaration" ||
-      hasNewlineInRange(
-        options.originalText,
-        options.locStart(node.decorators[0]),
-        options.locEnd(getLast(node.decorators))
-      ) ||
-      hasNewline(options.originalText, options.locEnd(getLast(node.decorators)))
-    ) {
-      shouldBreakDecorators = true;
-    }
-
-    const separator = shouldBreakDecorators ? hardline : line;
+    const separator = hasNewlineBetweenOrAfterDecorators(node, options)
+      ? hardline
+      : line;
 
     path.each(decoratorPath => {
       let decorator = decoratorPath.getValue();
@@ -138,7 +133,7 @@ function genericPrint(path, options, printPath, args) {
     }, "decorators");
 
     if (parentExportDecl) {
-      decorators.unshift(hardline);
+      decorators.unshift(softline);
     }
   } else if (
     isExportDeclaration(node) &&
@@ -187,13 +182,32 @@ function genericPrint(path, options, printPath, args) {
   }
 
   if (decorators.length > 0) {
-    return !shouldBreakDecorators && willBreak(concat(parts))
-      ? group(concat([group(concat(decorators)), concat(parts)]))
-      : group(concat(decorators.concat(parts)));
+    return group(concat(decorators.concat(parts)));
   }
   return concat(parts);
 }
 
+function hasNewlineBetweenOrAfterDecorators(node, options) {
+  return (
+    hasNewlineInRange(
+      options.originalText,
+      options.locStart(node.decorators[0]),
+      options.locEnd(getLast(node.decorators))
+    ) ||
+    hasNewline(options.originalText, options.locEnd(getLast(node.decorators)))
+  );
+}
+
+function printDecorators(path, options, print) {
+  const node = path.getValue();
+  return group(
+    concat([
+      join(line, path.map(print, "decorators")),
+      hasNewlineBetweenOrAfterDecorators(node, options) ? hardline : line
+    ])
+  );
+}
+
 function hasPrettierIgnore(path) {
   return hasIgnoreComment(path) || hasJsxIgnoreComment(path);
 }
@@ -1421,6 +1435,9 @@ function printPathNoParens(path, options, print, args) {
 
       return concat(parts); // Babel 6
     case "ClassMethod":
+      if (n.decorators && n.decorators.length !== 0) {
+        parts.push(printDecorators(path, options, print));
+      }
       if (n.static) {
         parts.push("static ");
       }
@@ -2236,6 +2253,9 @@ function printPathNoParens(path, options, print, args) {
     case "ClassProperty":
     case "TSAbstractClassProperty":
     case "ClassPrivateProperty": {
+      if (n.decorators && n.decorators.length !== 0) {
+        parts.push(printDecorators(path, options, print));
+      }
       if (n.accessibility) {
         parts.push(n.accessibility + " ");
       }

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

@@ -17,8 +17,7 @@ const foo =
     //
   };
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-@deco
-class Foo {}
+@deco class Foo {}
 
 @deco
 export class Bar {}
@@ -161,6 +160,10 @@ import {observable} from "mobx";
   @computed @computed @computed @computed @computed @computed @computed get total() {
     return this.price * this.amount;
   }
+
+  @action handleDecrease = (event: React.ChangeEvent<HTMLInputElement>) => this.count--;
+
+  @action handleSomething = (event: React.ChangeEvent<HTMLInputElement>) => doSomething();
 }
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 import { observable } from "mobx";
@@ -202,6 +205,12 @@ class OrderLine {
   get total() {
     return this.price * this.amount;
   }
+
+  @action handleDecrease = (event: React.ChangeEvent<HTMLInputElement>) =>
+    this.count--;
+
+  @action handleSomething = (event: React.ChangeEvent<HTMLInputElement>) =>
+    doSomething();
 }
 
 `;

tests/decorators/mobx.js

@@ -29,4 +29,8 @@ import {observable} from "mobx";
   @computed @computed @computed @computed @computed @computed @computed get total() {
     return this.price * this.amount;
   }
+
+  @action handleDecrease = (event: React.ChangeEvent<HTMLInputElement>) => this.count--;
+
+  @action handleSomething = (event: React.ChangeEvent<HTMLInputElement>) => doSomething();
 }

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

@@ -3,28 +3,64 @@
 exports[`after_export.js - babylon-verify 1`] = `
 export @decorator class Foo {}
 
+export
+@decorator class Bar {}
+
+export @decorator
+class Baz {}
+
+export @decorator @decorator @decorator @decorator @decorator @decorator @decorator
+class Hello {}
+
 export default @decorator class {}
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+export @decorator class Foo {}
+
+export @decorator class Bar {}
+
 export
 @decorator
-class Foo {}
+class Baz {}
 
-export default
+export
+@decorator
+@decorator
+@decorator
+@decorator
+@decorator
+@decorator
 @decorator
-class {}
+class Hello {}
+
+export default @decorator class {}
 
 `;
 
 exports[`before_export.js - babylon-verify 1`] = `
 @decorator
 export class Foo {}
 
+@decorator export class Bar {}
+
+@decorator @decorator @decorator @decorator @decorator @decorator export class Baz {}
+
 @decorator
 export default class {}
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 @decorator
 export class Foo {}
 
+@decorator
+export class Bar {}
+
+@decorator
+@decorator
+@decorator
+@decorator
+@decorator
+@decorator
+export class Baz {}
+
 @decorator
 export default class {}
 

tests/decorators_export/after_export.js

@@ -1,3 +1,12 @@
-export @decorator class Foo {}
-
-export default @decorator class {}
+export @decorator class Foo {}
+
+export
+@decorator class Bar {}
+
+export @decorator
+class Baz {}
+
+export @decorator @decorator @decorator @decorator @decorator @decorator @decorator
+class Hello {}
+
+export default @decorator class {}

tests/decorators_export/before_export.js

@@ -1,5 +1,9 @@
 @decorator
 export class Foo {}
 
+@decorator export class Bar {}
+
+@decorator @decorator @decorator @decorator @decorator @decorator export class Baz {}
+
 @decorator
 export default class {}

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

@@ -233,16 +233,16 @@ class Class3 {
   ) {}
 }
 
-@decorated
-class Foo {}
+@decorated class Foo {}
 
 class Bar {
   @decorated method() {}
 }
 
 class MyContainerComponent {
-  @ContentChildren(MyComponent)
-  components: QueryListSomeBigName<MyComponentThat>;
+  @ContentChildren(MyComponent) components: QueryListSomeBigName<
+    MyComponentThat
+  >;
 }
 
 `;