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

Intersection doesn't work for exact object types #2626

Closed
philipp-spiess opened this Issue Oct 13, 2016 · 16 comments

Comments

Projects
None yet
@philipp-spiess

philipp-spiess commented Oct 13, 2016

Given the following example:

declare type Foo = {| foo: string |} & {| bar: string |}
const example: Foo = {foo: 'foo', bar: 'bar'}

I expect this to work since the intersection type should be allowed to further extend the exact object. But in the latest version, it errors.

Is there an easy way to make this work or is this intentional?

@vkurchatkin

This comment has been minimized.

Contributor

vkurchatkin commented Oct 13, 2016

If value v has type A & B that means that it has type A and has type B at the same type. If A is {| foo: string |} it means that v has exactly one property foo and if B is {| bar: string |} it means that v has exactly one property bar. Obviously, you can't satisfy these two constraints at the same time, so A & B is an empty type.

@philipp-spiess

This comment has been minimized.

philipp-spiess commented Oct 13, 2016

Thank you for the answer. Your explanation totally makes sense but I was hoping to build an exact object type using intersection somehow. The above example obviously works with a regular object type, so I expect the exact object to behave in a similar matter (so that the outcome equals declare type Foo = {| foo: string, bar: string |}.

Do you know of another why I could model this behaviour? I have two types, A and B, who define keys and values and I want to build an intersection A & B as an exact object, so that only the keys defined in either A or B are valid.

@vkurchatkin

This comment has been minimized.

Contributor

vkurchatkin commented Oct 13, 2016

I'm pretty sure there is no way do it at the moment.

@dsimmons

This comment has been minimized.

dsimmons commented Dec 15, 2016

Chiming in -- just ran into this myself.

Terminology aside, do you think "combining exact types" is a possibility at some point?

Going back to the explanation above, let's say I have an exact object type A and I want to create a new (preferably strict) type C that's a superset of A (we'll call the additional fields object type B).

Let's also say that A is used elsewhere. If I want to create this new type without duplicating all of the same fields included as part of the exact object type A, I'm forced to relax the strict/exact check.

To provide a more concrete example, I'm attempting to do the following:

// Used elsewhere as a standalone type.
export type MyStrictType = {|
  uri: string,
  name: string,
  type: string
|};

export type Action =
  // ... others
  | ({ type: 'SOME_CONST', anotherAttr: string } & MyStrictType)
;

Admittedly, in addition to relaxing the strict/exact object requirement, one way I could fix the above is by instead introducing a nested attribute of { type: 'SOME_CONST', anotherAttr: string } (which I've referred to above as object type B) rather than trying to combine the two.

That's how I've fixed "the problem" for now, because that's better than the alternative of relaxing the exact object type for MyStrictType (which allows for much better "save me from myself" Flow checking where it's used elsewhere).

I just wanted to provide a more concrete example of why it'd be desirable so that it's a little less contrived/abstract for the sake of a discussion. Mathematically, I'm not sure what the term would be for this relationship, but it'd be a welcomed addition! 😄

@vkurchatkin

This comment has been minimized.

Contributor

vkurchatkin commented Dec 15, 2016

@samwgoldman is working object type spread, I guess it should work with exact type as well:

type Foo = {| foo: string |};
type Bar = {| bar: string |};
type FooBar = { ...Foo, ...Bar };
@bb010g

This comment has been minimized.

bb010g commented Dec 16, 2016

@vkurchatkin That seems like the simplest way to express this without loosing the meaning of $Exact as something that doesn't like to be extended.

@dsimmons

This comment has been minimized.

dsimmons commented Dec 16, 2016

IMHO that'd be a much welcomed addition!

It doesn't get much simpler than that (from a user's perspective anyway)! 🎉

@samwgoldman

This comment has been minimized.

Member

samwgoldman commented Dec 16, 2016

Yeah, @vkurchatkin is exactly right on all counts. It's not possible to satisfy two different exact object types simultaneously. The root of the issue is that intersections of objects isn't a proper "merge" operation — it just behaves like that in many cases, and has become idiomatic. Proper object type spread, which I am working on, is a better solution.

@Falieson

This comment has been minimized.

Falieson commented Dec 16, 2016

Seems related to #1326

I'm very interested in this topic resolution!

@jedmao

This comment has been minimized.

jedmao commented Jan 19, 2017

@vkurchatkin, the object type spread feature is a great step forward, but it doesn't handle the perhaps more common scenario where you want to throw an error if the types aren't compatible with each other. For example, in a TypeScript interface:

interface Foo {
    baz: string;
}

interface Bar {
    baz: number;
}

interface Qux extends Foo, Bar {}

🔴 Interface 'Qux' cannot simultaneously extend types 'Foo' and 'Bar'. Named property 'baz' of types 'Foo' and 'Bar' are not identical. – interface Qux.

This is ideal, because it prevents unintended side effects, which is kind of the whole point of a type system.

Personally, I would prefer that strict object types are the default Flow behavior #3214 and Excess Property Checks are used to achieve Flow's current default behavior.

@adamjernst

This comment has been minimized.

Contributor

adamjernst commented Feb 9, 2017

@jedmao's concern is definitely relevant to my plan for using this feature (if object spread for types were the way to solve it).

@vkurchatkin

This comment has been minimized.

Contributor

vkurchatkin commented Mar 29, 2017

Closing. Now you can do this:

declare type Foo = {| ...{| foo: string |}, ...{| bar: string |} |}
const example: Foo = {foo: 'foo', bar: 'bar'}
@dperetti

This comment has been minimized.

dperetti commented Sep 4, 2017

However, this still doesn't:

type One = {| n: number |}
type Two = {| bar: string |}
type Foo = {| ...One, ...Two |}

const one: One = {n: 1}
const two: Two = {bar: 'bar'}
const example_OK: Foo = {n: 1, bar: 'bar'}
const example_NOT_OK: Foo = {...one, ...two}

marudor added a commit to flow-typed/flow-typed that referenced this issue Sep 8, 2017

React router flow@0.53.x fixes (#1214)
* Specify `ContextRouter` as an exact object

Our type signature should not assume that additional keys may be present in this object.

* withRouter uses type spread notation, instead of intersection

intersection of inexact object types behaves much like spreading, but when both objects are exact, the intersection type becomes an empty set.

See: facebook/flow#2626

* Upgrade babel-eslint version in definitions/ to match cli

* Tests use spread notation instead of intersection
@AlexandreBossard

This comment has been minimized.

AlexandreBossard commented Oct 23, 2017

Hi,
I did not find this awesome feature in the doc ? Did i miss it ?

@jcready

This comment has been minimized.

jcready commented Oct 23, 2017

@AlexandreBossard no you did not miss it. It is simply undocumented.

mwiencek added a commit to mwiencek/musicbrainz-server that referenced this issue Mar 17, 2018

Use object type spreads instead of intersections
The previous code is wrong because an object can't simultaneously
satisfy two distinct, exact object types.

The correct way is using object type spreads as described here:
facebook/flow#2626

I'm not sure why things currently type-check.

mwiencek added a commit to mwiencek/musicbrainz-server that referenced this issue Mar 17, 2018

Use object type spreads instead of intersections
The previous code is wrong because an object can't simultaneously
satisfy two distinct, exact object types.

The correct way is using object type spreads as described here:
facebook/flow#2626

I'm not sure why things currently type-check.

mattgstevens added a commit to mattgstevens/flow that referenced this issue Apr 11, 2018

Usage of object spread for exact object types
Came across this issue: facebook#2626 and decided it could be in the documentation :)

mattgstevens added a commit to mattgstevens/flow that referenced this issue Apr 11, 2018

Usage of object spread for exact object types
Came across this issue: facebook#2626 and decided it could be in the documentation :)

mattgstevens added a commit to mattgstevens/flow that referenced this issue Jun 26, 2018

Usage of object spread for exact object types
Came across this issue: facebook#2626 and decided it could be in the documentation :)
@gajus

This comment has been minimized.

gajus commented Jun 26, 2018

I think I am hitting the same issue, @matthewjohnston4.

#6526

facebook-github-bot added a commit that referenced this issue Jul 9, 2018

[PR] Usage of object spread for exact object types
Summary:
Came across this issue: #2626 and decided it could be in the documentation :)
Closes #6124

Reviewed By: gabelevi

Differential Revision: D8648216

Pulled By: fishythefish

fbshipit-source-id: f6428880f6d8d3bff942cac13c83b4d8b4fb47d7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment