From b8395d30ca8bfd651277b8c613e7eb9943bccc69 Mon Sep 17 00:00:00 2001 From: Ika Date: Fri, 9 Nov 2018 17:46:56 +0800 Subject: [PATCH 1/6] test: add tests --- tests/decorators/__snapshots__/jsfmt.spec.js.snap | 11 +++++++++++ tests/decorators/mobx.js | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/tests/decorators/__snapshots__/jsfmt.spec.js.snap b/tests/decorators/__snapshots__/jsfmt.spec.js.snap index aebdd51b21d8..61e6227d5381 100644 --- a/tests/decorators/__snapshots__/jsfmt.spec.js.snap +++ b/tests/decorators/__snapshots__/jsfmt.spec.js.snap @@ -161,6 +161,10 @@ import {observable} from "mobx"; @computed @computed @computed @computed @computed @computed @computed get total() { return this.price * this.amount; } + + @action handleDecrease = (event: React.ChangeEvent) => this.count--; + + @action handleSomething = (event: React.ChangeEvent) => doSomething(); } ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ import { observable } from "mobx"; @@ -202,6 +206,13 @@ class OrderLine { get total() { return this.price * this.amount; } + + @action + handleDecrease = (event: React.ChangeEvent) => this.count--; + + @action + handleSomething = (event: React.ChangeEvent) => + doSomething(); } `; diff --git a/tests/decorators/mobx.js b/tests/decorators/mobx.js index 2ab8ebd45458..a75a70af929e 100644 --- a/tests/decorators/mobx.js +++ b/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) => this.count--; + + @action handleSomething = (event: React.ChangeEvent) => doSomething(); } From e7c9e582f6a3097db45c4100c677b9b6aa94ea68 Mon Sep 17 00:00:00 2001 From: Ika Date: Fri, 9 Nov 2018 18:50:02 +0800 Subject: [PATCH 2/6] fix(javascript): inline property decorator should stay inline (part 2) --- src/language-js/printer-estree.js | 53 +++++++++++++------ .../__snapshots__/jsfmt.spec.js.snap | 7 ++- .../__snapshots__/jsfmt.spec.js.snap | 5 +- 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 8ffd8df003e3..61b7b067f920 100644 --- a/src/language-js/printer-estree.js +++ b/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,12 @@ function genericPrint(path, options, printPath, args) { options.locStart(node.decorators[0]) ) ) { - if ( + const shouldBreak = 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; - } + hasNewlineBetweenOrAfterDecorators(node, options); - const separator = shouldBreakDecorators ? hardline : line; + const separator = shouldBreak ? hardline : line; path.each(decoratorPath => { let decorator = decoratorPath.getValue(); @@ -187,13 +185,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 +1438,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 +2256,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 + " "); } diff --git a/tests/decorators/__snapshots__/jsfmt.spec.js.snap b/tests/decorators/__snapshots__/jsfmt.spec.js.snap index 61e6227d5381..25b6bb55ca8e 100644 --- a/tests/decorators/__snapshots__/jsfmt.spec.js.snap +++ b/tests/decorators/__snapshots__/jsfmt.spec.js.snap @@ -207,11 +207,10 @@ class OrderLine { return this.price * this.amount; } - @action - handleDecrease = (event: React.ChangeEvent) => this.count--; + @action handleDecrease = (event: React.ChangeEvent) => + this.count--; - @action - handleSomething = (event: React.ChangeEvent) => + @action handleSomething = (event: React.ChangeEvent) => doSomething(); } diff --git a/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap index dffb6cdb1b7b..fc764ebe4a1f 100644 --- a/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap @@ -241,8 +241,9 @@ class Bar { } class MyContainerComponent { - @ContentChildren(MyComponent) - components: QueryListSomeBigName; + @ContentChildren(MyComponent) components: QueryListSomeBigName< + MyComponentThat + >; } `; From 6c0d6d6b13bfc24327f1a9b18490fbad1da991d5 Mon Sep 17 00:00:00 2001 From: Ika Date: Fri, 9 Nov 2018 20:32:34 +0800 Subject: [PATCH 3/6] test: add tests --- .../__snapshots__/jsfmt.spec.js.snap | 42 +++++++++++++++++++ tests/decorators_export/after_export.js | 15 +++++-- tests/decorators_export/before_export.js | 4 ++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap b/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap index 4797feb8c01c..5fe8a4690cb9 100644 --- a/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap +++ b/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap @@ -3,12 +3,39 @@ 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 Baz {} + +export +@decorator +@decorator +@decorator +@decorator +@decorator +@decorator +@decorator +class Hello {} + export default @decorator class {} @@ -19,12 +46,27 @@ 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 {} diff --git a/tests/decorators_export/after_export.js b/tests/decorators_export/after_export.js index 7421b315e590..64dcb12ac15a 100644 --- a/tests/decorators_export/after_export.js +++ b/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 {} diff --git a/tests/decorators_export/before_export.js b/tests/decorators_export/before_export.js index d5eb648b15c5..7f0a5d926baa 100644 --- a/tests/decorators_export/before_export.js +++ b/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 {} From 32a981c10b0003bec48fae68f401eb59e2aa66a6 Mon Sep 17 00:00:00 2001 From: Ika Date: Fri, 9 Nov 2018 20:34:44 +0800 Subject: [PATCH 4/6] fix(javascript): remove that special case for class --- docs/rationale.md | 18 ------------------ src/language-js/printer-estree.js | 11 ++++------- .../__snapshots__/jsfmt.spec.js.snap | 3 +-- .../__snapshots__/jsfmt.spec.js.snap | 12 +++--------- .../__snapshots__/jsfmt.spec.js.snap | 3 +-- 5 files changed, 9 insertions(+), 38 deletions(-) diff --git a/docs/rationale.md b/docs/rationale.md index ca7132dddfba..837a87bab9a7 100644 --- a/docs/rationale.md +++ b/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. - - -```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 diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 61b7b067f920..a20ea45d6fda 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -117,12 +117,9 @@ function genericPrint(path, options, printPath, args) { options.locStart(node.decorators[0]) ) ) { - const shouldBreak = - node.type === "ClassExpression" || - node.type === "ClassDeclaration" || - hasNewlineBetweenOrAfterDecorators(node, options); - - const separator = shouldBreak ? hardline : line; + const separator = hasNewlineBetweenOrAfterDecorators(node, options) + ? hardline + : line; path.each(decoratorPath => { let decorator = decoratorPath.getValue(); @@ -136,7 +133,7 @@ function genericPrint(path, options, printPath, args) { }, "decorators"); if (parentExportDecl) { - decorators.unshift(hardline); + decorators.unshift(softline); } } else if ( isExportDeclaration(node) && diff --git a/tests/decorators/__snapshots__/jsfmt.spec.js.snap b/tests/decorators/__snapshots__/jsfmt.spec.js.snap index 25b6bb55ca8e..b9b6d9bdbaff 100644 --- a/tests/decorators/__snapshots__/jsfmt.spec.js.snap +++ b/tests/decorators/__snapshots__/jsfmt.spec.js.snap @@ -17,8 +17,7 @@ const foo = // }; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -@deco -class Foo {} +@deco class Foo {} @deco export class Bar {} diff --git a/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap b/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap index 5fe8a4690cb9..12d99ac2a731 100644 --- a/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap +++ b/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap @@ -14,13 +14,9 @@ class Hello {} export default @decorator class {} ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -export -@decorator -class Foo {} +export @decorator class Foo {} -export -@decorator -class Bar {} +export @decorator class Bar {} export @decorator @@ -36,9 +32,7 @@ export @decorator class Hello {} -export default -@decorator -class {} +export default @decorator class {} `; diff --git a/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap index fc764ebe4a1f..a4a887e0cf4b 100644 --- a/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap @@ -233,8 +233,7 @@ class Class3 { ) {} } -@decorated -class Foo {} +@decorated class Foo {} class Bar { @decorated method() {} From be4ae186d4183253c497b76a96299031e5055da2 Mon Sep 17 00:00:00 2001 From: Ika Date: Fri, 9 Nov 2018 22:00:02 +0800 Subject: [PATCH 5/6] Revert "fix(javascript): remove that special case for class" This reverts commit 32a981c10b0003bec48fae68f401eb59e2aa66a6. --- docs/rationale.md | 18 ++++++++++++++++++ src/language-js/printer-estree.js | 11 +++++++---- .../__snapshots__/jsfmt.spec.js.snap | 3 ++- .../__snapshots__/jsfmt.spec.js.snap | 12 +++++++++--- .../__snapshots__/jsfmt.spec.js.snap | 3 ++- 5 files changed, 38 insertions(+), 9 deletions(-) diff --git a/docs/rationale.md b/docs/rationale.md index 837a87bab9a7..ca7132dddfba 100644 --- a/docs/rationale.md +++ b/docs/rationale.md @@ -99,6 +99,24 @@ 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. + + +```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 diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index a20ea45d6fda..61b7b067f920 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -117,9 +117,12 @@ function genericPrint(path, options, printPath, args) { options.locStart(node.decorators[0]) ) ) { - const separator = hasNewlineBetweenOrAfterDecorators(node, options) - ? hardline - : line; + const shouldBreak = + node.type === "ClassExpression" || + node.type === "ClassDeclaration" || + hasNewlineBetweenOrAfterDecorators(node, options); + + const separator = shouldBreak ? hardline : line; path.each(decoratorPath => { let decorator = decoratorPath.getValue(); @@ -133,7 +136,7 @@ function genericPrint(path, options, printPath, args) { }, "decorators"); if (parentExportDecl) { - decorators.unshift(softline); + decorators.unshift(hardline); } } else if ( isExportDeclaration(node) && diff --git a/tests/decorators/__snapshots__/jsfmt.spec.js.snap b/tests/decorators/__snapshots__/jsfmt.spec.js.snap index b9b6d9bdbaff..25b6bb55ca8e 100644 --- a/tests/decorators/__snapshots__/jsfmt.spec.js.snap +++ b/tests/decorators/__snapshots__/jsfmt.spec.js.snap @@ -17,7 +17,8 @@ const foo = // }; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -@deco class Foo {} +@deco +class Foo {} @deco export class Bar {} diff --git a/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap b/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap index 12d99ac2a731..5fe8a4690cb9 100644 --- a/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap +++ b/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap @@ -14,9 +14,13 @@ class Hello {} export default @decorator class {} ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -export @decorator class Foo {} +export +@decorator +class Foo {} -export @decorator class Bar {} +export +@decorator +class Bar {} export @decorator @@ -32,7 +36,9 @@ export @decorator class Hello {} -export default @decorator class {} +export default +@decorator +class {} `; diff --git a/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap index a4a887e0cf4b..fc764ebe4a1f 100644 --- a/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_decorators/__snapshots__/jsfmt.spec.js.snap @@ -233,7 +233,8 @@ class Class3 { ) {} } -@decorated class Foo {} +@decorated +class Foo {} class Bar { @decorated method() {} From d31d7b49132e26173a1446295eb6fb06f399eee1 Mon Sep 17 00:00:00 2001 From: Ika Date: Fri, 9 Nov 2018 22:00:12 +0800 Subject: [PATCH 6/6] Revert "test: add tests" This reverts commit 6c0d6d6b13bfc24327f1a9b18490fbad1da991d5. --- .../__snapshots__/jsfmt.spec.js.snap | 42 ------------------- tests/decorators_export/after_export.js | 15 ++----- tests/decorators_export/before_export.js | 4 -- 3 files changed, 3 insertions(+), 58 deletions(-) diff --git a/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap b/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap index 5fe8a4690cb9..4797feb8c01c 100644 --- a/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap +++ b/tests/decorators_export/__snapshots__/jsfmt.spec.js.snap @@ -3,39 +3,12 @@ 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 Baz {} - -export -@decorator -@decorator -@decorator -@decorator -@decorator -@decorator -@decorator -class Hello {} - export default @decorator class {} @@ -46,27 +19,12 @@ 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 {} diff --git a/tests/decorators_export/after_export.js b/tests/decorators_export/after_export.js index 64dcb12ac15a..7421b315e590 100644 --- a/tests/decorators_export/after_export.js +++ b/tests/decorators_export/after_export.js @@ -1,12 +1,3 @@ -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 default @decorator class {} diff --git a/tests/decorators_export/before_export.js b/tests/decorators_export/before_export.js index 7f0a5d926baa..d5eb648b15c5 100644 --- a/tests/decorators_export/before_export.js +++ b/tests/decorators_export/before_export.js @@ -1,9 +1,5 @@ @decorator export class Foo {} -@decorator export class Bar {} - -@decorator @decorator @decorator @decorator @decorator @decorator export class Baz {} - @decorator export default class {}