Skip to content

Commit 2c3c6df

Browse files
perf: show early spinner instantly and keep it running through all API calls
Start a "Loading..." spinner before the dynamic import so there's zero silence after invoking a command. The spinner is adopted by each API call's withSpinner (text updates seamlessly), released back on completion so the next call re-adopts it, and auto-cleared via a stdout interceptor the moment command output begins. Also adds getTasksByFilter to the API spinner map so td today, td upcoming, and td filter show get proper spinner coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 7e5ccfb commit 2c3c6df

4 files changed

Lines changed: 273 additions & 31 deletions

File tree

src/__tests__/spinner.test.ts

Lines changed: 160 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2-
import { LoadingSpinner, withSpinner } from '../lib/spinner.js'
2+
import yoctoSpinnerFactory from 'yocto-spinner'
3+
import {
4+
LoadingSpinner,
5+
resetEarlySpinner,
6+
startEarlySpinner,
7+
stopEarlySpinner,
8+
withSpinner,
9+
} from '../lib/spinner.js'
310

411
// Mock yocto-spinner
512
const mockSpinnerInstance = {
613
start: vi.fn().mockReturnThis(),
714
success: vi.fn(),
815
error: vi.fn(),
916
stop: vi.fn(),
17+
text: '',
1018
}
1119

