facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.09k stars 1.86k forks source link

Flow ignores some method overloads #3188

Closed Macil closed 7 years ago

Macil commented 7 years ago

A new version of the library Kefir added two new overloads of the Kefir.combine method which take an object, and now Flow complains if I pass an array. Here's a stripped down example:

/* @flow */

declare class Observable<+V,+E=*> {
}

declare var Kefir: {
  Observable: typeof Observable;
  never(): Observable<*>;
  combine<E>(obss: Observable<any,E>[], combinator?: Function): Observable<any,E>;
  combine<E>(obss: Observable<any,E>[], passiveObss?: Observable<any,E>[], combinator?: Function): Observable<any,E>;
  combine<E>(obss: {[key:string]:Observable<any,E>}, combinator?: Function): Observable<any,E>;
  combine<E>(obss: {[key:string]:Observable<any,E>}, passiveObss?: {[key:string]:Observable<any,E>}, combinator?: Function): Observable<any,E>;
};

const a = Kefir.never();
const b = Kefir.never();

Kefir.combine([a, b]);

and the error:

foo.js:18
 18: Kefir.combine([a, b]);
                   ^^^^^^ array literal. This type is incompatible with the expected param type of
 12:   combine<E>(obss: {[key:string]:Observable<any,E>}, passiveObss?: {[key:string]:Observable<any,E>}, combinator?: Function): Observable<any,E>;
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ object type

Found 1 error

Running Flow v0.37.4

Commenting out the last two new overloads makes Flow not give an error.

Macil commented 7 years ago

Oh, Kefir is an object rather than a class... is Flow silently treating the subsequent combine property definitions as redefinitions instead of something like method overloads?

benadamstyles commented 7 years ago

@AgentME I can't find where now, but earlier today I read that duplicate properties in object declarations simply override previous ones. Overloading only works in class declarations.

EDIT: found it: https://github.com/facebook/flow/issues/1556#issuecomment-200051475