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

width subtyping with nonexact known properties fails in idiomatic javascript #6146

Open
villesau opened this issue Apr 16, 2018 · 9 comments

Comments

@villesau
Copy link
Contributor

See in try flow

Documentation mentions width-subtyping, caveats and how to use exact types to be able to be more precise: https://flow.org/en/docs/lang/width-subtyping/

However, there are cases where all the possible types are known even when the ojbects are not exact.

As you can see, both object properties defines property selector. This means that the property can be either true or false and based on that props has at least bunch of properties. If it's true, we know that property a will be number, not string etc.

Anyhow this is not working in flow which is major defect since the code pasted in try flow is pretty idiomatic way of using Javascript, and as far as I know, there can't be uncertainty of defined types which means it should be completely possible for Flow to type check the above code properly.

PS. the above example does not work with exact types either.

@apsavin
Copy link
Contributor

apsavin commented Apr 16, 2018

Here the fixed example.

Issues with destructuring of unions are alredy reported: https://github.com/facebook/flow/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+union+destructuring

@villesau
Copy link
Contributor Author

villesau commented Apr 16, 2018

@apsavin that's not logically the same. In your example, selector is passed into sub components when in the original example it's parsed away from the props. I can live with that, but it's suboptimal as I need to change the code in order to make it work with Flow.

E: Interesting examples:
This works but passes also selector: example

But when spreading the object, it stops working: example

@apsavin
Copy link
Contributor

apsavin commented Apr 16, 2018

it's suboptimal as I need to change the code in order to make it work with Flow.

Agree, I hope it will be fixed with the ability to use destructuring of unions.

@villesau
Copy link
Contributor Author

This is actually very broken with exact types too: Try Flow

There might be duplicate of this issue though.

@villesau
Copy link
Contributor Author

villesau commented Nov 15, 2018

Here are bunch of related issues or duplicates:

#6805
#6594
#6408
#5745
#4772
#3932
#5461

Could e.g @mrkev or @jbrown215 comment on wether this could be prioritized? Looks like there are quite a many developers struggling with this.

@jbrown215
Copy link
Contributor

cc @samwgoldman, who has a lot of changes to object types in the works.

@samwgoldman
Copy link
Member

There are a few issues here, one is a know bug with destructuring unions of objects. Here is a working version:

import * as React from 'react'

type Sub1Props = {| a: number, b: number |};
type Sub2Props = {| a: string, c: string |};

const Sub1 = (props: Sub1Props) => <div />;
const Sub2 = (props: Sub2Props) => <div />;

const Com = (props: {| selector: true, ...Sub1Props |} | {| selector: false, ...Sub2Props |}) => {
  if (props.selector) {
    let {selector, ...rest} = props;
    return <Sub1 {...rest} />;
  } else {
    let {selector, ...rest} = props;
    return <Sub2 {...rest} />;
  }
}

Once you destructure selector and props into separate variables, you can't refine props by doing a check on selector—the relation between these variables is not tracked by Flow. Instead, you need to keep the union together, discriminate between the arms of the union as I've done above, then you can destructure.

@villesau
Copy link
Contributor Author

Thanks a lot! Definitely sounds very reasonable, but it's also surprising if you don't know exactly how Flow internals works. That's probably the reason for the confusion that's caused so many bug reports.

@shanshanzhu
Copy link

@samwgoldman The solution is not ideal, especially if we wanna use flowtype to represent the proto3 oneof field. Is there any plan to improve it?
https://developers.google.com/protocol-buffers/docs/reference/proto3-spec#oneof_and_oneof_field

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

6 participants