felladrin / felladrin/LinkedTimer
Stopping the timer broadcasts a periodic sync that still says isRunning: true
- Dominant language
- TypeScript
- Stars
- 6
- Forks
- 2
- Avg merge
- 3h 51m
- Merged PRs (30d)
- 33
Description
Stopping the timer emits one periodic sync that still reports `isRunning: true`.
In `configureTimerEventHandlers` (`src/webview/scripts/constants/timer.ts`), the handler that updates the counters is registered before the one that updates `isTimerRunning`, and both listen to `stopped`:
```ts
(["started", "stopped", "secondsUpdated"] satisfies TimerEventType[]).forEach(...) // calls setTotalTimerSeconds
(["started", "stopped"] satisfies TimerEventType[]).forEach(...) // calls setTimerRunning
```
easytimer dispatches listeners in registration order, and `create-pubsub` publishes on every set with no equality check, so stopping the timer runs this chain:
1. `timer.stop()` resets the counters and dispatches `stopped`.
2. The first handler calls `setTotalTimerSeconds(0)`.
3. `onTotalTimerSecondsUpdated.ts` emits `PeriodicSync` with `isRunning: isTimerRunning()`, which is still `true`.
4. Only then does the second handler set it to `false`.
So every stop broadcasts one frame of `{ isRunning: true, totalSeconds: 0, timeValues: 00:00:00 }`.
What it costs, checked against `easytimer.js` rather than assumed:
- A peer that is **stopped** calls `startTimerWithValues({ hours: 0, minutes: 0, seconds: 0 })` if its own total differs by more than a second. That does not start anything: a countdown timer started at zero has already reached its target, so `isRunning` stays `false` and the UI stays on the editor. Verified with `new Timer({ countdown: true }).start({ startValues: { hours: 0, minutes: 0, seconds: 0 } })`, which leaves `isRunning()` at `false`.
- A peer that is **running** gets stopped by it, because `startTimerWithValues` stops the timer before the start that then no-ops.
The second case needs a room that is already desynced, which is why this is not urgent. #1198 makes it rarer rather than worse: it removes the state where a running joiner sits in a stopped room.
The fix is to register the `setTimerRunning` handler first. That also changes the Stop-button path, where the drift tolerance in `handlePeriodicSyncEvent` currently absorbs the stale frame, so it wants its own testing. See the comment above the condition in `handleInitialSyncEvent` (`src/webview/scripts/subscriptions/onRoomUpdated.ts`) for why that tolerance is there.
Found while reviewing #1198.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in src/webview/scripts/constants/timer.ts at configureTimerEventHandlers and inspect the ordering of the stopped handlers. Review src/webview/scripts/subscriptions/onRoomUpdated.ts, especially the tolerance comment and the periodic-sync handling, then test the Stop-button path and confirm stopping no longer broadcasts a frame with isRunning: true and totalSeconds: 0.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100