feat: add createSpinner factory#5
Merged
Merged
Conversation
Extracts the spinner module shared across todoist-cli, twist-cli, and
outline-cli — yocto-spinner + chalk wrapper, optional early-spinner
singleton with stdout interception, adopt/release semantics for
chained API calls, and a withSpinner helper.
Each consumer's `~120 LoC of duplicated spinner code can now reduce to
a thin wrapper:
import { createSpinner } from '@doist/cli-core'
import { shouldDisableSpinner } from './global-args.js'
export const {
LoadingSpinner,
withSpinner,
startEarlySpinner,
stopEarlySpinner,
resetEarlySpinner,
} = createSpinner({ isDisabled: shouldDisableSpinner })
The factory returns a kit with its own closure-scoped early-spinner
state, so two consumers in the same process don't step on each other.
CLI-specific opt-out logic (TD_SPINNER vs TW_SPINNER vs OL_SPINNER, plus
each CLI's `--json` / `--ndjson` / `--no-spinner` parsing) deliberately
stays per-CLI — passed in via `isDisabled`.
Adds chalk + yocto-spinner as cli-core runtime deps (pinned to the same
versions all three CLIs already use).
Auto-generated by semantic-release on every release using the conventional-commits preset (uses '*' for list items). oxfmt prefers '-' and would flip them every release cycle; the file isn't hand-edited so reformatting it adds churn without value.
doistbot
reviewed
May 6, 2026
Member
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR thoughtfully extracts the duplicated spinner logic across the CLIs into a centralized createSpinner factory with support for early and adopted spinners. Consolidating this module is a great step for consistency and maintainability across the command-line tools. A few adjustments are noted regarding early spinner state management, safe monkey-patching of standard output, duplicate success icons, and minor testing and API export refinements.
Codifies a code-style preference in AGENTS.md and converts every existing `interface` declaration in src/ to `type` (5 in src + 1 in config.test.ts). `class implements TypeAlias` works fine for the one spot that needed it (LoadingSpinnerImpl), so the change is purely stylistic. `LoadingSpinner.start()`'s return type changes from `this` to the concrete `LoadingSpinner` because `this` types only work cleanly in interface declarations / class methods, not in `type` aliases. No caller chains beyond a single `.start()` so the precision loss is academic. Also adds an initial AGENTS.md covering the project's basic rules (build/run scripts, code style, module layout, release flow).
P1 bugs: - Adoption now restores stdout.write so the early-spinner interceptor doesn't keep firing against a now-null earlyInstance while the adopted spinner runs unattended. Tested. - succeed(text) on an adopted spinner now terminates with the success line instead of silently releasing back to the pool. The release- silently chained-call pattern is preserved when succeed() is called with no text. Both paths covered. - Drop the manual `✓ ` / `✗ ` prefixes — yocto-spinner.success/error prepend their own ✔ / ✖ glyphs. Tests now expect the bare text. P2 cleanup: - restoreStdoutWrite helper deduplicates the restore logic between stopEarlySpinner and resetEarlySpinner, and identity-checks the current process.stdout.write so it doesn't clobber a foreign hook layered on top. - releaseAdopted helper deduplicates the release-back block between succeed and stop. - Capture process.stdout.write in a local closure variable instead of .bind() so repeat startEarlySpinner calls don't nest binds. - resetEarlySpinner annotated @internal — exposed for test isolation only; not part of the supported runtime API. Tests: - yocto-spinner mock now returns a fresh instance per call (and uses the input text), so isolation/adoption assertions verify real per- instance state instead of mutations on a shared object. - Spy + swallow process.stdout.write so the auto-stop test no longer prints to the test runner output. - index.test.ts pins createSpinner + the spinner type re-exports alongside the existing public-API anchors. - New regressions: stdout-restore on adoption; succeed(text) on adopted; foreign stdout.write hook stays intact when restoring.
doist-release-bot Bot
added a commit
that referenced
this pull request
May 6, 2026
## [0.3.0](v0.2.0...v0.3.0) (2026-05-06) ### Features * add createSpinner factory ([#5](#5)) ([7092427](7092427))
Contributor
|
🎉 This PR is included in version 0.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extracts the spinner module that all three Doist CLIs duplicate (~120 LoC of nearly-identical code each). Ships as a
createSpinner({ isDisabled })factory so per-CLI opt-out signals (env vars likeTD_SPINNER/TW_SPINNER/OL_SPINNER, and per-CLI flag parsing) stay where they belong.Each kit has its own closure-scoped early-spinner state, so two kits in the same process don't step on each other (covered by a regression test).
What's in the module
LoadingSpinnerclass withstart/succeed/fail/stop. HonoursisStdoutTTY(),isDisabled(), andoptions.noSpinner.withSpinner(options, op)— runs an async operation flanked by a spinner.startEarlySpinner(text?)/stopEarlySpinner/resetEarlySpinner— long-running spinner shown before command modules load. Interceptsprocess.stdout.writeso the spinner auto-clears the moment any command output is produced.LoadingSpinner.startadopts an existing early spinner if one's running rather than stacking a second one.stopandsucceedrelease the adopted instance back to the pool so the next API call can re-adopt;failalways terminates.CLI-specific bits left in each CLI
isDisabledpredicate (well-known signals + opt-out env var name + flag parsing) — passed in via config.defaultColor,earlySpinnerText).Deps
Adds
chalk@5.6.2andyocto-spinner@1.1.0to cli-core's runtime dependencies — first non-builtin runtime deps for the package, but both are already pinned at these versions across all three consuming CLIs.Test plan
npm run buildnpm run checknpm test— 66 / 66 pass (21 new spinner tests acrosswithSpinner,LoadingSpinner, and early-spinner behaviour, plus a kit-isolation regression)🤖 Generated with Claude Code