1220
vi.mock('yocto-spinner', () => ({
@@ -29,6 +37,7 @@ vi.mock('chalk', () => ({
2937
describe('withSpinner', () => {
3038
beforeEach(() => {
3139
vi.clearAllMocks()
40+
resetEarlySpinner()
3241
// Reset environment variables
3342
delete process.env.TD_SPINNER
3443
delete process.env.CI
@@ -43,6 +52,7 @@ describe('withSpinner', () => {
4352

4453
afterEach(() => {
4554
vi.clearAllMocks()
55+
resetEarlySpinner()
4656
})
4757

4858
it('should handle successful operations', async () => {
@@ -140,6 +150,7 @@ describe('withSpinner', () => {
140150
describe('LoadingSpinner', () => {
141151
beforeEach(() => {
142152
vi.clearAllMocks()
153+
resetEarlySpinner()
143154
// Reset environment variables
144155
delete process.env.TD_SPINNER
145156
delete process.env.CI
@@ -154,6 +165,7 @@ describe('LoadingSpinner', () => {
154165

155166
afterEach(() => {
156167
vi.clearAllMocks()
168+
resetEarlySpinner()
157169
})
158170

159171
it('should start and stop spinner', () => {
@@ -197,3 +209,150 @@ describe('LoadingSpinner', () => {
197209
expect(mockSpinnerInstance.error).not.toHaveBeenCalled()
198210
})
199211
})
212+
213+
describe('early spinner', () => {
214+
const yoctoSpinner = vi.mocked(yoctoSpinnerFactory)
215+
216+
beforeEach(() => {
217+
vi.clearAllMocks()
218+
resetEarlySpinner()
219+
mockSpinnerInstance.text = ''
220+
// Reset environment variables
221+
delete process.env.TD_SPINNER
222+
delete process.env.CI
223+
// Mock TTY as true by default
224+
Object.defineProperty(process.stdout, 'isTTY', {
225+
value: true,
226+
configurable: true,
227+
})
228+
// Clear process.argv
229+
process.argv = ['node', 'td']
230+
})
231+
232+
afterEach(() => {
233+
vi.clearAllMocks()
234+
resetEarlySpinner()
235+
})
236+
237+
it('should start and stop early spinner', () => {
238+
startEarlySpinner()
239+
240+
expect(yoctoSpinner).toHaveBeenCalledWith({ text: 'Loading...' })
241+
expect(mockSpinnerInstance.start).toHaveBeenCalled()
242+
243+
stopEarlySpinner()
244+
expect(mockSpinnerInstance.stop).toHaveBeenCalled()
245+
})
246+
247+
it('should not start early spinner when not in TTY', () => {
248+
Object.defineProperty(process.stdout, 'isTTY', {
249+
value: false,
250+
configurable: true,
251+
})
252+
253+
startEarlySpinner()
254+
expect(yoctoSpinner).not.toHaveBeenCalled()
255+
})
256+
257+
it.each([
258+
['--json', ['node', 'td', 'today', '--json']],
259+
['--ndjson', ['node', 'td', 'today', '--ndjson']],
260+
['--no-spinner', ['node', 'td', 'today', '--no-spinner']],
261+
])('should not start early spinner with %s flag', (_flagName, argv) => {
262+
process.argv = argv
263+
264+
startEarlySpinner()
265+
expect(yoctoSpinner).not.toHaveBeenCalled()
266+
})
267+
268+
it('should not start early spinner in CI environment', () => {
269+
process.env.CI = 'true'
270+
271+
startEarlySpinner()
272+
expect(yoctoSpinner).not.toHaveBeenCalled()
273+
})
274+
275+
it('should not start early spinner when TD_SPINNER=false', () => {
276+
process.env.TD_SPINNER = 'false'
277+
278+
startEarlySpinner()
279+
expect(yoctoSpinner).not.toHaveBeenCalled()
280+
})
281+
282+
it('should be adopted by LoadingSpinner.start() — reuses instance and updates text', () => {
283+
startEarlySpinner()
284+
vi.clearAllMocks()
285+
286+
const spinner = new LoadingSpinner()
287+
spinner.start({ text: 'Loading tasks...', color: 'blue' })
288+
289+
// Should NOT have created a new yocto-spinner
290+
expect(yoctoSpinner).not.toHaveBeenCalled()
291+
// Should NOT have called .start() again (already running)
292+
expect(mockSpinnerInstance.start).not.toHaveBeenCalled()
293+
// Should have updated the text
294+
expect(mockSpinnerInstance.text).toBe('Loading tasks...')
295+
})
296+
297+
it('should release back on stop — available for re-adoption by next API call', () => {
298+
startEarlySpinner()
299+
vi.clearAllMocks()
300+
301+
// First LoadingSpinner adopts the early spinner
302+
const spinner1 = new LoadingSpinner()
303+
spinner1.start({ text: 'Loading tasks...', color: 'blue' })
304+
expect(yoctoSpinner).not.toHaveBeenCalled()
305+
306+
// stop() releases it back instead of actually stopping
307+
spinner1.stop()
308+
expect(mockSpinnerInstance.stop).not.toHaveBeenCalled()
309+
310+
// Second LoadingSpinner re-adopts the same instance
311+
const spinner2 = new LoadingSpinner()
312+
spinner2.start({ text: 'Checking authentication...', color: 'blue' })
313+
expect(yoctoSpinner).not.toHaveBeenCalled()
314+
expect(mockSpinnerInstance.start).not.toHaveBeenCalled()
315+
expect(mockSpinnerInstance.text).toBe('Checking authentication...')
316+
317+
// Final cleanup via stopEarlySpinner actually stops it
318+
spinner2.stop()
319+
stopEarlySpinner()
320+
expect(mockSpinnerInstance.stop).toHaveBeenCalledTimes(1)
321+
})
322+
323+
it('should actually stop on fail even if adopted', () => {
324+
startEarlySpinner()
325+
vi.clearAllMocks()
326+
327+
const spinner = new LoadingSpinner()
328+
spinner.start({ text: 'Loading tasks...', color: 'blue' })
329+
spinner.fail('Request failed')
330+
331+
expect(mockSpinnerInstance.error).toHaveBeenCalled()
332+
// Should not be released back — error terminates the spinner
333+
stopEarlySpinner()
334+
expect(mockSpinnerInstance.stop).not.toHaveBeenCalled()
335+
})
336+
337+
it('should auto-stop when stdout is written to', () => {
338+
startEarlySpinner()
339+
expect(mockSpinnerInstance.start).toHaveBeenCalled()
340+
341+
// Simulate command output — spinner should auto-clear
342+
process.stdout.write('output\n')
343+
expect(mockSpinnerInstance.stop).toHaveBeenCalled()
344+
})
345+
346+
it('should be cleaned up by stopEarlySpinner if never adopted', () => {
347+
startEarlySpinner()
348+
expect(mockSpinnerInstance.start).toHaveBeenCalled()
349+
350+
stopEarlySpinner()
351+
expect(mockSpinnerInstance.stop).toHaveBeenCalled()
352+
353+
// Subsequent stop should be a no-op
354+
vi.clearAllMocks()
355+
stopEarlySpinner()
356+
expect(mockSpinnerInstance.stop).not.toHaveBeenCalled()
357+
})
358+
})

src/index.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { type Command, program } from 'commander'
44
import packageJson from '../package.json' with { type: 'json' }
5+
import { startEarlySpinner, stopEarlySpinner } from './lib/spinner.js'
56

67
program
78
.name('td')
@@ -107,11 +108,21 @@ if (commandName && commands[commandName]) {
107108
const idx = program.commands.findIndex((c) => c.name() === commandName)
108109
if (idx !== -1) (program.commands as Command[]).splice(idx, 1)
109110
const loader = commands[commandName][1]
110-
const register = await loader()
111-
register(program)
111+
112+
startEarlySpinner()
113+
try {
114+
const register = await loader()
115+
register(program)
116+
} catch (err) {
117+
stopEarlySpinner()
118+
throw err
119+
}
112120
}
113121

114-
program.parseAsync().catch((err: Error) => {
115-
console.error(err.message)
116-
process.exit(1)
117-
})
122+
program
123+
.parseAsync()
124+
.catch((err: Error) => {
125+
console.error(err.message)
126+
process.exit(1)
127+
})
128+
.finally(() => stopEarlySpinner())

src/lib/api/core.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ const API_SPINNER_MESSAGES: Record<string, { text: string; color?: 'blue' | 'gre
3636
updateSection: { text: 'Updating section...', color: 'yellow' },
3737
deleteSection: { text: 'Deleting section...', color: 'yellow' },
3838
quickAddTask: { text: 'Adding task...', color: 'green' },
39+
getTasksByFilter: { text: 'Loading tasks...', color: 'blue' },
3940
}
4041

4142
function createSpinnerWrappedApi(api: TodoistApi): TodoistApi {

0 commit comments

Comments
 (0)