types/lodash/index.d.ts

to authors (@bczengel @chrootsu @stepancar @ericanderson @aj-r @Ailrun @e-cloud @thorn0). Could you review this PR?
👍 or 👎?

@unional Please fix the failures indicated in the Travis CI log.

@aj-r I think I was wrong and we should keep is Function there. The failing tests show why. What was your motivation when you added ((...args: any[]) => any) |? Did some code not compile without it?

#21015 is a stop-gap for microsoft/TypeScript#19480

Fixing that does means we lost the Function type, which is also useful (as the test show).

Would need some input.
Testing to use generics.

(It is so hard to do PR on DT, and no movement on microsoft/types-publisher#4)

The problem with Function is that after the type guard you can't call it with parameters.

You can call a Function. This compiles:

declare var a: Function;
a(10, 20);

But you can't call a Function | ((...args: any[]) => any):

declare var a: Function | ((...args: any[]) => any)
a(10, 20);

The question is which of the two parts of the union should stay in the type guard. The problem with Function is that it's an interface for declaring methods of Function.prototype. Just like the types Boolean and String, it's not supposed to be used for other purposes. However, people actually use it a lot thinking that it's an alias for (...args: any[]) => any, which it's not. Why not? I'd like to know it too, but now 1) (...args: any[]) => any isn't assignable to Function, 2) you can't pass a variable of the type Function to a function like declare function f<T>(a: (...args: any[]) => T): T;, etc. In these cases we get an error message that Function doesn't have a call signature, which means the type system considers it a supertype of (...args: any[]) => any, despite the fact that it can actually be called without any compilation errors, i.e. the call signature magically emerges in call expressions only.

So far my conclusion is that the type guard should be is Function. Although it's not recommended to use this type, we don't want to break the code of those who use it.

On the other hand, @aj-r might have had some extra considerations in mind when he added (...args: any[]) => any. Let's hear from him.

I was trying to replace Function with (...args: any[]) => any, but after testing I realized that this broke things for people using Function, so I added back | Function.

I did some more testing just now, and it looks like if we remove | Function, the only thing that is potentially broken is the type of value inside of the else block. Consider this test case:

let value: number|Function = any;

if (_.isFunction(value)) {
    value; // has type (...args: any[]) => any  - good
} else {
    value; // has type number | Function  - but we would have expected just number
}

I don't think this is a huge problem. If anything, we should be forcing people to stop using Function anyways, so I vote we remove | Function.

Whichever route we take, we should add this test to lodash-tests.ts to make sure the call signature works:

if (_.isFunction(any)) {
    any();
}

@unional Please fix the failures indicated in the Travis CI log.

You can test that your changes are working by running npm run lint lodash
@unional Please fix the failures indicated in the Travis CI log.

I cannot get the lint to run TypeScript@next compile error: Type 'Function' is not assignable to type 'number'

Working on DT is now too complex and frustrating. When will microsoft/types-publisher#4 be worked on?

There is no reason to download the whole DT repo just to make a change to one package.

More detail on the error:

Installing to /Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/typescript-installs/next...

Installed!
Error: Unexpected tag kind: JSDocTypeLiteral
    at checkTag (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/bin/rules/noRedundantJsdoc2Rule.js:95:23)
    at cb (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/bin/rules/noRedundantJsdoc2Rule.js:47:25)
    at visitNodes (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/node_modules/typescript/lib/typescript.js:12652:30)
    at Object.forEachChild (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/node_modules/typescript/lib/typescript.js:12910:21)
    at cb (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/bin/rules/noRedundantJsdoc2Rule.js:52:19)
    at visitNodes (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/node_modules/typescript/lib/typescript.js:12652:30)
    at Object.forEachChild (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/node_modules/typescript/lib/typescript.js:12828:24)
    at cb (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/bin/rules/noRedundantJsdoc2Rule.js:52:19)
    at visitNode (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/node_modules/typescript/lib/typescript.js:12643:24)
    at Object.forEachChild (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/node_modules/typescript/lib/typescript.js:12929:21)
Error: /Users/hwong/github/DefinitelyTyped/DefinitelyTyped/types/lodash/lodash-tests.ts
ERROR: 8063:17  expect  TypeScript@next compile error:
Type 'Function' is not assignable to type 'number'.

    at /Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/bin/index.js:76:19
    at Generator.next (<anonymous>)
    at fulfilled (/Users/hwong/github/DefinitelyTyped/DefinitelyTyped/node_modules/dtslint/bin/index.js:5:58)
    at <anonymous>

npm@5 doesn't work with node@9.

Approved by a listed owner. PR ready to merge pending express review by a maintainer.

Yep, sorry, I was wrong.