Skip to content

fix(vocal): stabilize the event handlers to prevent listener leaks on re-render #111

Closed
Muneebkhnnn wants to merge 1 commit intountemps:mainfrom
Muneebkhnnn:fix/vocal-handler-listener-leak
Closed

fix(vocal): stabilize the event handlers to prevent listener leaks on re-render #111
Muneebkhnnn wants to merge 1 commit intountemps:mainfrom
Muneebkhnnn:fix/vocal-handler-listener-leak

Conversation

@Muneebkhnnn
Copy link
Copy Markdown

Problem

Event handlers in Vocal.jsx were recreated on every render.
Since subscribe/unsubscribe rely on reference equality, this caused
mismatches when a re-render occurred during an active recognition session,
leading to potential listener leaks.

Solution

  • Memoized all event handlers using useCallback
  • Stabilized the handler map using useMemo
  • Added refs to stabilize unstable values from useVocal, useTimeout
    and useCommands hooks (stop, triggerCommand, on* prop callbacks)

Testing

  • Verified all subscribe/unsubscribe pairs use identical references
  • Tested re-renders during active sessions — no duplicate listeners observed

- Memoize handlers with the useCallback
- Stabilize handler map with useMemo
- Ensure subscribe/unsubscribe use same function references
- Prevent duplicate listeners during active session
@Muneebkhnnn Muneebkhnnn changed the title fix(vocal): stabilize the event handlers to prevent listener leaks on re-render fix(vocal): stabilize the event handlers to prevent listener leaks on re-render May 5, 2026
@Muneebkhnnn Muneebkhnnn closed this May 5, 2026
@Muneebkhnnn Muneebkhnnn reopened this May 5, 2026
@untemps
Copy link
Copy Markdown
Owner

untemps commented May 6, 2026

Hi @Muneebkhnnn, thanks for the thorough analysis — the problem you identified is real.

A quick heads-up though: main is being superseded by v2.0.0, currently in pre-release on the beta branch. That branch already includes a significant rewrite of Vocal.js (React 19 migration, propTypes removal, simplified event handling), so this patch doesn't apply cleanly there.

The stale prop callback issue still exists in beta. Two options:

  • Merge on main → fast, ships as v1.7.x, but v2.0.0 will need a separate fix anyway
  • Target beta → the fix lands in v2.0.0, adapted to the new code structure, but takes longer

What's your preference? Does the urgency justify a v1.x patch?

@untemps
Copy link
Copy Markdown
Owner

untemps commented May 6, 2026

PR #112 covers the full scope of yours, targeting beta (the upcoming v2.0.0) rather than main. It applies the same core approach — useCallback on all handlers, useMemo on HANDLERS, propsRef for stable prop callbacks, onEndRef/stableTimerCb to break the circular dep — with two corrections over your implementation: unsubscribeAllRef is assigned inline during render instead of in a useEffect (avoids a first-mount timing gap), and
stopRecognition uses finally to guarantee cleanup even when stop() throws.

@Muneebkhnnn
Copy link
Copy Markdown
Author

Thanks! So should I keep #111 open for the v1.7.x patch on main, or would you prefer to close it?

@untemps
Copy link
Copy Markdown
Owner

untemps commented May 7, 2026

Thanks @Muneebkhnnn! I'll close this in favour of the work already underway on the upcoming major release — the fix will ship as part of v2.0.0.

@untemps untemps closed this May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants