Tslint: `no-unnecessary-callback-wrapper` false positive (mismatched args)

Created on 29 Mar 2017 · 7Comments · Source: palantir/tslint

Bug Report

TypeScript code being linted

function a(arg1: string, arg2?: number) {
}

function b(callback: (arg1: string, arg2?: string) => void) {
}

b((arg1) => a(arg1)); // rule warns on this...
b(a);                 // ...but this isn't valid

with tslint.json configuration:

"no-unnecessary-callback-wrapper": true

Actual behavior

If the 2nd args are optional and different types, the rule flags this, but it can't be fixed by unwrapping

Expected behavior

Shouldn't warn, but I don't think this can be fixed without type checking

P3 Requires Type Checker Bug 🌹 R.I.P. 🌹

Source

nchen63

👍18

Most helpful comment

This rule breaks several window methods, for example:

const foo = {
    bar: (cb) => setTimeout(cb)
}

which becomes

const foo = {
    bar: setTimeout
}

And foo.bar() throws TypeError: illegal invocation.

This thing happens with all window methods like setTimeout, clearTimeout, etc.

mad-gooze on 21 Nov 2017

👍4 ❤1

All 7 comments

The classic example of this is map and parseInt:

> ['010', '010', '010'].map(x => parseInt(x))
[10, 10, 10]
> ['010', '010', '010'].map(parseInt)
[10, NaN, 2]

danvk on 20 Apr 2017

👍5

Same problem

felixfbecker on 16 Aug 2017

This rule breaks several window methods, for example:

const foo = {
    bar: (cb) => setTimeout(cb)
}

which becomes

const foo = {
    bar: setTimeout
}

And foo.bar() throws TypeError: illegal invocation.

This thing happens with all window methods like setTimeout, clearTimeout, etc.

mad-gooze on 21 Nov 2017

👍4 ❤1

I made a small reproducement repo for a simplified case of something we ran into in our code.

await new Promise<void>(res => {
  document.addEventListener('click', () => res());
});

Replacing () => res() with res results in a Typescript error (Event and void is not compatible for the Promise).

https://github.com/csvn/tslint-issue-no-unnecessary-callback-wrapper

csvn on 20 Dec 2017

👍2

Is there a workaround for this issue?

Springrbua on 29 May 2019

👍1

💀 _It's time!_ 💀

TSLint is deprecated and no longer accepting pull requests other than security fixes. See #4534. ☠️
We recommend you instead use typescript-eslint to lint your TypeScript code with ESLint. ✅

👋 It was a pleasure open sourcing with you!