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

Object type spread #1326

Closed
samwgoldman opened this issue Jan 27, 2016 · 27 comments
Closed

Object type spread #1326

samwgoldman opened this issue Jan 27, 2016 · 27 comments
Assignees

Comments

@samwgoldman
Copy link
Member

It would be nice to be able to spread an existing object type into another one:

type A = {
  foo: string,
  bar: number,
};
type B = {
  bar: boolean,
};
type C = {
  ...A,
  ...B,
};

In the above, C would resolve to { foo: string, bar: boolean }.

This is different from intersection types. With intersection types, A & B becomes something more like { foo: string, bar: number & boolean }, which the observant reader will notice is uninhabitable.

@STRML
Copy link
Contributor

STRML commented Feb 3, 2016

This would be incredibly useful to us, we extend types all the time just for arguments. This is really nice if a function has defaults, but then we want all functions called by that to actually have a proper object with all missing attributes filled in. A simple use case:

type OptionsInput = {
  a: number,
  b: ?string,
  c: Date
};

type FnOptions = {
  ...OptionsInput,
  b: string
};

function foo(opts: OptionsInput) {
  if (!opts.b) opts.b = 'some default';
  doSomethingWith(opts);
}

@vkurchatkin
Copy link
Contributor

@STRML in this case intersection should probably work

@STRML
Copy link
Contributor

STRML commented Feb 3, 2016

@vkurchatkin It doesn't. For example, I'm intersecting such an optional property with a required property:

 16:   keys?: Array<string>,
              ^^^^^^^^^^^^^ undefined. This type is incompatible with
 22:   keys: Array<string>,
             ^^^^^^^^^^^^^ array type

For the record, this simply failed without warning in 0.20.1 - appears to have been coercing to any.

@pixelprizm
Copy link

Another thought: Even in cases where intersection would work, the spread syntax would enable an easier-to-read shorthand.

For example, to include common types in a disjoint union:
Currently, with intersections:

type Todo = {
  id: string,
  text: string,
};

type Action =
  | (
    {
      type: 'CREATE_TODO',
    } & Todo
  )
  | {
    type: 'DELETE_TODO',
    id: string,
  }
;

With spread:

type Todo = {
  id: string,
  text: string,
};

type Action =
  | {
    type: 'CREATE_TODO',
    ...Todo,
  }
  | {
    type: 'DELETE_TODO',
    id: string,
  }
;

@STRML
Copy link
Contributor

STRML commented Sep 21, 2016

Any status on this? This would be a huge help in defining so many types.

@kamek-pf
Copy link

kamek-pf commented Nov 4, 2016

This would be useful to type database schemas.

