Fixing lodash defs for flow 0.88.0+ #3020
Conversation
| @@ -158,7 +158,7 @@ declare module "lodash" { | |||
|
|
|||
| declare type NestedArray<T> = Array<Array<T>>; | |||
|
|
|||
| declare type matchesIterateeShorthand = Object; | |||
| declare type matchesIterateeShorthand = {[id: any]: any}; | |||
There was a problem hiding this comment.
Since Object now means any we can require a very liberal indexed object definition instead.
| @@ -746,8 +746,8 @@ declare module "lodash" { | |||
| shuffle<T>(array: ?Array<T>): Array<T>; | |||
| shuffle<V, T: Object>(object: T): Array<V>; | |||
| size(collection: $ReadOnlyArray<any> | Object | string): number; | |||
| some<T>(array: ?$ReadOnlyArray<T>, predicate?: Predicate<T>): boolean; | |||
There was a problem hiding this comment.
By making the null case overload first, the null case test passes successfully.
| @@ -776,22 +776,22 @@ declare module "lodash" { | |||
| after(n: number, fn: Function): Function; | |||
| ary(func: Function, n?: number): Function; | |||
| before(n: number, fn: Function): Function; | |||
| bind(func: Function, thisArg: any, ...partials: Array<any>): Function; | |||
| bind<R>(func: (...any[]) => R, thisArg: any, ...partials: Array<any>): (...any[]) => R; | |||
There was a problem hiding this comment.
lodash.bind returns a partially applied function with the this set with thisArg. Since Function is now any we can instead use (...any[]) => R as the function type.
It does not seem there are any Flow utility types that will allow us to manipulate the argument types in a way that will allow us to correctly type the arguments and partials beyond the use of any.
| bindKey(obj?: ?Object, key?: ?string, ...partials?: Array<?any>): Function; | ||
| curry: Curry; | ||
| curry(func: Function, arity?: number): Function; | ||
| curryRight(func: Function, arity?: number): Function; | ||
| debounce<F: Function>(func: F, wait?: number, options?: DebounceOptions): F; | ||
| defer(func: Function, ...args?: Array<any>): TimeoutID; | ||
| defer(func: (...any[]) => any, ...args?: Array<any>): TimeoutID; |
There was a problem hiding this comment.
Since Function is now any type, (...any[]) => any will at least require func to be a function. This allows the null case to correctly raise a type error.
// $ExpectError
defer(null);
| flip<R>(func: (...any[]) => R): (...any[]) => R; | ||
| memoize<A, R>(func: (...A) => R, resolver?: (...A) => mixed): (...A) => R; | ||
| negate<A, R>(predicate: (...A) => R): (...A) => boolean; | ||
| once<A, R>(func: (...A) => R): (...A) => R; |
There was a problem hiding this comment.
More cases where Function is being replaced with a () => {} type. Arguments and return values are made generic where possible.
| partial(func: Function, ...partials: any[]): Function; | ||
| partialRight(func: Function, ...partials: Array<any>): Function; | ||
| partial<R>(func: (...any[]) => R, ...partials: any[]): (...any[]) => R; | ||
| partialRight(func: (...any[]) => any, ...partials: Array<any>): Function; |
There was a problem hiding this comment.
Function is now any, this at least requires a valid function.
| @@ -891,12 +891,10 @@ declare module "lodash" { | |||
| isNull(value: any): false; | |||
| isNumber(value: number): true; | |||
| isNumber(value: any): false; | |||
| isObject(value: Object): true; | |||
| isObject(value: any): false; | |||
| isObject(value: any): boolean; | |||
There was a problem hiding this comment.
Object is now any so isObject(value: Object): true no longer works.
| @@ -1066,6 +1064,7 @@ declare module "lodash" { | |||
| ): Object; | |||
| at(object?: ?Object, ...paths: Array<string>): Array<any>; | |||
| at(object?: ?Object, paths: Array<string>): Array<any>; | |||
| create(prototype: void | null, properties: void | null): {}; | |||
There was a problem hiding this comment.
Explicitly adding the null case for create which allows the test to pass.
| unset(object: void | null, path?: ?Array<string> | ?string): true; | ||
| unset(object: Object, path?: ?Array<string> | ?string): boolean; |
There was a problem hiding this comment.
Moving the null case first allows the null test to pass.
| @@ -1393,13 +1392,13 @@ declare module "lodash" { | |||
| cond(pairs?: ?NestedArray<Function>): Function; | |||
| conforms(source?: ?Object): Function; | |||
| constant<T>(value: T): () => T; | |||
| defaultTo<T1: string | boolean | Object, T2>( | |||
| defaultTo<T1: void | null, T2>(value: T1, defaultValue: T2): T2; | |||
There was a problem hiding this comment.
Move the null case to appear first and remove Object from the non-null case since it means any.
| @@ -1608,7 +1607,7 @@ declare module "lodash/fp" { | |||
|
|
|||
| declare type NestedArray<T> = Array<Array<T>>; | |||
|
|
|||
| declare type matchesIterateeShorthand = Object; | |||
| declare type matchesIterateeShorthand = {[string | number]: any}; | |||
There was a problem hiding this comment.
Now that Object means any an indexed object type more accurately describes that the matchesIterateeShorthand type can be.
| @@ -2358,7 +2357,7 @@ declare module "lodash/fp" { | |||
| delay(wait: number, func: Function): TimeoutID; | |||
| flip(func: Function): Function; | |||
| memoize<F: Function>(func: F): F; | |||
| negate(predicate: Function): Function; | |||
| negate<A, R>(predicate: (...A) => R): (...A) => R; | |||
There was a problem hiding this comment.
Should be:
| negate<A, R>(predicate: (...A) => R): (...A) => R; | |
| negate<A, R>(predicate: (...A) => R): (...A) => boolean; |
| @@ -3154,18 +3152,18 @@ declare module "lodash/fp" { | |||
| cond(pairs: NestedArray<Function>): Function; | |||
| constant<T>(value: T): () => T; | |||
| always<T>(value: T): () => T; | |||
| defaultTo<T1: string | boolean | Object, T2>( | |||
| defaultTo<T1: void | null, T2>(defaultValue: T2): (value: T1) => T2; | |||
There was a problem hiding this comment.
Place the null cases first and remove Object since it means any.
| @@ -106,9 +106,6 @@ opaque type O = string; | |||
| const v: { [O]: number } = { x: 1, y: 2 }; | |||
| find(v, { x: 3 }); | |||
|
|
|||
| // $ExpectError undefined. This type is incompatible with object type. | |||
| var result: Object = find(users, "active"); | |||
There was a problem hiding this comment.
Since Object now means any this will never cause an error.
|
@beaucollins what's the status of this? Good to go from your side? |
|
@beaucollins some feedback from quick test: this seems to work better:
Same apply for others where function is restructured. Rather have it as a single type. That way Flow does not require type parameters. |
|
@villesau perhaps do both. I believe the typed parameters and return values should carry through and then it can fall back to the |
|
@beaucollins |
|
@villesau you're suggestion is implemented. These changes should be "good enough" to keep us going. There are a lot of definitions in this library that still use |
|
Nice, thanks for the contribution @beaucollins ! |
Now that
FunctionandObjectare equivalent toanytype many Lodash typedefs no longer work as expected.For example this is no longer valid:
Also, now that
Functionis the same asany, many of thenulltests failed because with v0.88.0 and onward this:becomes the same as:
We can fix some of these by defining the types a bit better. For instance this better describes what
.unaryactually does.