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

Better support for object types with union types indexer keys #5276

Closed
eloytoro opened this issue Nov 6, 2017 · 29 comments
Closed

Better support for object types with union types indexer keys #5276

eloytoro opened this issue Nov 6, 2017 · 29 comments

Comments

@eloytoro
Copy link

eloytoro commented Nov 6, 2017

Flexible types

type Key = 'xs' | 'sm'
type Value = { [Key]: number }

Right now flow will complain if you try to define an object that contains less properties than its defined type allows it to have, however this is not the case for types that have a indexer key defined for an enum.

In most typed languages enums should always be exhausted, however flow seems to ignore this by allowing less keys to be present in the object, which goes against the specified behavior for non-strict object types.

Solution

  • Make flow check for the presence of all of the keys of enums when these are statically defined.
  • Do not type check keys that are not present in the object definition or the enum

Special cases

The case where the enum both statically and generically defined

type Key = 'xs' | string

Here all we have to do is to take the most general case (string) and ignore the static definitions

Exact types

type Key = 'xs' | 'sm'
type Value = {| [Key]: number |}

Right now flow just doesnt really know what to do here, I don't think there's a single object that matches this type, while it looks pretty well defined, flow disagrees

Solution

In the example above the Value type could behave as a "key comprehension" of the Key union type in a way that it makes sure that the object has only keys defined by the union type, no more, no less

Special cases

Basically any general types should throw an error eagerly for trying to define all of the keys of an object generically. I.e.

{| [string]: number |} // this object has all the possible string keys EVER

Advantages

Now that we know which keys are in the object, calling Object.keys should return the $Keys of its argument

TL;DR The example is pretty self explanatory:

https://flow.org/try/#0PTAEAEDMBsHsHcBQBLAtgB1gJwC6gEoCmAhgMZ6RayqgDkWJ5tA3IojgJ7qGgDShHUAF46ADwDOtUAB8641LTalYAO3F5YAIwBWABgBcoAN6yA2vw4BdQyoCuqTYSwyAvsOOgJhgMyg3IUHEAC1hbaAATUCcqZ0hsUFRkcXFkFQBzUABrAVAAA3lcxGU1DR0ARkMTUHMBa1A7BydXdyNPcR8AGkDUQwAWP1AA4NCI+tg8aOwi1XVQLW0AJkrqizqGx2c3EVavUF9-MGGwyMnY+MTk1IzswXzUQuLZ+e9lmqsbew2B7dA42B8BkMQscxhMsDEgA

@eloytoro eloytoro changed the title Better support for object types with enum indexer keys Better support for object types with union types indexer keys Nov 6, 2017
@vkurchatkin
Copy link
Contributor

Maps don't require all possible keys to be set, that's the most important property. What you probably want is a way to create an object type from a give union of string literals.

@eloytoro
Copy link
Author

eloytoro commented Nov 6, 2017

Maps don't require all possible keys to be set, that's the most important property

Why is that an important property? after all you can mix object maps and literals alike, that would cause a lot of confusion

What you probably want is a way to create an object type from a give union of string literals.

I do, but I also need the guarantee that the resulting type's keys are all of the keys of the enum, otherwise I cant safely access those properties using an array of the union of string literals. I.e.

const obj: {| [Enum]: number |} = {...}
const keys: Array<Enum> = Object.keys(obj)

@eloytoro
Copy link
Author

eloytoro commented Nov 6, 2017

Also, flow allows for the {| [Enum]: number |} type declaration, but nothing matches that type, why is that? looks like a design flaw

@TiddoLangerak
Copy link
Contributor

TiddoLangerak commented Nov 6, 2017

Flow is also inconsistent with this: flow doesn't require that all keys are used when creating the object, but it does assume all keys are set when using the object. E.g.:

type Enum = 'A' | 'B';
type EnumMap = { [Enum] : string };

var map : EnumMap = {};

map['A'].substring(0);
// No errors!

(This is even worse with { [string] : ... } maps, since it assumes a value is set for any string).

I think it would be a very good idea to let flow make a distinction between complete and incomplete maps.

@vkurchatkin
Copy link
Contributor

Flow is also inconsistent with this: flow doesn't require that all keys are used when creating the object, but it does assume all keys are set when using the object

Nothing inconsistent about this, this works the same as arrays.

@eloytoro
Copy link
Author

eloytoro commented Nov 6, 2017

Nothing inconsistent about this, this works the same as arrays.

I disagree, its very inconsistent;

  • Objects and arrays are very different, the whole idea of having JS typed is so my objects can abide to their types and give the developer guarantees of what to find in them. Arrays don't have to have a specified size in order to comply to their types, but objects do
  • Objects require certain props to exist in them in order to comply to their types, but object maps don't, why is that? specially when they're treated as the same thing at a semantic level.
  • Object maps at the current state give us 0 guarantees up until the point we shouldnt even bother trying to type them, which is a moot point for using flow.

I think it would be a very good idea to let flow make a distinction between complete and incomplete maps.

Exactly my thoughts!

@vkurchatkin
Copy link
Contributor

