Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

spinners: fix remove logic #4950

Merged
merged 3 commits into from
Mar 30, 2022
Merged

spinners: fix remove logic #4950

merged 3 commits into from
Mar 30, 2022

Conversation

benjamincburns
Copy link
Contributor

@benjamincburns benjamincburns commented Mar 28, 2022

Works around a bug in the spinnies module (jbcarpanelli/spinnies#34) where removal of the final spinner may cause the spinner text to not disappear and/or may cause the process to hang on termination due to an interval not being cleared.

Copy link
Member

@cds-amal cds-amal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works around a bug in the spinnies module where removal of the final spinner may cause the spinner text to not disappear and/or may cause the process to hang on termination due to an interval not being cleared.

That's quite the premise! This should probably link to the bug/issue described.

packages/spinners/lib/spinner.ts Outdated Show resolved Hide resolved
packages/spinners/typings/spinnies/index.d.ts Show resolved Hide resolved
Prior to this change, if `remove` was called for an individual spinner,
the spinner may not have been removed and the process wouldn't exit due
to an interval not being cleared properly.
@benjamincburns
Copy link
Contributor Author

benjamincburns commented Mar 30, 2022

This should probably link to the bug/issue described.

No such issue exists, and I didn't bother to create one as spinnies appears to be unmaintained. I'll go ahead and create one now, however. Issue created: jbcarpanelli/spinnies#34

I did reach out to the repo owner and offer to volunteer as a maintainer however, as it's a project that gets 100k+ weekly downloads and appears to see very few bug reports and/or pull requests.

@cds-amal
Copy link
Member

... Issue created: jcarpanelli/spinnies#34

I did reach out to the repo owner and offer to volunteer as a maintainer however, as it's a project that gets 100k+ weekly downloads and appears to see very few bug reports and/or pull requests.

It's a better experience to ora, imo. If the author abandoned it, perhaps we can switch to a maintained fork, or fork our own.

Copy link
Member

@cds-amal cds-amal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants