fix: cancel flash-message timers; make store init() idempotent#14
Open
Teamingzooper wants to merge 1 commit into
Open
fix: cancel flash-message timers; make store init() idempotent#14Teamingzooper wants to merge 1 commit into
Teamingzooper wants to merge 1 commit into
Conversation
…tent
Two latent bugs I noticed during the audit pass at the end of the
recent feature batch. Both are real but minor — no user-visible
breakage today, but they cause noisy warnings or accumulating
listeners in edge cases.
### Flash-message timers
SettingsPanel and ThemeEditor both used the pattern
setX('Saved'); setTimeout(() => setX(null), 4000);
without ever cancelling the timer. If the component unmounted (user
closed the settings modal) within those 4 seconds, React 18 logged a
"can't perform a React state update on an unmounted component" warning
and the pending setState ran on a stale component instance.
New `useFlash` hook in `src/renderer/hooks/useFlash.ts` wraps the
pattern: stores the timer id in a ref, cancels it on unmount, and
cancels any previous timer when called again so two flashes in quick
succession don't fight. Both components now use it; all eight raw
setTimeout sites are gone.
### Store init() accumulating listeners
`store.init()` subscribes to five `window.nexus.onX(...)` events
(unread, instance-activated, view-crashed, instance-hibernated,
instance-woken). The Retry button on the error screen calls init()
again, which would have added a second set of listeners on top of
the first — every event would then fire its state-update closure
twice (or N times after N retries).
Now there's a module-scoped `initDisposers` array. init() tears down
any pre-existing subscriptions at the top before creating new ones.
No tests added — both fixes are in code paths that are hard to
exercise without renderer-side test infrastructure. Verified
manually that the audit findings no longer reproduce, typecheck +
178/178 unit tests + build all clean.
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
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
Two latent bugs spotted during the audit pass at the end of the recent feature batch. Both are real but minor — no user-visible breakage today, but they cause React warnings or accumulating listeners in edge cases.
Bug 1 — Flash-message timers leak on unmount
`SettingsPanel` and `ThemeEditor` both use the pattern:
```ts
setMsg('Saved'); setTimeout(() => setMsg(null), 4000);
```
without ever cancelling the timer. If the user closes the settings modal within those 4 seconds, React 18 logs "can't perform a React state update on an unmounted component" and the pending setState runs on a stale component instance.
Fix: new `useFlash` hook in `src/renderer/hooks/useFlash.ts` that:
Both components now use it; all 8 raw `setTimeout(() => setX(null), ...)` sites are gone.
Bug 2 — Store `init()` accumulates listeners across retries
`store.init()` subscribes to five `window.nexus.onX(...)` events (unread, instance-activated, view-crashed, instance-hibernated, instance-woken). The Retry button on the error screen calls `init()` again, which would have added a second set of listeners — every event would then fire its state-update closure twice (or N times after N retries).
Fix: module-scoped `initDisposers` array. `init()` calls `disposeInitSubscriptions()` at the top before creating new ones. The `onX` IPC helpers already return a remove-listener disposer, so we just collect and replay them.
Test plan
🤖 Generated with Claude Code