Objects and arrays are very different, the whole idea of having JS typed is so my objects can abide to their types and give the developer guarantees of what to find in them

This is only true for normal objects, not for map objects. Map objects are exactly like arrays in this respect.

Objects require certain props to exist in them in order to comply to their types, but object maps don't, why is that?

Because that's exactly why maps exist. If you need a type, where specific properties are required, use normal object type.

Object maps at the current state give us 0 guarantees up until the point we shouldnt even bother trying to type them, which is a moot point for using flow.

They give some guarantees, but they are unsound in general. The same as arrays.

I think it would be a very good idea to let flow make a distinction between complete and incomplete maps.

"Complete map" is just a normal object type.

@eloytoro
Copy link
Author

eloytoro commented Nov 6, 2017

This is only true for normal objects, not for map objects. Map objects are exactly like arrays in this respect.

Thats bizarre, one would never think that an object map should behave like an array, in fact it should be far from it. The idea that one could define all the possible keys of a map in an union type is not very far fetched, in fact most typed languages do so, Map<K, V> usually allows you to define K using enums, I'm not going to go too much on why this is the case, but the main idea is that the type checker should be able to check the types of the keys you're using to map values to.

This idea is also very limiting, forcing type definitions to be lenient when it comes to union types in object maps will almost lead to bad type definitions.

Take this example

https://flow.org/try/#0PTAEAEDMBsHsHcBQBLAtgB1gJwC6gEoCmAhgMZ6RayqgDkWJ5tA3IjgJ7qGgCiAdgFcaAXjrFaoAD50ARrTaduAQVCiA3qADa-IQF0AXKEGoZhLABpQpAPyHjprKAC+iUrD4BnPLBkArQyrqoMSGAMzOru5eoDJ2Qg6qoD6+AHQyoCCgHgAWsALQACagZlSOpqTEAh7c6W75RaZG+dCRnnikcSZmickppEA

Flow is usually very strict when it comes to null/undefined values, but its not when it comes to object maps because of contradictions this set of rules, you're not guaranteeing the properties of the enum to be present in the object, therefore this object should never guarantee all of its values to be defined either

@eloytoro
Copy link
Author

eloytoro commented Nov 6, 2017

Not trying to compare the two libs, but typescript sets an example of how to define this type correctly.

type Enum = 'a' | 'b'

type Value {
    [key in Enum]: number
}

const value: Value = { a: 3 } // error: missing `b` key

@vkurchatkin
Copy link
Contributor

@eloytoro your TypeScript demonstrates my point: it's not a map, it's an object type. In fact, in TypeScript you can't use unions as map types at all.

@eloytoro
Copy link
Author

eloytoro commented Nov 6, 2017

If you could point me in the direction of achieving what the ts example shows I'll consider this issue solved

@eloytoro
Copy link
Author

eloytoro commented Nov 6, 2017

@vkurchatkin I think I understand your point, you're saying that object maps in flow should be defined like Map<K, V> where Map has keys of type K and values of type V, but not necessarily all possible values of K or V have to exist, that is a completely valid point.

However flow has some limitations even by that definition, such as Object.keys, even for well defined enums.

For some reason this will not cause an error, even when there's absolutely no guarantee that key is part of the union type
https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVAXAngBwKZgCiAdgK4C2YAvGAOQAeAzrWAD52Pm3rb5gBqAQxikCNAN6owYTgH4AXDIwAnAJbEA5gBopYJgqVrNqAL7oAxnGKMMYQYqEixYSdM6LaARgBMAZlo60kwePv6BYDAaIX7cZqgA8gBGAFZ45hgAdADWeFiMABSCAJQZUHDKhILmABb5+TlYiiQURdQAfGANRUA

And for some reason this does cause an error, even when its pretty clear that key can be no other than the union type
https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVAXAngBwKZgCiAdgK4C2YAvGAOQAeAzrWAD52Pm3rb5gBqAQxikCNAN6owYANokKAXQBcYRhgBOAS2IBzVAF90AYzjE1YQSqEixYSdM4raARgBMAZloAaKWCZO3TwN0AHkAIwArPCMMADoAazwsRgAKQQBKWKg4dUJBIwALFJTErBV5cnTqAD4wUvSgA

@eloytoro
Copy link
Author

eloytoro commented Nov 6, 2017

Another flaw in that definition would be that flow's syntax allows object maps to be defined as objects, which creates a huge contradiction: Object types have to have all of its keys defined and allow for unknown keys to exist whereas map types dont have to have all of its keys defined and dont allow unknown keys to exist. This contradiction is the result of the design flaw I refer to

@bradennapier
Copy link

bradennapier commented Nov 7, 2017

I have also ran into this. I haven't read this entire post - just the OP. I ended up having to create an exact object using $ObjMap to get the results I desired.

type ExtractPluginStateType = <P>(plugin: P) => Datastream$PluginSessionState;

export type Datastream$PluginStates = $ObjMap<
  PluginTypes,
  ExtractPluginStateType,
>;

I think the exact object type with indexer is a good syntax for something like this. It would need to be able to discern the difference of string and the string literals and either ignore any primitives or throw an error in such a case.