Depending on how you perform queries, a field that represent a relationship with another table / document may return an id or an object (if you're asking for a join).

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

type Campaign = {
    id: string,
    name: string,
    client: Client | string // <- this
};

This works, but forces you to type check the client field at run time. It feels awkward because you already know what this is going to be depending on what your query looks like.
I'd rather handle that sort of things on a case-by-case basis, something like that :

export type Campaign = {
    id: string,
    name: string,
    client: string
};

// somewhere else in the code, when querying Campaigns and populating the client field
import type { Client } from '.....';
import type { Campaign as CampaignBase } from '....';
type Campaign = {
    ...CampaignBase,
    client: Client
};

Would love to hear how you guys handle database / ORM stuff.

@guigrpa
Copy link

guigrpa commented Jan 10, 2017

Any update on this? It would be very useful to handle the very common use case described by @STRML

@mtford90
Copy link

👍

@conartist6
Copy link

conartist6 commented Jan 18, 2017

Proposal: additional syntax: ...?, meaning declare keys of the type to be spreaded to be optional.

Here's the use case:

type DefaultProps = { hasDefault: boolean };
type RequiredProps = { callback: function };
type InternalProps = { ...DefaultProps, ...RequiredProps };
export type Props = { ...?DefaultProps, ...RequiredProps };
// Props evaluates to: { hasDefault?: boolean, callback: function}; 

class Base {
  props: InternalProps

  constructor(props :Props) {
    this.props = {
      hasDefault: false,
      ...props,
    };
  }
}

Of course while this example is simplistic, once you have a class which receives 20 named properties it is currently a major DRY violation when it becomes necessary to duplicate type declarations for all types which have defaults.

Another major advantage here is that if you define a class that extends Base, its constructor can now be well defined!

import type Props from 'Base';
class Wrapper extends Base {
  constructor(props :Props) {
    doWrapperThings();
    super(props);
  }
}

For completeness ...?? could also be defined to result in {hasDefault?: ?boolean}

@conartist6
Copy link

Another note on the usefulness of this feature: currently using intersection types the resultant errors can be rather indecipherable, displaying full lists of all the intersecting types. Simplified error messaging for spreads should be possible, given that a type mismatch will now be traceable to a single concrete property definition and never to something strange like an uninhabitable union, which is a problem that is fundamentally split between definitions in two locations.

@conartist6
Copy link

I'm thinking of trying to tackle the code for this. Does anyone have thoughts about what the limitations of the spread operator should be?

How should it treat a spreaded object type that has an indexer or a callable?
Current thought: It should ignore callables and indexers.

Should it be able to spread unsealed object types? What should the behavior be?
Current thought: I have no idea. Example with intersections: https://flowtype.org/try/#0C4TwDgpgBAIhBmBDArgG2ABQE4HswGcoBeKAbygAtF84k1gAuKAIxx1QkQDsoBfAbgBQggMY4u+YFCwQAjsTKCBg0JCgBJLsAhYuiVNjyEStFOkMEoAMiiqIOeNLlDBM2QDoR+1M0QiA1grwyFwiwACW4gAUAJRkym7uSAAeCgBEAKxZALRZGbl5aS4AbohYthCSUAya2rr6FsaKUFBeqD5+-kzBoRHRcaS8ADSCAPSjLSlMAOTTIy1UNAhmjLZYyBBK-EA

Should it be able to spread intersection of object and/or union of object types?
Current thought: Being able to spread intersections of objects would help people who have been making do with intersections while waiting for the type spread operator. They are after all mostly identical operations if executed on objects with fully disjoint sets of properties. If the user wants the behavior that could ostensibly result for spreading a union, they should instead spread all the constituents of the union individually.

@bgw
Copy link

bgw commented Feb 13, 2017

@conartist6, I think you can do something like {...?DefaultProps} today with

type DefaultProps = {callback: function};
type OptionalProps = $ObjMap<DefaultProps, <V>(v: V) => ?V>;

But you need to watch out for #2674.

@conartist6
Copy link

That was one of the first things I looked at, but it is actually not correct! The problem is that it is creating optional values not optional keys.

https://flowtype.org/try/#0C4TwDgpgBACgTgezAZygXigbwFBSgMwQQC4oB+ZYOASwDsBzbAXwG5tsBjAGwENlUAwggC2YKBAAewCLQAmqAEoQeHYABIhohLRnAAPADcE1WQBpYiFOaMmAfFibtO2ylGmuMezWID0tlkA

@STRML
Copy link
Contributor

STRML commented Mar 17, 2017

This seems good to close!

@nmn nmn closed this as completed Mar 20, 2017
@seveibar
Copy link

This may need to be reopened,

I don't think the current implementation is what @samwgoldman proposed- see this example

The example above shows that

type A = {
  foo: string,
  bar: number,
};
type B = {
  bar: boolean,
};
type C = {
  ...A,
  ...B,
};

Does not resolve C to { foo: string, bar: boolean }.

@STRML
Copy link
Contributor

STRML commented Apr 30, 2017

@seveibar See #3534. You can work around this by using exact types for now.

This is an indication of how unintuitive non-exact object types are, and how something like #3214 is probably a good idea.

li-kai added a commit to nusmodifications/nusmods that referenced this issue Sep 8, 2017
Due to intersection types being incredibly instrusive, we opt to ignore the typing for now. The right way would be to spread the object types, but currently the implementation is buggy. See: facebook/flow#1326
li-kai added a commit to nusmodifications/nusmods that referenced this issue Sep 11, 2017
Due to intersection types being incredibly instrusive, we opt to ignore the typing for now. The right way would be to spread the object types, but currently the implementation is buggy. See: facebook/flow#1326
li-kai added a commit to nusmodifications/nusmods that referenced this issue Sep 15, 2017
Due to intersection types being incredibly instrusive, we opt to ignore the typing for now. The right way would be to spread the object types, but currently the implementation is buggy. See: facebook/flow#1326
li-kai added a commit to nusmodifications/nusmods that referenced this issue Sep 15, 2017
* Refactor TimetableContainer by seperating action row buttons

* Add CSS Modules support

* Refactor timetable

* Temporarily ignore flow type errors
Due to intersection types being incredibly instrusive, we opt to ignore the typing for now. The right way would be to spread the object types, but currently the implementation is buggy. See: facebook/flow#1326

* Fix global styling issues

* Fix production build issues

* Minor stylistic changes
@msiebert
Copy link

@STRML I get SyntaxError: Unexpected token when I try to use the spread notation. I found a suggestion to use flow comments to get around this issue, and it works, but I was wondering if you had a more elegant solution. Thanks!

@STRML
Copy link
Contributor

STRML commented Oct 19, 2017

Be sure your babel version is up to date for that.

@msiebert
Copy link

Just tried 6.26.0 and got the same issue. As far as I can tell, that's the latest version of babel, right?

@STRML
Copy link
Contributor

STRML commented Oct 19, 2017 via email

@msiebert
Copy link

Cool. Thanks for your help. 👍 I'll just stick with the comment syntax for now.

@mgtitimoli
Copy link

@conartist6 you can do what you asked here using a combination of spreading and $Shape like follows:

type Result = {
  ...$Shape<OtherType>,
  // additional entries
};

@jedwards1211
Copy link
Contributor

jedwards1211 commented Aug 21, 2019

Does object type spread work?

// @flow

type Foo = {foo: number}
type Spread = {...Foo}

function f(o: Spread) {
	const n: number = o.foo // should be fine, right?
}
7: 	const n: number = o.foo
                      ^ Cannot assign `o.foo` to `n` because undefined [1] is incompatible with number [2].
References:
3: type Foo = {foo: number}
                    ^ [1]
7: 	const n: number = o.foo
             ^ [2]

Problem doesn't happen with exact object shapes but I don't get why there should be a problem with inexact object shapes.

@goodmind
Copy link
Contributor

@jedwards1211 it works fine in master

@jedwards1211
Copy link
Contributor

all released versions I've tried don't work though, going back a long way. So I'm not sure whether it's a bug or something I fundamentally misunderstand about how object type spread is intended to work

@goodmind
Copy link
Contributor

it's long standing bug

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