Special logic for flow intersection #1018

Merged
merged 1 commit into from Mar 22, 2017

Conversation

Projects
None yet
2 participants
@vjeux
Collaborator

vjeux commented Mar 16, 2017

We started using the same logic for union and intersection but then added a special case for a & {}. It turns out that they should be handled completely differently in practice.

The heuristic i'm using is if you go from object to non-object or vis-versa, then inline, otherwise go to the next line and indent (like &&).

Fixes #864
Fixes #1017

@@ -1687,11 +1683,9 @@ type Success = {
result: string
};
-type Error =
- & {

This comment has been minimized.

@vjeux

vjeux Mar 16, 2017

Collaborator

Leading & were super weird.

@vjeux

vjeux Mar 16, 2017

Collaborator

Leading & were super weird.

@jlongster

This comment has been minimized.

Show comment
Hide comment
@jlongster

jlongster Mar 17, 2017

Member

Why inline in this special case? Just curious. Is it mainly that the leading & is weird?

Member

jlongster commented Mar 17, 2017

Why inline in this special case? Just curious. Is it mainly that the leading & is weird?

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Mar 17, 2017

Collaborator

Union are likely to be chained where each element is a case, so leading | that are aligned make sense.

For intersection, it feels a lot more like a binary operator and therefore should be put at the end and behave like && or +.

Collaborator

vjeux commented Mar 17, 2017

Union are likely to be chained where each element is a case, so leading | that are aligned make sense.

For intersection, it feels a lot more like a binary operator and therefore should be put at the end and behave like && or +.

@jlongster

This comment has been minimized.

Show comment
Hide comment
@jlongster

jlongster Mar 17, 2017

Member

But why the special heuristic to do something different when mixing objects and non-objects?

Member

jlongster commented Mar 17, 2017

But why the special heuristic to do something different when mixing objects and non-objects?

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Mar 18, 2017

Collaborator

People write

type a = x & {

};

or

type a = {

} & x;

so you want to inline them there.

But if you have two literals, you want them to break:

type a = x &
  y;
Collaborator

vjeux commented Mar 18, 2017

People write

type a = x & {

};

or

type a = {

} & x;

so you want to inline them there.

But if you have two literals, you want them to break:

type a = x &
  y;
@@ -79,7 +79,8 @@ function hasObjectMode_ok(options: DuplexStreamOptions): boolean {
* @flow
*/
-type DuplexStreamOptions = ReadableStreamOptions & WritableStreamOptions & {
+type DuplexStreamOptions = ReadableStreamOptions &
+ WritableStreamOptions & {
allowHalfOpen?: boolean,
readableObjectMode?: boolean,

This comment has been minimized.

@jlongster

jlongster Mar 20, 2017

Member

This seems slightly unfortunate. The contents of the object line up with previous expressions. Are we sure we want that?

@jlongster

jlongster Mar 20, 2017

Member

This seems slightly unfortunate. The contents of the object line up with previous expressions. Are we sure we want that?

This comment has been minimized.

@vjeux

vjeux Mar 20, 2017

Collaborator

Oh you are right, it's missing one level of indentation. Hmm, I need to think about it.

@vjeux

vjeux Mar 20, 2017

Collaborator

Oh you are right, it's missing one level of indentation. Hmm, I need to think about it.

@jlongster

This comment has been minimized.

Show comment
Hide comment
@jlongster

jlongster Mar 20, 2017

Member

I left one comment, but I'm fine merging this as we can tweak it later if needed. If you rebase you can merge it.

Member

jlongster commented Mar 20, 2017

I left one comment, but I'm fine merging this as we can tweak it later if needed. If you rebase you can merge it.

Special logic for flow intersection
We started using the same logic for union and intersection but then added a special case for `a & {}`. It turns out that they should be handled completely differently in practice.

The heuristic i'm using is if you go from object to non-object or vis-versa, then inline, otherwise go to the next line and indent (like &&).

Fixes #864
Fixes #1017

@vjeux vjeux merged commit dc0fbf7 into prettier:master Mar 22, 2017

@jlongster

This comment has been minimized.

Show comment
Hide comment
@jlongster

jlongster Mar 22, 2017

Member

Are you going to figure out a better way to print the example that wasn't indented?

Member

jlongster commented Mar 22, 2017

Are you going to figure out a better way to print the example that wasn't indented?

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Mar 22, 2017

Collaborator

Yes, I should probably file an issue for it :)

Collaborator

vjeux commented Mar 22, 2017

Yes, I should probably file an issue for it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment