New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tagged unions #1664

Closed
coot opened this Issue Apr 14, 2016 · 5 comments

Comments

Projects
None yet
4 participants
@coot
Copy link
Contributor

coot commented Apr 14, 2016

The following code should type check while it does not:

/* @flow */

declare interface DataBase {
  id: string,
  name: string,
};

declare interface UserData extends DataBase {
  kind: "user",
}

declare interface SystemData extends DataBase {
  kind: "system",
}

declare type Data = UserData | SystemData;

const data: Data = {
  id: "",
  name: "",
  kind: "system",
}

if (data.kind == "system") {
  let system: SystemData = data;
}

This fails to typecheck with the following error (using flow v0.23.0):

 15:   kind: "system",
             ^^^^^^^^ string literal `system`. Expected string literal `user`, got `system` instead
 11:   kind: "user",
             ^^^^^^ string literal `user`
@7ynk3r

This comment has been minimized.

Copy link

7ynk3r commented Apr 18, 2016

Same here. For some reason is making an intersection when reading the properties.

// @flow

export type Action =
    { type: 'A', a: string }
  | { type: 'B', b: string }
  ;

const a0: Action = {type: 'A', a:'a'}
const b0: Action = {type: 'B', b:'b'}
const c0: Action = {type: 'A', b:'b'} // error: this is right
const d0: Action = {type: 'B', a:'a'} // error: this is right


console.log(a0.a); // error: this is NOT right!
const {type, a} = a0; // error: this is NOT right!

console.log(b0.b); // error: this is NOT right!
const {type, b} = b0; // error: this is NOT right!
@avikchaudhuri

This comment has been minimized.

Copy link
Contributor

avikchaudhuri commented Jun 1, 2016

@7ynk3r Couldn't see the connection between your example and the original issue, but looking at it all the errors look legit. You cannot access a property on an union of object types unless the property exists on all branches: otherwise, it is easy to see that we'd miss catching runtime errors.

@coot: If you (1) rewrite your example using object types and intersections instead of interfaces, and (2) use === instead of ==, then you fall into supported territory for disjoint union types, and you have a legit issue that will be fixed in an upcoming release.

Disjoint unions should be made to work for interfaces as well, but that is a separate issue. Meanwhile, the use of === is mandatory, since == implies too many implicit equalities that we don't model.

@avikchaudhuri

This comment has been minimized.

Copy link
Contributor

avikchaudhuri commented Jun 1, 2016

@coot: Here's a rewrite that will work in an upcoming release:

/* @flow */

type DataBase = {
  id: string,
  name: string,
};

type UserData = DataBase & {
  kind: "user",
};

type SystemData = DataBase & {
  kind: "system",
}

type Data = UserData | SystemData;

const data: Data = {
  id: "",
  name: "",
  kind: "system",
}

if (data.kind === "system") {
  (data: SystemData);
}
@avikchaudhuri

This comment has been minimized.

Copy link
Contributor

avikchaudhuri commented Jun 1, 2016

Beyond the immediate union/intersection type issue, there's an incompleteness issue that falls into a similar bucket of work as #1569.

@coot

This comment has been minimized.

Copy link
Contributor

coot commented Jun 23, 2016

@avikchaudhuri using your last example, I am getting error (flow 0.27.0):

 25:   (data: SystemData);
        ^^^^ intersection. This type is incompatible with
 12: type SystemData = DataBase & {
                                  ^ object type
  Member 1:
    3: type DataBase = {
                       ^ object type
  Error:
   12: type SystemData = DataBase & {
                                    ^ property `kind`. Property not found in
    3: type DataBase = {
                       ^ object type
  Member 2:
    8: type UserData = DataBase & {
                                  ^ object type
  Error:
    9:   kind: "user",
               ^^^^^^ string literal `user`. Expected string literal `system`, got `user` instead
   13:   kind: "system",
               ^^^^^^^^ string literal `system`

ghost pushed a commit that referenced this issue Jul 7, 2016

improve support for tagged unions of interfaces
Summary:
Treating flows to structural interface types similarly to flows to object types.

Fixing `sentinel_prop_test` seems to fix issue:

#1664

Does not yet handle class intersections.

Reviewed By: avikchaudhuri

Differential Revision: D3527224

fbshipit-source-id: 072ffbee78b94518f69fbfa5b788c272e9507510

This issue was closed.

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