Flow: Flow doesn't recognize this disjoint union refinement

[mruzekw]

Flow doesn't seem to recognize this refinement.

export type A = {
  id: string;
  name: string;

  legacyId?: number;
};

export type B = {
  id: number;
  title: string;
};

export type unionType = A | B;

function normalizeToA(item: unionType): A {
    if (typeof item.id === 'string') {
        return { id: item.id, legacyId: item.legacyId, name: item.name };
    } else {
        return { id: '', legacyId: item.id, name: item.title };
    }
}

flow.org/try here

[asolove]

There are already a number of issues reporting similar cases that boil down to the same thing.

For now, when I need to distinguish different cases in a union, I add a clearly distinguishing feature, often a type tag with a string literal value. This is also helpful for debugging and logging, since at run-time the Flow types aren't available. So something like this does work:

export type A = {
  id: string;
  name: string;
  type: "A",

  legacyId?: number;
};

export type B = {
  id: number;
  title: string;
  type: "B"
};

export type unionType = A | B;

function normalizeToA(item: unionType): A {
    if (item.type === "A") {
        return { id: item.id, legacyId: item.legacyId, name: item.name, type: "A" };
    } else {
        return { id: '', legacyId: item.id, name: item.title, type: "A" };
    }
}

In the mean time, since this issue is a duplicate of other similar issues, I think we can close it.

[lll000111]

As @asolove says. I would not say this is a bug but a feature request to extend how disjoint unions operate. You are supposed to have _a concrete value_ that identifies a given type that is part of the disjoint union, not a type.

Furthermore, personally I would consider a union of types where a property of the same name has different types an anti-pattern, especially if that is the property you use to create the disjoint union (won't matter if it's any other property).

One thing I learned the hard way is that Flow is not meant to support whatever you would do with "just Javascript". It is meant to also change how you code, to write in a different way that you would not if you didn't have Flow. Sometimes there are constructs we _know_ "this works perfectly well, why doesn't Flow support it?" - but that's a misunderstanding of Flow. If you use Flow the code you write needs to adapt in a lot of cases. You cannot make _any_ valid Javascript code "Flow code" and that is as intended.

PS: I think this issue is quite instructive and again @asolove has a good answer: https://github.com/facebook/flow/issues/4683

[jcready]

You are supposed to have a concrete value that identifies a given type that is part of the disjoint union, not a type.

This is not specified in the docs.

[mruzekw]

Thanks for responding everyone.

It would be nice to have a specific note in the docs about the need for a concrete value. That wasn't apparent to me at the first glance, but looking at it now I see the requirement.

[pulli23]

@lll000111 I "agree" that is is an anti-pattern, however we are dealing with an immense chain of libraries on npm. Many of which use this as a "feature".

Take for example "sails": the ORM loads data. If the data contains a reference to another table, the "value" is an integer - the id of the other data.

However one can also call "populate(propertyname)", in which case the property is replace with the actual referred object.

[goodmind]

I'm not sure if this possible, then every key would be discriminant property on unions.
Maybe some new syntax to manually mark discriminant property?

type User = $DiscriminateBy<
    | { id: string; name: string }
    | { id: number; title: string },
    'id'
>

function unwrap<T>(result: User) {
    if (typeof result.id === 'string') {
        // Here 'name' is string
        return result.name
    }

    // Now 'title' is string
    return result.title;
}

[leplatrem]

About @asolove example, note that using triple equals is necessary for the type check to work (ie. if you do if (item.type == "A") {, it won't be enough to distinguish cases)