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

Mixing map annotations with other properties #187

Closed
crm416 opened this Issue Dec 12, 2014 · 2 comments

Comments

Projects
None yet
3 participants
@crm416
Contributor

crm416 commented Dec 12, 2014

I may not have a great understanding of how the objects-as-maps type system should work, but it's strange to me that Flow lets you add map-violating properties to an object literal at time of creation, only to throw an error when you attempt to read from it.

For example:

var numToString: { [key:number]: string} = {
  foo: 'bar' // no error here, even though the key is not a number
};
numToString.foo; // throws an error because the key is a string
numToString['foo']; // throws an error because the key is a string

You get the same behavior if you include the property in the type annotation:

var numToString: { foo: string; [key:number]: string} = {
  foo: 'bar' // no error here
};
numToString.foo; // throws an error
numToString['foo']; // throws an error

With the current behavior, the only reason I can think of to allow mixing map annotations and specific-property annotations would be to mark some keys as required, and that would only work (as-is) if you're using a map from strings to something else.

Intuitively, my guess is that my first example should throw an error at the time of creation, but that my second should work throughout (i.e., adding specific-property annotations is way of white-list properties that don't adhere to the overall map type).

@samwgoldman

This comment has been minimized.

Member

samwgoldman commented Jul 14, 2015

I ran into this as well, and after a bit of digging, it looks like the cause is in the (ObjT, GetT) flow, which always compares the property with the indexer, without considering that the property could be in the prop map instead.

@samwgoldman

This comment has been minimized.

Member

samwgoldman commented Sep 1, 2016

This no longer reproduces.

@samwgoldman samwgoldman closed this Sep 1, 2016

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