Skip to content

Region-Prioritized Error Checking for Editors #57393

Closed
microsoft/vscode
#208713
@DanielRosenwasser

Description

@DanielRosenwasser

One of the difficulties in working in larger files is that error updates have per-file granularity. This means that even if a viewport is focused on a region of 60 lines of code, the entire file must be type-checked. For a file that's more than 50,000 lines like TypeScript's checker.ts, it can be painfully slow to wait for errors to go away after typing something.

One idea might be to enrich TypeScript's existing errors requests with regions of interest. By specifying the viewport of the editor as a reigon, TypeScript can first prioritize gathering errors across statements that are visible to a user, and inform an editor of any diagnostic/error spans discovered in that region. Any prior diagnostics in that region can be considered invalidated, and a subsequent response with all the diagnostics can be returned afterwards.

Implementation

Right now TypeScript has a GeterrRequestArgs that looks like this:

/**
 * Arguments for geterr messages.
 */
export interface GeterrRequestArgs {
    /**
     * List of file names for which to compute compiler errors.
     * The files will be checked in list order.
     */
    files: string[];

    /**
     * Delay in milliseconds to wait before starting to compute
     * errors for the files in the file list
     */
    delay: number;
}

Instead of files being string[], we could switch over to each element being string | FileSpan or something similar.

When TypeScript receives a span, it can find a more appropriate span to start/end checking on. Errors are typically emitted as events that are defined through these types:

export interface DiagnosticEventBody {
    /**
     * The file for which diagnostic information is reported.
     */
    file: string;

    /**
     * An array of diagnostic information items.
     */
    diagnostics: Diagnostic[];
}

export type DiagnosticEventKind = "semanticDiag" | "syntaxDiag" | "suggestionDiag";

/**
 * Event message for DiagnosticEventKind event types.
 * These events provide syntactic and semantic errors for a file.
 */
export interface DiagnosticEvent extends Event {
    body?: DiagnosticEventBody;
    event: DiagnosticEventKind;
}

and these events types can be adjusted to indicate the following information:

  • whether the event is a region-based diagnostic event, and
  • what the adjusted region for a diagnostic request was

This information MUST be provided so that editors

  • do not clear out all the diagnostics
  • know what to adjust in case the user scrolls, or TypeScript has adjusted the span for whatever reason

Risks

There are some risks associated with this approach.

Checking Inconsistencies

First, out-of-order type-checking is already something that can cause inconsistencies between tsc and the editor. This could get worse. However, we removed the separate diagnostics-producing type-checker from the one used by the rest of the language service 2 years ago (#36747), so completions, quick info, etc. can already cause this. Since then, the problems have mostly smoothed out over time.

The other concern is that diagnostics in a region are not always caused by the code within that same region. Diagnostics can be placed wherever they choose. The risk here is that if type-checking the code within a viewport does not create the currently-visible errors, then a user will end up with flashing diagnostics as they type. Basically:

  1. A user types some code
  2. TypeScript comes back with diagnostics for the viewport
  3. There are no diagnostics due to the current viewport, so the editor clears out all the diagnostics
  4. TypeScript then comes back with diagnostics for the entire file, and the errors re-appear

This could get aggravating.

For the most part, we do not issue errors in such a non-local fashion, and strive to keep most errors close to the relevant code. The exception to this might be conflicting declarations that are duplicates. Otherwise, most non-local diagnostics are related spans. If there are any other errors that are not co-located with their code, we should probably try to relocate them to improve the experience.

Chattiness

In some cases, it may be unnecessary to check only a specific region. For some files, we might be better off just checking the entire file. This is why TypeScript should reserve the right to decide on an appropriate span, or to even report diagnostics only for a specific span.

Metadata

Metadata

Assignees

Labels

Domain: Error MessagesThe issue relates to error messagingFix AvailableA PR has been opened for this issueIn DiscussionNot yet reached consensusSuggestionAn idea for TypeScript

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions