Exact object shapes lost after refinement

[Andarist]

When inspecting this repro's "bottom gutter" we can see that:

• on the 23th line it has a type of {|type: 'push'|} | {|request_id: string, type: 'response'|} | {|request_id: string, type: 'push'|} - all have exact shapes
• in the refined block on the 25th line the exactness seems to be lost and the inspected type is {request_id: string, type: 'response'}
• AFTER the refining if statement, so basically in other case on the 29th line it stays inexact - {type: 'push'} | {request_id: string, type: 'push'}

[AugustinLF]

I feel that it's an issue with the type-at-pos (or the flow try website), and not the refinements. As you can see with my example, the "inexact type" can flow in the exact one , while this example shows that flow errors on that (if I'm not mistaking).

[Andarist]

@AugustinLF Thanks for the quick answer.

As to your second example - shouldn't it be possible though? Imho if the inexact shape and exact one have the same defined properties - it should be possible to cast them, but that would ofc 'lose' some information for the rest of the program. However if the lost information is that it potentially could have other properties too and now Im assuring the type checker that it wont have - that's fine by me.

While in fact, in the first snippet given by you, it seems that the exact object shape might not be lost there, it seems that the second if statement is still not able to refine things correctly here. When inspecting message inside that if statement we can see in the gutter - {type: 'push'} | {request_id: string, type: 'push'}

[bradennapier]

I think there are a few things going on here to consider.

Flow is only showing you one of the types that are possible (the least specific). While it is "aware" that the type can be the exact version, it is possible that this function could be called from some outside source that Flow can not type check -- so it is showing you what it knows without a doubt is the "shape" of the object.

So as @AugustinLF pointed out, it does actually correctly type the exactness of the object when you use it:

Try Example

---

That being said, there are definitely some bugs as well which have shown themselves in quite a few ways but all appear to be related.

In @Andarist example - the issue that striked me as the biggest issue was that the last function call could not be inferred properly as being nothign but a push message. Flow is simply not aware of what happened before it and if that caused a short-circuit of the call.

We can see that refinement does not do anything at all with Unions. While you woudl think that each refinement would remove the matching values from the possible values, it actually does not.

---

typeof check does not validate in objects

So our first "bug" that this highlights is the fact that the typeof check does not actually qualify anything at all as it would seem. (typeof val.id === 'string' !== {| id: string |}) :

// $Works
  if (val.id && typeof val.id === 'string') {
    two(val);
    return;
  }
  // $ExpectError
  if (typeof val.id === 'string') {
    two(val);
  }

typeof check bug example

---

refinement can not understand function interruption

