TS: "Declare method" is really bad

[bpasero]

So many things wrong:

• import uses a weird syntax
• I am not being asked for the visibility, it is just public by default
• any (!) is declared as return type??? I should be asked

[bpasero]

With this, it took me more keystrokes to fix the method compared to just writing it myself.

[mjbvz]

Please provide steps to reproduce this issue in the VS Code codebase so we can investigate

[bpasero]

@mjbvz in commandsQuickAccess#CommandsHistory add this._register(this.storageService.onDidChangeStorage(e => this.onDidChangeState(e))); into the registerListeners and then invoke quick fix on the error.

[mjbvz]

Thanks.

Point 1 is definitely a bug. I'll create a priority issue on TS for this

Point 2 is microsoft/TypeScript#36249 but for declare method. I'll open a separate issue for that

For point 3, TS should try to infer the return type based on context. In this case, the listener is typed to return any. Here's an example that shows this behavior:

function onSomething(e: (x: number) => number) { } // change this to 'any' to compare behavior

class Foo {
  bar() {
    onSomething(() => this.baz())
  }
}

The intent of quick fixes is to fix the issue, so we've avoid requiring user input on them

[bpasero]

TS should never suggest any. It is called TypeScript, not AnyScript 👍

[mjbvz]

But in your example, it's our code that explicitly says the listener returns any. We should change the return type of the listener if we don't want this to happen

[mjbvz]

1. 'declare method' quick fix adds inline import TypeScript#37781

2. 'declare method' quick fix for adding a private method TypeScript#37782

3. Is as-designed since we use any in our code