It is pretty much a shortcut for doing something like this to end up with the same types:

type Value = {|
  xs: number,
  sm: number,
|};

type Key = $Keys<Value>;

@eloytoro
Copy link
Author

eloytoro commented Nov 7, 2017

I have also ran into this. I haven't read this entire post - just the OP. I ended up having to create an exact object using $ObjMap to get the results I desired.

Flow's definition of $ObjMap is not an object map, its an object transformation function type. And honestly it shouldn't be part of the spec if flow had guarantees of object key types

@bradennapier
Copy link

Yep, just saying that it can provide the utility needed to compose your types in many cases -- providing a similar effect in the meantime. Not arguing against the need for an exact map.

@shinster
Copy link

shinster commented Nov 26, 2017

Here's a workaround for the "exact enum key" case:

type EnumMap<K: string, V, O: Object = *> = O & {[K]: V & $ElementType<O, K>}

type Obj = EnumMap<'A' | 'B' | 'C', number>

const obj: Obj = {
  A: 123,
  // B key missing
  C: 'wrong value type',
  extraKey: 456,
}

It gives the expected errors:

5: type Obj = EnumMap<'A' | 'B' | 'C', number>
                            ^ property `B`. Property not found in
7: const obj: Obj = {
                    ^ object literal

7: const obj: Obj = {
                    ^ property `extraKey` is a string. This type is incompatible with
5: type Obj = EnumMap<'A' | 'B' | 'C', number>
                      ^ string enum

10:   C: 'wrong value type',
         ^ string. This type is incompatible with
5: type Obj = EnumMap<'A' | 'B' | 'C', number>
                                       ^ number

@skovhus
Copy link

skovhus commented Feb 2, 2018

@shinster thanks!

@jlkalberer
Copy link

Does anyone have a good solution since inferred types are being deprecated?

@Tyrcheg
Copy link

Tyrcheg commented Jul 20, 2018

@jlkalberer I've made the following:

type Character = 'a' | 'b';

const response: { [char in Character]: number } = { a: 1, b: 2 };

console.log(response.a); // 1

@eloytoro
Copy link
Author

That doesn't compile

@jlkalberer
Copy link

@Tyrcheg - yeah, that looks like typescript to me.

It doesn't work here

@istarkov
Copy link

istarkov commented Sep 5, 2018

To check that obj contains all values from eum I use

/* @flow */
// any change in this type will cause flow error
type Languages = 'en' | 'de' | 'fr';

// must be untyped for check to work
const i18nLocalesCheck = {
  en: 1,
  de: 2,
  fr: 3,
};
// this will fail if new language is added
((('': any): Languages): $Keys<typeof i18nLocalesCheck>);
// this will fail if new language is removed
((('': any): $Keys<typeof i18nLocalesCheck>): Languages);
// in order of fail add/remove locales if needed

// now give type and use
const i18nLocales: { [Languages]: number } = i18nLocalesCheck;

see in action

@jranks123
Copy link

jranks123 commented Oct 5, 2018

@joelochlann and I had a situation where we wanted to create an object that would exhaustively handle every instance of a Union Type.
So for example, if we had:

type SomeEnum = 'A' | 'B' | 'C'

we wanted to be able to create an object for which the keys would be an exhaustive list of each instance of the Union Type, i.e

const a: { [SomeEnumType]: string } = {
     A: 'a',
     B: 'b',
     C: 'c',
}

and we would expect this to fail:

const a: { [SomeEnumType]: string }  = {
     A: 'a',
     B: 'b',
}

, as C isn't accounted for. But as we know, this doesn't fail.

This was our solution:

type SomeEnumMap<T> = {
  A: T,
  B: T,
  C: T
}

// We need to supply the type parameter, but we're only using the keys so it's irrelevant - so we supply null
export type SomeEnum = $Keys<SomeEnumMap<null>>

const enumMap: SomeEnumMap<string> = {
     A: 'a',
     B: 'b',
     C: 'c',
}
// success

const enumMap: SomeEnumMap<string> = {
     A: 'a',
     B: 'b',
}
// fails as C isn't accounted for

@Danchoys
Copy link

Danchoys commented Feb 15, 2019

I think I nailed it:

type MyUser = {
  name: string,
  id: number,
};

type MapType<T> = $Exact<$ObjMap<T, () => string>>;

const myMap: MapType<MyUser> = {
  name: 'some',
  id: 'identifier',
};

It will complain if

  • myMap is an empty object
  • myMap contains an extra field
  • myMap misses some field
  • value is not a string

Note: it works if you are mapping some object type keys into strings, so it is not exactly what TS was asking about, but will hopefully help or at least give a hint.

@jlkalberer
Copy link

@Danchoys - this is great. The $Exact part is what I was missing. I'll test this out when I get to work.

@goodmind
Copy link
Contributor

goodmind commented Jul 2, 2019

Related #7862

@goodmind
Copy link
Contributor

goodmind commented Jul 2, 2019

Related PR #7849

@SamChou19815
Copy link
Contributor

Closing since we now have mapped type

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