Add ability for flow generics to break #1041

Merged
merged 1 commit into from Mar 20, 2017

Conversation

Projects
None yet
3 participants
@vjeux
Collaborator

vjeux commented Mar 19, 2017

While looking at an instability on the React codebase ( sebmarkbage/react@ba1299a#diff-2550aa3d377452ae29361f5e53c51c10 ), I realized that we don't let them break which makes the code look weird.

So I added the ability for them to break :)

Add ability for flow generics to break
While looking at an instability on the React codebase ( sebmarkbage/react@ba1299a#diff-2550aa3d377452ae29361f5e53c51c10 ), I realized that we don't let them break which makes the code look weird.

So I added the ability for them to break :)
+ case "TypeParameterInstantiation": {
+ const shouldInline =
+ n.params.length === 1 &&
+ n.params[0].type === "ObjectTypeAnnotation";

This comment has been minimized.

@vjeux

vjeux Mar 19, 2017

Collaborator

It looks better when they hug ("deleted" one here)

 function isActive(
   ad: {
-    state: $Keys<{
-      PAUSED: string,
-      ACTIVE: string,
-      DELETED: string
-    }>
+    state: $Keys<
+      {
+        PAUSED: string,
+        ACTIVE: string,
+        DELETED: string
+      }
+    >
   }
 ): boolean {
   return ad.state === "ACTIVE";
 }
@vjeux

vjeux Mar 19, 2017

Collaborator

It looks better when they hug ("deleted" one here)

 function isActive(
   ad: {
-    state: $Keys<{
-      PAUSED: string,
-      ACTIVE: string,
-      DELETED: string
-    }>
+    state: $Keys<
+      {
+        PAUSED: string,
+        ACTIVE: string,
+        DELETED: string
+      }
+    >
   }
 ): boolean {
   return ad.state === "ACTIVE";
 }
+ Props,
+ Config: $Diff<Props, DefaultProps>,
+ C: $React.Component<DefaultProps, Props, any>
+> = $React.Element<Config>;

This comment has been minimized.

@vjeux

vjeux Mar 19, 2017

Collaborator

<3

@vjeux

vjeux Mar 19, 2017

Collaborator

<3

@@ -1559,7 +1559,7 @@ function genericPrintNoParens(path, options, print) {
);
}
- return concat(parts);
+ return group(concat(parts));

This comment has been minimized.

@vjeux

vjeux Mar 19, 2017

Collaborator

By moving the group one level outside, it now first breaks on the arguments and then on the generics. Without this change, it broke a lot of tests with results like this one ("deleted" is the behavior we now have):

-declare function my_filter<T, P: $Pred<1>>(
-  v: Array<T>,
-  cb: P
-): Array<$Refine<T, P, 1>>;
+declare function my_filter<T, P: $Pred<1>>(v: Array<T>, cb: P): Array<
+  $Refine<T, P, 1>
+>;
@vjeux

vjeux Mar 19, 2017

Collaborator

By moving the group one level outside, it now first breaks on the arguments and then on the generics. Without this change, it broke a lot of tests with results like this one ("deleted" is the behavior we now have):

-declare function my_filter<T, P: $Pred<1>>(
-  v: Array<T>,
-  cb: P
-): Array<$Refine<T, P, 1>>;
+declare function my_filter<T, P: $Pred<1>>(v: Array<T>, cb: P): Array<
+  $Refine<T, P, 1>
+>;
+ T: (a: A, b: B, c: C, d: D, e: E, f: F) => G // eslint-disable-line space-before-function-paren
+ >(method: T, scope: any, a: A, b: B, c: C, d: D, e: E, f: F): G {}
+};
+"

This comment has been minimized.

@vjeux

vjeux Mar 19, 2017

Collaborator

Used to be printed

var X = {
  perform: function<A, B, C, D, E, F, G, T: (
    a: A,
    b: B,
    c: C,
    d: D,
    e: E,
    f: F
  ) => G>(method: T, scope: any, a: A, b: B, c: C, d: D, e: E, f: F): G {} // eslint-disable-line space-before-function-paren
};
@vjeux

vjeux Mar 19, 2017

Collaborator

Used to be printed

var X = {
  perform: function<A, B, C, D, E, F, G, T: (
    a: A,
    b: B,
    c: C,
    d: D,
    e: E,
    f: F
  ) => G>(method: T, scope: any, a: A, b: B, c: C, d: D, e: E, f: F): G {} // eslint-disable-line space-before-function-paren
};

@Pajn Pajn referenced this pull request Mar 19, 2017

Closed

TypeScript Support #13

@jlongster

This comment has been minimized.

Show comment
Hide comment
@jlongster

jlongster Mar 20, 2017

Member

Sounds good to me!

Member

jlongster commented Mar 20, 2017

Sounds good to me!

@jlongster jlongster merged commit a502bf9 into prettier:master Mar 20, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment