Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix clear() method return value #3

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

lumirlumir
Copy link
Contributor

Hello,

While the documentation mentions that the clear() method returns the instance, it doesn't actually do so. This causes an error when attempting to use method chaining, as shown in the example below:

spinner.clear().stop();

To fix this, I modified the clear() method to return this, enabling method chaining. I also followed the formatting style used in the start() and stop() methods.

  • For reference, here is the formatting used in the start() and stop() methods:

    yocto-spinner/index.js

    Lines 64 to 100 in 4e18542

    start(text) {
    if (text) {
    this.#text = text;
    }
    if (this.isSpinning) {
    return this;
    }
    this.#hideCursor();
    this.#render();
    this.#subscribeToProcessEvents();
    this.#timer = setInterval(() => {
    this.#render();
    }, this.#interval);
    return this;
    }
    stop(finalText) {
    if (!this.isSpinning) {
    return this;
    }
    clearInterval(this.#timer);
    this.#timer = undefined;
    this.#showCursor();
    this.clear();
    this.#unsubscribeFromProcessEvents();
    if (finalText) {
    this.#stream.write(`${finalText}\n`);
    }
    return this;
    }

  • I’ve also attached a screenshot of the documentation for reference:
    image

@sindresorhus sindresorhus changed the title fix: update clear() method to return this to support method chaining Fix clear() method return value Dec 10, 2024
@sindresorhus sindresorhus merged commit 41cda43 into sindresorhus:main Dec 10, 2024
@lumirlumir lumirlumir deleted the lumirlumir-patch-1 branch December 11, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants