Publish diagnostics for all open files when multiple files are open

[keyboardDrummer]

Notes

• We now create a debounce function per open file, so two different files will not 'bounce' the diagnostics for one another.
• I had to use test specific uri's in the tests to prevent the tests from interacting. Even though the tests run in series, they re-use some state (the LSP server) which will causes the tests to interact. Not re-using the state is, I think, not an option because it will slow down the tests.

Testing

• Ran the tests
• Tested against an IDE. This CR resolves this issue

[akosyakov]

Won't it execute publishDiagnostics as many times as updateDiagnostics is called instead of once? How about passing publishDiagnostics to FileDiagnostics and using firePublishDiagnostics as it was before, i.e. without any async logic.

[keyboardDrummer]

Won't it execute publishDiagnostics as many times as updateDiagnostics is called instead of once?

It won't. If you look at updateDiagnostics, you can see it only resolves the promise once if multiple calls happen within the debounce window, since the call to resolve is within the function passed to debounce.

[akosyakov]

ok, although the code will be less tricky (without any async handling) on impl and client sides:

    constructor(
        readonly uri: string,
        protected publishDiagnostics: (params: lsp.PublishDiagnosticsParams) => void
    ) { }

    updateDiagnostics(kind: EventTypes, diagnostics: tsp.Diagnostic[]): void {
        this.diagnosticsPerKind.set(kind, diagnostics.map(toDiagnostic));
        this.firePublishDiagnostics();
    }
    protected firePublishDiagnostics = debounce((): void => {
        const { uri } = this;
        const diagnostics = this.getDiagnostics();
        this.publishDiagnostics({ uri, diagnostics });
    }, 50);

[akosyakov]

@keyboardDrummer Goot catch!

Before it will postpone publishing diagnostics for all files while tsserver is spamming with diagnostic events, e.g. during the full build on a monorepo. With this change, diagnostics debounced only for a single file leading to republishing a lot of unnecessary diagnostics in some cases. I would prefer that we debounce for all files as before, but fix an issue with publishing only for last.

[keyboardDrummer]

Before it will postpone publishing diagnostics for all files while tsserver is spamming with diagnostic events, e.g. during the full build on a monorepo. With this change, diagnostics debounced only for a single file leading to republishing a lot of unnecessary diagnostics in some cases. I would prefer that we debounce for all files as before, but fix an issue with publishing only for last.

The current implementation will publish diagnostics for all open files whenever diagnostics change, this allows IDE's to implement something like a problems view, where diagnostics are shown not just for the last edited file but for multiple files.

If you look at the problems view in VS Code you will see that it shows errors for all open files (for TypeScript).

[akosyakov]

@keyboardDrummer please take care of build failures and let's merge it

[gitpod-io-legacy-app]

Open in Gitpod - starts a development workspace for this pull request in code review mode and opens it in a browser IDE.