Skip to content

Commit d9522d5

Browse files
committed
fix(react): guard processor cast and detach on unmount
Two correctness fixes from PR review: - Guard the `KrispNoiseFilterProcessor` cast in the returned `processor` value with the same `name === 'livekit-noise-filter'` check used inside the effect. The unconditional cast assumed nothing else could attach a `TrackProcessor` to the track; if anything did, the hook handed consumers a value typed as a Krisp processor that wasn't one. - Add an unmount-only effect that detaches the Krisp processor from the currently-managed track. Empty deps so it doesn't interfere with the existing toggle-reuse path on shouldEnable / filterOptions / track changes. Closes the gap where a caller-owned `trackRef` outlives the hook's component, leaving the track with a Krisp processor attached that the hook no longer manages.
1 parent 63fa161 commit d9522d5

1 file changed

Lines changed: 32 additions & 2 deletions

File tree

packages/react/src/hooks/cloud/krisp/useKrispNoiseFilter.ts

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ export function useKrispNoiseFilter(options: useKrispNoiseFilterOptions = {}) {
4848
micPublication = options.trackRef.publication;
4949
}
5050

51+
// Holds the LocalAudioTrack the hook is currently managing, so the unmount
52+
// cleanup below can detach the Krisp processor even when a caller-owned
53+
// trackRef outlives this component.
54+
const managedTrackRef = React.useRef<LocalAudioTrack | undefined>(undefined);
55+
managedTrackRef.current =
56+
micPublication?.track instanceof LocalAudioTrack ? micPublication.track : undefined;
57+
5158
const setNoiseFilterEnabled = React.useCallback(async (enable: boolean) => {
5259
if (enable) {
5360
const { isKrispNoiseFilterSupported } = await import('@livekit/krisp-noise-filter');
@@ -126,9 +133,32 @@ export function useKrispNoiseFilter(options: useKrispNoiseFilterOptions = {}) {
126133
};
127134
}, [shouldEnable, micPublication?.track, options.filterOptions]);
128135

129-
const processor =
136+
// Detach on unmount. Empty deps mean the cleanup only fires when the hook
137+
// unmounts, not on every track / shouldEnable / filterOptions change — so it
138+
// doesn't interfere with the toggle-reuse path above. Covers the case where
139+
// a caller-owned trackRef outlives the hook's component.
140+
React.useEffect(() => {
141+
return () => {
142+
const track = managedTrackRef.current;
143+
if (!track) return;
144+
if (track.getProcessor()?.name !== 'livekit-noise-filter') return;
145+
// Fire-and-forget: cleanup can't await. stopProcessor is a no-op when
146+
// the track was already stopped externally (e.g. on Room disconnect).
147+
track.stopProcessor().catch((e) => {
148+
log.warn('Krisp hook: error detaching processor on unmount', e);
149+
});
150+
};
151+
}, []);
152+
153+
const trackProcessor =
130154
micPublication?.track instanceof LocalAudioTrack
131-
? (micPublication.track.getProcessor() as KrispNoiseFilterProcessor | undefined)
155+
? micPublication.track.getProcessor()
156+
: undefined;
157+
// Guard the cast: another module could have set a non-Krisp processor on the
158+
// track, and we should not lie to consumers about its type.
159+
const processor =
160+
trackProcessor?.name === 'livekit-noise-filter'
161+
? (trackProcessor as KrispNoiseFilterProcessor)
132162
: undefined;
133163

134164
return {

0 commit comments

Comments
 (0)