codemirror / dev

Development repository for the CodeMirror editor project
https://codemirror.net/
Other
5.94k stars 377 forks source link

forceLinting() doesn't cancel pending linting #1381

Closed KDean-Dolphin closed 6 months ago

KDean-Dolphin commented 6 months ago

Describe the issue

I'm using browser local storage to store documents that are edited using CodeMirror. The code below installs the editor view, loading the document from browser local storage as it goes.

Documents may be stored in an invalid state (according to the linter), so, after loading, I want to run the linter immediately to restore the validation errors for the user. I've made the linter delay user-configurable, so I don't want to wait for the timeout. At the end of the installation, I call forceLinting() to restore the validation errors.

This results in the linter being called twice: once when the document is opened, and once when the delay times out, even if the user does nothing to change the document. There's a lot going on behind the scenes (not just the linting) when the linter kicks in, so I'd rather not spend the cycles doing the validation again.

If forceLinting() is called, any pending linting should be cancelled.

installView(editorID: string, storageKey?: string, lintDelay?: number) {
    if (this._editorView) {
        throw new Error(`View already installed`);
    }

    const editorElement = $(`#${editorID}`);

    if (!editorElement) {
        throw new Error(`Editor ID "${editorID}" not found`);
    }

    // Set storage key if defined.
    if (storageKey !== undefined) {
        this.storageKey = storageKey;
    }

    // Set lint delay if defined.
    if (lintDelay !== undefined) {
        this.lintDelay = lintDelay;
    }

    this._editorView = new EditorView({
        doc: this.storageKey ? window.localStorage.getItem(this.storageKey) ?? undefined : undefined,
        extensions: this._extensions,
        parent: editorElement.get(0)
    });

    this._docOpenPending = true;

    forceLinting(this._editorView);
}

Browser and platform

No response

Reproduction link

No response

marijnh commented 6 months ago

Indeed, it shouldn't do that. Attached patch changes that.

KDean-Dolphin commented 6 months ago

Thanks. I'll verify once the update is available via npm.

KDean-Dolphin commented 6 months ago

@marijnh

I updated and tested and the problem remains. It might be because the editor view has, at this point in my code, been installed but not fully initialized, and during initialization it's setting up the first linter run.

marijnh commented 6 months ago

I cannot reproduce the issue when I try it like this (the linter's console.log only runs once on startup).

KDean-Dolphin commented 6 months ago

I apologize, it turns out that I needed to restart my server after updating the lint package. It's working fine now.