The other bug that @Andarist shows (and the one that gets me a bit more than the others with his example is that refinement is not inferred later in the function. In the following example, even though we have refined out the other two possible combinations and no other possibility other than the One type exists, it is still including Two when one() is called:

export function refine(val: One | Two | Three) {
  if (val.type === 'response') {
    // $Works
    (val: Object);
    // $Works
    (val: Three);
    // $Works
    three(val);
    return;
  }
  // $Works
  if (val.id && typeof val.id === 'string') {
    two(val);
    return;
  }
  // $Error
  (val: One);

  // $Error
  (val: Two);

  // $Works
  (val: One | Two);
  /*
    property `id`
      Property not found in
      object type
  */
  one(val);

  if (val.id === undefined) {
    // $Works
    one(val);
  }
}

refinement bug example

---

There were two others that I had discovered with this but don't have the time to put together at the moment.

I also tried making all the typing much more strict with no change:

fully commented example w/ strict typing

[Andarist]

@bradennapier Thanks for putting those repros together! Great work, I've also learned a little bit from it about how to write flow repros.

I've also tried adding additional checks, wrapping it into extra functions etc, cannot make it to refine this. However I've noticed that this might be some kind of a bug with refining through 2 distinct properties or something.

const handleMessage = (message: SocketMessage) => {
  if (message.type === 'response') {
    handleResponse(message)
    return
  }

  if (message.request_id === undefined) {
    // $Works
    (message: Push);  
    handlePush(message)
    return
  }

  // $Error
  (message: PushResponse);  
  handleResponse(message)
}

but if we reverse this second refining logic, we end up with

const handleMessage = (message: SocketMessage) => {
  if (message.type === 'response') {
    handleResponse(message)
    return
  }

  if (message.request_id !== undefined) {
    // $Error  
    (message: PushResponse);  
    handleResponse(message)
    return
  }
  
  // $Works
  (message: Push);  
  handlePush(message)
}

[bradennapier]

@Andarist well you can definitely type it, it just seems to require some serious refinement.

Try Flow

---

We personally implement this type of pattern by using Flow's opaque types now that eslint-plugin-flowtype supports opaque types (for most cases). They definitely can make life easier in handling these things.

They work well with the validation handling, and by providing the opaque type there is really no chance you accidentally try to send an object that you weren't intending to.

In the case below I use flow-type-transformer as a object transformation helper but return opaque types.

Actually caught 3 bugs that weren't known about for the past 3 years by doing it too.

It's a bit more boilerplate but I can't recommend it enough.

// send function shape
// send = (data: Dash$ResponseWrapper): WebSocketSession => {}

export type Dash$ResponseWrapper =
  | Dash$SuccessWrapper
  | Dash$ErrorWrapper;

type SuccessWrapper = {|
  result: 'success',
  code: number,
  uuid: ?string,
  responses: Array<Dash$ResponseTypes>,
|};

export opaque type Dash$SuccessWrapper: SuccessWrapper = SuccessWrapper;

type ErrorWrapperShape = {|
  result: 'error',
  code: number,
  uuid: ?string,
  errors: Array<string>,
|};

export opaque type Dash$ErrorWrapper: ErrorWrapperShape = ErrorWrapperShape;

type SuccessResponse = {|
  +result: 'success',
  +code: number,
  +uuid: string,
  +path?: ?string,
  +data?: void | Route$SuccessResponseTypes,
|};

export opaque type Dash$SuccessResponse: SuccessResponse = SuccessResponse;

type ErrorResponseShape = {|
  +result: 'error',
  +code: number,
  +uuid: string,
  +path?: ?string,
  +errors: Array<string>,
|};

export opaque type Dash$ErrorResponse: ErrorResponseShape = ErrorResponseShape;

export const formatErrorResponse: $Transform<
  $Shape<{ ...Dash$ErrorResponse }>,
  Dash$ErrorResponse,
> = createTransformer(transformErrorResponse);

export const formatSuccessResponse: $Transform<
  $Shape<{ ...Dash$SuccessResponse }>,
  Dash$SuccessResponse,
> = createTransformer(transformSuccessResponse);

export const formatResponseWrapper: $Transform<
  // we accept the shape of either wrapper -- but require result is provided in minimum
  $Shape<{ ...Dash$SuccessWrapper }> | $Shape<{ ...Dash$ErrorWrapper }>,
  Dash$ResponseWrapper,
> = createTransformer(transformResponseWrapper);

[Andarist]

Hm, I've studied your flow-type-transformer and I can't say I fully understand why this is needed, I somewhat understand how it works.

In my example corrected by you and in your example given above I see you are using read only properties, do they give any extra guarantees in those situations or is just a matter of your preference?

I suppose the core of your extra refinements lies in the fact that you define non-existent prop on those shapes, just so both props are there on any of the disjoint types, even if some is always undefined and that makes flow easier to understand this situation, right?

[esturcke]

I think I'm running into this issue in this example. I wanted to handle results from a form that will either be an update if an id is set and a create otherwise. I can change the type of Create to:

type Create = {| id?: void, field: string |}

to get this to work, but I'm not sure why and if I'm missing something.

[Andarist]

I think this is somewhat buggy behaviour - flow seems to be able to refine disjoint types only when sentinel is present in all types. That's why you have to put it in the Create type, even if only to specify it as undefined.

[BlooJeans]

I just wrote my own simple repro on flow 0.65 identifying it as a bug with type-at-pos, and in testing it on tryflow, noticed it no longer happens on 0.66

tryflow

0.66 changelog includes

Improvements to type reporting services like type-at-pos and coverage thanks to a type "normalizer" rewrite. Notably, the normalizer and client services like type-at-pos now:
Make more aggressive use of type aliases, leading to more compact results,
Correctly distinguish between class types and their instance counterparts,
Report abstract type parameters themselves instead of their bounds, and
Precisely report recursive types.