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

Functions should not match {[key: string]: any} type #946

Closed
danvk opened this issue Oct 15, 2015 · 8 comments
Closed

Functions should not match {[key: string]: any} type #946

danvk opened this issue Oct 15, 2015 · 8 comments

Comments

@danvk
Copy link
Contributor

danvk commented Oct 15, 2015

I'm surprised that this snippet type checks:

function foo(props: {[key: string]: any}) { }
foo(x => x);
$ echo 'function foo(props: {[key: string]: any}) { }; foo(x => x);' | flow check-contents
No errors!
$ flow --version
Flow, a static type checker for JavaScript, version 0.17.0

In what sense is a function type the same as an object type?

@danvk
Copy link
Contributor Author

danvk commented Oct 15, 2015

From IRC:

[3:05pm] glevi: danvk: So technically a function is an object
[3:05pm] glevi: I think we've always been a little unsure about how to approach that reality
[3:06pm] glevi: For example, we say that Function is a subtype of Object
[3:06pm] glevi: Since all functions are objects
[3:07pm] danvk: {[key: string]: any} feels more specific than Object
[3:07pm] danvk: seems like it’s calling specifically for a string—>value mapping
[3:07pm] danvk: i.e. an object with string keys
[3:07pm] jeffmo: danvk: glevi: Arguably a function fits that description
[3:07pm] glevi: danvk: So technically all object keys are strings
[3:07pm] jeffmo: function foo(){} foo.prop = ‘asdf’;
[3:08pm] jeffmo: function foo(){} foo.name
[3:08pm] danvk: the prompt for this was that I was calling _.findWhere() instead of _.find()
[3:08pm] danvk: which is an error that Flow can’t catch if it considers function and object the same thing
[3:08pm] jeffmo: Practically speaking, I don’t think anyone expects that a function would pass checking if {[:string]: anye} were specified, though
[3:09pm] danvk: right, 1 * “1” is legal JS but Flow doesn’t allow it
[3:10pm] danvk: seems like this may be another one of those cases
[3:11pm] samg
: Uh, so the function is converted into a { "$call": Function } type, and that's a subtype of the indexable sig? Seems like we should exclude $call from that subtyping relationship.
[3:13pm] samg_: feels like a bug to me
[3:14pm] danvk: right… I’d expect {} to match that type
[3:14pm] danvk: but not function(){}
[3:17pm] samg_: { [:string]: any } is the same as Object, as far as I can tell
[3:17pm] samg
: but it should exclude the function, if I'm reading this right
[3:18pm] samg_: err, nope. no error with that as well, which is good, because at least flow is consistent
[3:20pm] samg_: In fact, flow is fairly explicit about functions being objects:

| t -> function_like t

@danvk
Copy link
Contributor Author

danvk commented Oct 30, 2015

@samwgoldman was this addressed by #984?

@samwgoldman
Copy link
Member

@danvk I don't think so. At least, that wasn't my intention.

@STRML
Copy link
Contributor

STRML commented Apr 24, 2016

Is there any way to create a typedef specifying a parameter should be an Object but not a Function? This otherwise makes it impossible to properly make a distinction between _.merge and _.mergeWith, for example.

@STRML
Copy link
Contributor

STRML commented Sep 30, 2016

I found a way to make this work:

type GenericMap<T> = {[key: string]: T} & {$call?: void};

Example

@calebmer
Copy link
Contributor

This is valid behavior for object types. You could also use an exact object type.

@nodkz
Copy link
Contributor

nodkz commented Oct 27, 2017

@STRML thanks for the solution. Please fix your code snippet for further googlers.

- type GenericMap<T> = {[key: string}: T} & {$call?: void};
                                    ^
+ type GenericMap<T> = {[key: string]: T} & {$call?: void};

@STRML
Copy link
Contributor

STRML commented Oct 27, 2017

Good catch, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants