Repository: jlongster/prettier (js)

Issue:
JSX opening element closing tag gets moved to a new line
If you have a line of JSX that is over printWidth, something like:

```js
const El = (<span width="1" height="2">x</span>)
```

For the sake of reproducing the issue assume the printWidth is set to 20 but this reproduces anytime the line needs to break for JSX. Then the code is formatted as:

```js
const El = (
    <span
        width="1"
        height="2"
    >
        x
    </span>
);
```

Notice the span `>` being placed on a new line by itself. 
Again this wastes lots of vertical space whenever you have many tags in a component. Instead a better option would be either to make it configurable or to place it on the same line as the last prop, eg:

```js
const El = (
    <span
        width="1"
        height="2" >
        x
    </span>
);
```

Pull request description:
Add option for putting > on the last line in jsx
Feel free to suggest a better name for the option. Otherwise it works great :)

Fixes #654

Patch:
diff --git a/README.md b/README.md
index 39fd19e3f..d408e9786 100644
--- a/README.md
+++ b/README.md
@@ -148,6 +148,10 @@ prettier.format(source, {
   // Controls the printing of spaces inside object literals
   bracketSpacing: true,
 
+  // If true, puts the `>` of a multi-line jsx element at the end of
+  // the last line instead of being alone on the next line
+  jsxBracketSameLine: false,
+
   // Which parser to use. Valid options are 'flow' and 'babylon'
   parser: 'babylon'
 });
diff --git a/bin/prettier.js b/bin/prettier.js
index f7b32acb6..66c516b8d 100755
--- a/bin/prettier.js
+++ b/bin/prettier.js
@@ -16,6 +16,7 @@ const argv = minimist(process.argv.slice(2), {
     "single-quote",
     "trailing-comma",
     "bracket-spacing",
+    "jsx-bracket-same-line",
     // The supports-color package (a sub sub dependency) looks directly at
     // `process.argv` for `--no-color` and such-like options. The reason it is
     // listed here is to avoid "Ignored unknown option: --no-color" warnings.
@@ -59,6 +60,7 @@ if (argv["help"] || !filepatterns.length && !stdin) {
       "  --single-quote           Use single quotes instead of double.\n" +
       "  --trailing-comma         Print trailing commas wherever possible.\n" +
       "  --bracket-spacing        Put spaces between brackets. Defaults to true.\n" +
+      "  --jsx-bracket-same-line  Put > on the last line. Defaults to false.\n" +
       "  --parser <flow|babylon>  Specify which parse to use. Defaults to babylon.\n" +
       "  --color                  Colorize error messages. Defaults to true.\n" +
       "  --version                Print prettier version.\n" +
@@ -125,7 +127,8 @@ const options = {
   bracketSpacing: argv["bracket-spacing"],
   parser: getParserOption(),
   singleQuote: argv["single-quote"],
-  trailingComma: argv["trailing-comma"]
+  trailingComma: argv["trailing-comma"],
+  jsxBracketSameLine: argv["jsx-bracket-same-line"],
 };
 
 function format(input) {
diff --git a/docs/index.html b/docs/index.html
index 90fb6e6e6..1bbb9d15f 100644
--- a/docs/index.html
+++ b/docs/index.html
@@ -91,6 +91,7 @@
     <label><input type="checkbox" id="singleQuote"></input> singleQuote</label>
     <label><input type="checkbox" id="trailingComma"></input> trailingComma</label>
     <label><input type="checkbox" id="bracketSpacing" checked></input> bracketSpacing</label>
+    <label><input type="checkbox" id="jsxBracketSameLine"></input> jsxBracketSameLine</label>
     <span style="flex: 1"></span>
     <label><input type="checkbox" id="doc"></input> doc</label>
   </div>
@@ -109,7 +110,7 @@
 </div>
 
 <script id="code">
-var OPTIONS = ['printWidth', 'tabWidth', 'singleQuote', 'trailingComma', 'bracketSpacing', 'doc'];
+var OPTIONS = ['printWidth', 'tabWidth', 'singleQuote', 'trailingComma', 'bracketSpacing', 'jsxBracketSameLine', 'doc'];
 function setOptions(options) {
   OPTIONS.forEach(function(option) {
     var elem = document.getElementById(option);
diff --git a/src/options.js b/src/options.js
index e696e7eed..73fc4191d 100644
--- a/src/options.js
+++ b/src/options.js
@@ -4,17 +4,12 @@ var validate = require("jest-validate").validate;
 var deprecatedConfig = require("./deprecated");
 
 var defaults = {
-  // Number of spaces the pretty-printer should use per tab
   tabWidth: 2,
-  // Fit code within this line limit
   printWidth: 80,
-  // If true, will use single instead of double quotes
   singleQuote: false,
-  // Controls the printing of trailing commas wherever possible
   trailingComma: false,
-  // Controls the printing of spaces inside array and objects
   bracketSpacing: true,
-  // Which parser to use. Valid options are 'flow' and 'babylon'
+  jsxBracketSameLine: false,
   parser: "babylon"
 };
 
diff --git a/src/printer.js b/src/printer.js
index b99391c46..2ab204c38 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -1219,9 +1219,9 @@ function genericPrintNoParens(path, options, print) {
                 path.map(attr => concat([line, print(attr)]), "attributes")
               )
             ),
-            n.selfClosing ? line : softline
+            n.selfClosing ? line : (options.jsxBracketSameLine ? ">" : softline)
           ]),
-          n.selfClosing ? "/>" : ">"
+          n.selfClosing ? "/>" : (options.jsxBracketSameLine ? "" : ">")
         ])
       );
     }
