Skip to content
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

structural compatibility of union types / mutable union #2647

Closed
Swatinem opened this issue Oct 18, 2016 · 10 comments
Closed

structural compatibility of union types / mutable union #2647

Swatinem opened this issue Oct 18, 2016 · 10 comments

Comments

@Swatinem
Copy link

Consider the following code:

declare var a: {type: "a" | "b"};
const c: {type: "a"} | {type: "b"} = a;

Flow says those types are not compatible with one another

    2: const c: {type: "a"} | {type: "b"} = a;
                                            ^ object type. This type is incompatible with
    2: const c: {type: "a"} | {type: "b"} = a;
                ^ union: object type(s)

While flow is right to say so, since the declared var is not compatible with either one of the variants. But it is compatible with both of them.

What I want to do is the following:

I want to have a union type that is mutable at runtime. I tried to do that via a class whos properties are the sum of all of the unions variants properties. But flow always says it is not compatible.

To elaborate further, consider this example code:

type NotLoaded = {
    loaded: false;
};
type Loaded = {
    loaded: true;
    content: string;
};

class Loader {
    loaded: boolean; // or `true | false` if you wanna be pedantic
    content: string;
}

The problem is that I absolute need internal mutability / referencial integrity, via the class. But I also want to be typesafe on the outside, via the union type.

@vkurchatkin
Copy link
Contributor

The problem is that I absolute need internal mutability / referencial integrity, via the class. But I also want to be typesafe on the outside, via the union type.

These properties are kind of mutually exclusive. If you want mutability then type refinement is not possible.

@Swatinem
Copy link
Author

Swatinem commented Nov 9, 2016

These properties are kind of mutually exclusive. If you want mutability then type refinement is not possible.

Why would that be? Other languages with proper language-level unions/enums can also have internal mutability.

@vkurchatkin
Copy link
Contributor

@Swatinem any example?

@Swatinem
Copy link
Author

Swatinem commented Nov 9, 2016

For example in rust:

#[derive(Debug)]
enum U {
    A(u32),
    B(bool),
}

fn main() {
    let mut u = U::A(42);
    std::mem::replace(&mut u, U::B(true));
    println!("{:?}", u);
}

Ok, provided that rusts borrowck would complain if you had any references to that object, but there are also ways to deal with that.

My point is that at least for the example in comment#1, the class properties are compatible with every one of the union variants, so mutating the discriminant should still yield a valid type. It does so at runtime, thats for sure.

The problem here is more of what the programmers intent is. I could just use the plain class and access .content without a problem. But I want to add discriminant to say "hey, .content is not valid unless .loaded is true."

Like I said, I could just use a plain class, but that would not be sound anyway since flow does not enforce that properties are actually initialized, so .content might be null anyway and throw at runtime.

@vkurchatkin
Copy link
Contributor

I see. It probably should be possible if all properties in the union are covariant. One caveat though is that flow should invalidate refinement after function calls, because they can potentially mutate the type.

To be fair, your example seems to be very specific to Rust.

@Swatinem
Copy link
Author

Ok, then let me elaborate more on a js example:

type NotLoaded = {
    loaded: false;
    load: () => void;
};
type Loaded = {
    loaded: true;
    load: () => void;
    content: string;
};
type ILoader = NotLoaded | Loaded;

class Loader {
    loaded: boolean; // or `true | false` if you wanna be pedantic
    content: string;

    constructor() {
        this.loaded = false;
    }

    load() {
        this.content = "foobar";
        this.loaded = true;
    }
}

const l = new Loader();

l.content.length; // this will not warn at compile time but throw at runtime, because of #650
// which is a flaw in flow anyway, but lets ignore that for now. In practice you should define the prop anyway because of hidden class optimizations.

const il: ILoader = new Loader(); // <- this will warn at compile time, what this issue is about.

il.content.length; // this will warn at compile time and is exactly what I want!

il.load();
if (il.loaded) {
    il.content.length; // I want this to only be valid inside the branch checking for `.loaded`!
}

@vkurchatkin
Copy link
Contributor

Right now when you assign a value to a union type, it needs to match at least one branch. I'm not sure that it's going to change

@vkurchatkin
Copy link
Contributor

BTW, here is your Rust example in Flow:

type MutT<T> = { val: T };
function Mut<T>(val: T): MutT<T> {
  return { val };
}

function replace<T>(mut: MutT<T>, val: T) {
  mut.val = val;
}

type U =
  | { t: 'A', v: number }
  | { t: 'B', v: bool }
;

function A(v: number): U {
  return { t: 'A', v };
}

function B(v: bool): U {
  return { t: 'B', v };
}

const u = Mut(A(42));
replace(u, B(true));

@aij
Copy link

aij commented Sep 17, 2018

Is this actually a feature request, or a question?

Making {type: "a" | "b"} compatible with {type: "a"} | {type: "b"} would be unsound in light of mutability and refinement. (You could end up with the value {type: "a"} having type {type: "b"})

@Swatinem
Copy link
Author

Closing, since I am not interested in a solution here anymore.

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

No branches or pull requests

3 participants