[RFC] Always expand objects if contained in another object or array

The flow tests look really bad with it but the more real world tests feel much better. I think we'll have to run that on a real codebase and see how it feels. Fixes prettier#74
vjeux · Jan 26, 2017 · faf46e5 · faf46e5
1 parent 6e68f74
commit faf46e5
Show file tree

Hide file tree

Showing 11 changed files with 312 additions and 53 deletions.
diff --git a/src/printer.js b/src/printer.js
@@ -543,7 +543,7 @@ function genericPrintNoParens(path, options, print) {
     case "ObjectExpression":
     case "ObjectPattern":
     case "ObjectTypeAnnotation":
-      var allowBreak = false;
+      var parent = path.getParentNode();
       var isTypeAnnotation = n.type === "ObjectTypeAnnotation";
       // Leave this here because we *might* want to make this
       // configurable later -- flow accepts ";" for type separators
@@ -570,6 +570,13 @@ function genericPrintNoParens(path, options, print) {
         );
       });
 
+      // We want to always expand an object if it's contained inside of
+      // another object or array
+      const shouldForceBreak = parent.type === "ObjectProperty" ||
+        parent.type === "ArrayExpression";
+
+      const lineType = shouldForceBreak ? hardline : (options.bracketSpacing ? line : softline);
+
       if (props.length === 0) {
         return "{}";
       } else {
@@ -579,7 +586,7 @@ function genericPrintNoParens(path, options, print) {
             indent(
               options.tabWidth,
               concat([
-                options.bracketSpacing ? line : softline,
+                lineType,
                 join(concat([ separator, line ]), props)
               ])
             ),

diff --git a/tests/flow/arrays/__snapshots__/jsfmt.spec.js.snap b/tests/flow/arrays/__snapshots__/jsfmt.spec.js.snap
@@ -57,23 +57,78 @@ var alittle: Array<?number> = [0, 1, 2, 3, null];
 var abig: Array<?number> = [0, 1, 2, 3, 4, 5, 6, 8, null];
 
 var abig2: Array<{ x: number, y: number }> = [
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0 },
-  { x: 0, y: 0, a: true },
-  { x: 0, y: 0, b: \"hey\" },
-  { x: 0, y: 0, c: 1 },
-  { x: 0, y: 0, c: \"hey\" }
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0
+  },
+  {
+    x: 0,
+    y: 0,
+    a: true
+  },
+  {
+    x: 0,
+    y: 0,
+    b: \"hey\"
+  },
+  {
+    x: 0,
+    y: 0,
+    c: 1
+  },
+  {
+    x: 0,
+    y: 0,
+    c: \"hey\"
+  }
 ];
 
 module.exports = \"arrays\";

diff --git a/tests/flow/destructuring/__snapshots__/jsfmt.spec.js.snap b/tests/flow/destructuring/__snapshots__/jsfmt.spec.js.snap
@@ -325,7 +325,13 @@ var cp2_err: string = others.childprop2; // Error: number ~> string
 declare var a: string;
 declare var b: string;
 declare var c: string;
-[{ a1: a, b }, c] = [{ a1: 0, b: 1 }, 2];
+[{ a1: a, b }, c] = [
+  {
+    a1: 0,
+    b: 1
+  },
+  2
+];
 
 var { m } = { m: 0 };
 ({ m } = { m: m });

diff --git a/tests/flow/logical/__snapshots__/jsfmt.spec.js.snap b/tests/flow/logical/__snapshots__/jsfmt.spec.js.snap
@@ -901,14 +901,30 @@ function logical12b(y: number): number {
  */
 function logical13(x: number): Array<{ x: string }> {
   return [
-    { x: x && \"bar\"  },
-    { x: true && \"bar\"  },
-    { x: true && false  },
-    { x: false && false  },
-    { x: 1 && \"bar\"  },
-    { x: \"foo\" && \"bar\"  },
-    { x: \"foo\" && \"bar\"  },
-    { x: \"foo\" && \"bar\"  }
+    {
+      x: x && \"bar\"
+    },
+    {
+      x: true && \"bar\"
+    },
+    {
+      x: true && false
+    },
+    {
+      x: false && false
+    },
+    {
+      x: 1 && \"bar\"
+    },
+    {
+      x: \"foo\" && \"bar\"
+    },
+    {
+      x: \"foo\" && \"bar\"
+    },
+    {
+      x: \"foo\" && \"bar\"
+    }
   ];
 }
 
@@ -917,14 +933,30 @@ function logical13(x: number): Array<{ x: string }> {
  */
 function logical14(x: number): Array<{ x: string }> {
   return [
-    { x: x || \"bar\"  },
-    { x: false || \"bar\"  },
-    { x: false || true  },
-    { x: true || false  },
-    { x: 0 || \"bar\"  },
-    { x: \"foo\" || \"bar\"  },
-    { x: \"foo\" || \"bar\"  },
-    { x: \"foo\" || \"bar\"  }
+    {
+      x: x || \"bar\"
+    },
+    {
+      x: false || \"bar\"
+    },
+    {
+      x: false || true
+    },
+    {
+      x: true || false
+    },
+    {
+      x: 0 || \"bar\"
+    },
+    {
+      x: \"foo\" || \"bar\"
+    },
+    {
+      x: \"foo\" || \"bar\"
+    },
+    {
+      x: \"foo\" || \"bar\"
+    }
   ];
 }
 

diff --git a/tests/flow/new_react/__snapshots__/jsfmt.spec.js.snap b/tests/flow/new_react/__snapshots__/jsfmt.spec.js.snap
@@ -506,12 +506,23 @@ var C = React.createClass({
   }
 });
 
-<C statistics={[{}, { label: \"\", value: undefined }]} />;
+<C
+  statistics={[
+    {},
+    {
+      label: \"\",
+      value: undefined
+    }
+  ]}
+/>;
 
 // error (label is required, value not required)
 var props: Array<{ label: string, value?: number }> = [
   {},
-  { label: \"\", value: undefined }
+  {
+    label: \"\",
+    value: undefined
+  }
 ]; // error (same as ^)
 "
 `;

diff --git a/tests/flow/object_assign/__snapshots__/jsfmt.spec.js.snap b/tests/flow/object_assign/__snapshots__/jsfmt.spec.js.snap
@@ -105,11 +105,26 @@ exports[`test apply.js 1`] = `
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // @flow
 
-(Object.assign.apply(null, [{}, { a: 1 }, { b: \"foo\" }]): {
-  a: number,
-  b: string
-});
-(Object.assign({}, ...[{ a: 1 }, { b: \"foo\" }]): { a: number, b: string });
+(Object.assign.apply(null, [
+  {},
+  {
+    a: 1
+  },
+  {
+    b: \"foo\"
+  }
+]): { a: number, b: string });
+(Object.assign(
+  {},
+  ...[
+    {
+      a: 1
+    },
+    {
+      b: \"foo\"
+    }
+  ]
+): { a: number, b: string });
 "
 `;
 

diff --git a/tests/flow/union/__snapshots__/jsfmt.spec.js.snap b/tests/flow/union/__snapshots__/jsfmt.spec.js.snap
@@ -61,14 +61,38 @@ var l: Array<T> = [
 type T = { type: \"a\", a: number } | { type: \"b\", b: string };
 
 var l: Array<T> = [
-  { type: \"a\", a: 1 },
-  { type: \"a\", a: 2 },
-  { type: \"a\", a: 3 },
-  { type: \"a\", a: 4 },
-  { type: \"b\", b: \"monkey\" },
-  { type: \"b\", b: \"gorilla\" },
-  { type: \"b\", b: \"giraffe\" },
-  { type: \"b\", b: \"penguin\" }
+  {
+    type: \"a\",
+    a: 1
+  },
+  {
+    type: \"a\",
+    a: 2
+  },
+  {
+    type: \"a\",
+    a: 3
+  },
+  {
+    type: \"a\",
+    a: 4
+  },
+  {
+    type: \"b\",
+    b: \"monkey\"
+  },
+  {
+    type: \"b\",
+    b: \"gorilla\"
+  },
+  {
+    type: \"b\",
+    b: \"giraffe\"
+  },
+  {
+    type: \"b\",
+    b: \"penguin\"
+  }
 ];
 "
 `;

diff --git a/tests/flow/union_new/__snapshots__/jsfmt.spec.js.snap b/tests/flow/union_new/__snapshots__/jsfmt.spec.js.snap
@@ -118,8 +118,13 @@ var bar: Array<{b: ?boolean, c: number} | {b: boolean}> = [
 /* @flow */
 
 var bar: Array<{ b: ?boolean, c: number } | { b: boolean }> = [
-  { b: true, c: 123 },
-  { b: true }
+  {
+    b: true,
+    c: 123
+  },
+  {
+    b: true
+  }
 ];
 "
 `;