@@ -1453,8 +1453,10 @@ function genericPrintNoParens(path, options, print) {
       ]);
     case "DeclareInterface":
     case "InterfaceDeclaration": {
-      const parent = path.getParentNode(1);
-      if (parent && parent.type === "DeclareModule") {
+      if (
+        n.type === "DeclareInterface" ||
+          isFlowNodeStartingWithDeclare(n, options)
+      ) {
         parts.push("declare ");
       }
 
@@ -1568,8 +1570,10 @@ function genericPrintNoParens(path, options, print) {
       return "string";
     case "DeclareTypeAlias":
     case "TypeAlias": {
-      const parent = path.getParentNode(1);
-      if (parent && parent.type === "DeclareModule") {
+      if (
+        n.type === "DeclareTypeAlias" ||
+          isFlowNodeStartingWithDeclare(n, options)
+      ) {
         parts.push("declare ");
       }
 
@@ -2779,6 +2783,16 @@ function shouldPrintSameLine(node) {
     type === "UnaryExpression";
 }
 
+function isFlowNodeStartingWithDeclare(node, options) {
+  if (options.parser !== "flow") {
+    return false;
+  }
+
+  return options.originalText
+    .slice(0, util.locStart(node))
+    .match(/declare\s*$/);
+}
+
 function printAstToDoc(ast, options) {
   function printGenerically(path) {
     return comments.printComments(


Test patch:
diff --git a/tests/flow/async_iteration/__snapshots__/jsfmt.spec.js.snap b/tests/flow/async_iteration/__snapshots__/jsfmt.spec.js.snap
index c8118a694..c4f8a3418 100644
--- a/tests/flow/async_iteration/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/async_iteration/__snapshots__/jsfmt.spec.js.snap
@@ -82,7 +82,7 @@ async function f() {
   }
 }
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-interface File {
+declare interface File {
   readLine(): Promise<string>,
   close(): void,
   EOF: boolean
diff --git a/tests/flow/declare_type/lib/__snapshots__/jsfmt.spec.js.snap b/tests/flow/declare_type/lib/__snapshots__/jsfmt.spec.js.snap
index 6da5947e2..f2c6c921a 100644
--- a/tests/flow/declare_type/lib/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/declare_type/lib/__snapshots__/jsfmt.spec.js.snap
@@ -1,23 +1,29 @@
-exports[`test DeclareModule_TypeAlias.js 1`] = `
+exports[`test declare_module_type_alias.js 1`] = `
 "/**
  * @flow
  */
 
+// TODO: adding an empty line should not be dropped.
+
 declare module ModuleAliasFoo {
     declare type baz = number;
     declare type toz = string;
     declare function foo(bar : baz) : toz;
 }
+declare type Foo = string;
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 /**
  * @flow
  */
 
+// TODO: adding an empty line should not be dropped.
+
 declare module ModuleAliasFoo {
   declare type baz = number;
   declare type toz = string;
   declare function foo(bar: baz): toz;
 }
+declare type Foo = string;
 "
 `;
 
diff --git a/tests/flow/declare_type/lib/DeclareModule_TypeAlias.js b/tests/flow/declare_type/lib/declare_module_type_alias.js
similarity index 65%
rename from tests/flow/declare_type/lib/DeclareModule_TypeAlias.js
rename to tests/flow/declare_type/lib/declare_module_type_alias.js
index 325d344de..5194cc672 100644
--- a/tests/flow/declare_type/lib/DeclareModule_TypeAlias.js
+++ b/tests/flow/declare_type/lib/declare_module_type_alias.js
@@ -2,8 +2,11 @@
  * @flow
  */
 
+// TODO: adding an empty line should not be dropped.
+
 declare module ModuleAliasFoo {
     declare type baz = number;
     declare type toz = string;
     declare function foo(bar : baz) : toz;
 }
+declare type Foo = string;
diff --git a/tests/flow/libflow-typed/flow-typed/__snapshots__/jsfmt.spec.js.snap b/tests/flow/libflow-typed/flow-typed/__snapshots__/jsfmt.spec.js.snap
index 8d037dfdc..5874c610f 100644
--- a/tests/flow/libflow-typed/flow-typed/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/libflow-typed/flow-typed/__snapshots__/jsfmt.spec.js.snap
@@ -1,6 +1,6 @@
 exports[`test dino.js 1`] = `
 "declare type Dinosaur = \"T-Rex\" | \"Apatosaurus\";
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-type Dinosaur = \"T-Rex\" | \"Apatosaurus\";
+declare type Dinosaur = \"T-Rex\" | \"Apatosaurus\";
 "
 `;
diff --git a/tests/flow/poly/__snapshots__/jsfmt.spec.js.snap b/tests/flow/poly/__snapshots__/jsfmt.spec.js.snap
index 923475f3a..eaeb0ef07 100644
--- a/tests/flow/poly/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/poly/__snapshots__/jsfmt.spec.js.snap
@@ -97,7 +97,7 @@ unbox(bool);
 // naive unification causes combinatorial explosion here,
 // effectively hangs
 
-type Box<T> = {
+declare type Box<T> = {
   map1<U>(f: (x: T) => U): Box<U>,
   map2<U>(f: (x: T) => U): Box<U>,
   map3<U>(f: (x: T) => U): Box<U>,
diff --git a/tests/flow/tagged-unions/__snapshots__/jsfmt.spec.js.snap b/tests/flow/tagged-unions/__snapshots__/jsfmt.spec.js.snap
index 575481e0c..c963928af 100644
--- a/tests/flow/tagged-unions/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/tagged-unions/__snapshots__/jsfmt.spec.js.snap
@@ -95,20 +95,20 @@ if (data.kind === \"user\") {
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 /* @flow */
 
-interface IDataBase {
+declare interface IDataBase {
   id: string,
   name: string
 }
 
-interface IUserData extends IDataBase {
+declare interface IUserData extends IDataBase {
   kind: \"user\"
 }
 
-interface ISystemData extends IDataBase {
+declare interface ISystemData extends IDataBase {
   kind: \"system\"
 }
 
-type IData = IUserData | ISystemData;
+declare type IData = IUserData | ISystemData;
 
 const data: IData = {
   id: \"\",
@@ -152,20 +152,20 @@ if (data.kind === \"system\") {
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 /* @flow */
 
-interface IDataBase {
+declare interface IDataBase {
   id: string,
   name: string
 }
 
-interface IUserData extends IDataBase {
+declare interface IUserData extends IDataBase {
   kind: \"user\"
 }
 
-interface ISystemData extends IDataBase {
+declare interface ISystemData extends IDataBase {
   kind: \"system\"
 }
 
-type IData = IUserData | ISystemData;
+declare type IData = IUserData | ISystemData;
 
 const data: IData = {
   id: \"\",
@@ -230,7 +230,7 @@ type SystemData = {
   kind: \"system\"
 };
 
-type Data = UserData | SystemData;
+declare type Data = UserData | SystemData;
 
 const data: Data = {
   id: \"\",
@@ -295,7 +295,7 @@ type SystemData = {
   kind: \"system\"
 };
 
-type Data = UserData | SystemData;
+declare type Data = UserData | SystemData;
 
 const data: Data = {
   id: \"\",
diff --git a/tests/flow/traits/__snapshots__/jsfmt.spec.js.snap b/tests/flow/traits/__snapshots__/jsfmt.spec.js.snap
index 8e66f054d..8d8953318 100644
--- a/tests/flow/traits/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/traits/__snapshots__/jsfmt.spec.js.snap
@@ -48,7 +48,7 @@ exports[`test test2.js 1`] = `
 "declare interface I { }
 declare class C mixins I { }
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-interface I {}
+declare interface I {}
 declare class C {}
 "
 `;
diff --git a/tests/flow/union_new/__snapshots__/jsfmt.spec.js.snap b/tests/flow/union_new/__snapshots__/jsfmt.spec.js.snap
index 2ecec1c85..f6a4580f7 100644
--- a/tests/flow/union_new/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/union_new/__snapshots__/jsfmt.spec.js.snap
@@ -403,20 +403,20 @@ function test(f: Foo| EmptyFoo) {
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 /* @flow */
 
-type Entity<T> = {
+declare type Entity<T> = {
   id: T,
   name: string
 };
 
-type StringEntity = Entity<string>;
+declare type StringEntity = Entity<string>;
 
-type Foo =
+declare type Foo =
   & StringEntity
   & {
     bars: Object,
     kind: 1
   };
-type EmptyFoo =
+declare type EmptyFoo =
   & StringEntity
   & {
     bars: null,
diff --git a/tests/jsx_last_line/__snapshots__/jsfmt.spec.js.snap b/tests/jsx_last_line/__snapshots__/jsfmt.spec.js.snap
new file mode 100644
index 000000000..0d19c4878
--- /dev/null
+++ b/tests/jsx_last_line/__snapshots__/jsfmt.spec.js.snap
@@ -0,0 +1,48 @@
+exports[`test last_line.js 1`] = `
+"<SomeHighlyConfiguredComponent
+  onEnter={this.onEnter}
+  onLeave={this.onLeave}
+  onChange={this.onChange}
+  initialValue={this.state.initialValue}
+  ignoreStuff={true}
+>
+  <div>and the children go here</div>
+  <div>and here too</div>
+</SomeHighlyConfiguredComponent>
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+<SomeHighlyConfiguredComponent
+  onEnter={this.onEnter}
+  onLeave={this.onLeave}
+  onChange={this.onChange}
+  initialValue={this.state.initialValue}
+  ignoreStuff={true}>
+  <div>and the children go here</div>
+  <div>and here too</div>
+</SomeHighlyConfiguredComponent>;
+"
+`;
+
+exports[`test last_line.js 2`] = `
+"<SomeHighlyConfiguredComponent
+  onEnter={this.onEnter}
+  onLeave={this.onLeave}
+  onChange={this.onChange}
+  initialValue={this.state.initialValue}
+  ignoreStuff={true}
+>
+  <div>and the children go here</div>
+  <div>and here too</div>
+</SomeHighlyConfiguredComponent>
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+<SomeHighlyConfiguredComponent
+  onEnter={this.onEnter}
+  onLeave={this.onLeave}
+  onChange={this.onChange}
+  initialValue={this.state.initialValue}
+  ignoreStuff={true}
+>
+  <div>and the children go here</div>
+  <div>and here too</div>
+</SomeHighlyConfiguredComponent>;
+"
+`;
diff --git a/tests/jsx_last_line/jsfmt.spec.js b/tests/jsx_last_line/jsfmt.spec.js
new file mode 100644
index 000000000..0cac44995
--- /dev/null
+++ b/tests/jsx_last_line/jsfmt.spec.js
@@ -0,0 +1,2 @@
+run_spec(__dirname, {jsxBracketSameLine: true});
+run_spec(__dirname, {jsxBracketSameLine: false});
diff --git a/tests/jsx_last_line/last_line.js b/tests/jsx_last_line/last_line.js
new file mode 100644
index 000000000..2417fa94b
--- /dev/null
+++ b/tests/jsx_last_line/last_line.js
@@ -0,0 +1,10 @@
+<SomeHighlyConfiguredComponent
+  onEnter={this.onEnter}
+  onLeave={this.onLeave}
+  onChange={this.onChange}
+  initialValue={this.state.initialValue}
+  ignoreStuff={true}
+>
+  <div>and the children go here</div>
+  <div>and here too</div>
+</SomeHighlyConfiguredComponent>