Skip to content

Commit d74ca75

Browse files
authored
fix(frontend): resolve stale closure in chatSearch.ts debounce (#1225) (#1292)
## Root cause `debounce()` in `chatSearch.ts` had three related bugs: 1. **Stale `fn` closure** — `fn` was captured at `debounce()` call time and never updated. When callers recreated their callback (e.g. an inline arrow function on each render), the debounced wrapper kept calling the original, stale function. 2. **Stale `timer` reference** — `timer` was never reset to `null` after the timeout fired. Future `clearTimeout(timer)` calls operated on an already-expired ID, giving a false sense of cancellation. 3. **No `cancel()` method** — there was no way for a caller to abort a pending invocation on component unmount, causing callbacks to fire against unmounted state. ## Fix (`src/lib/chatSearch.ts`) - Introduced `latestFn` variable inside `debounce`; added `updateFn(newFn)` on the returned function so callers can swap the underlying callback without rebuilding the debounced wrapper. - `timer = null` is now set inside the `setTimeout` callback so the reference is always accurate. - Added `cancel()` method that clears and nulls the pending timer. - Exported the `DebouncedFn<T>` interface for typed usage. ## Regression tests (`src/lib/chatSearch.test.ts`) Six new tests under `describe('debounce')` covering: - Basic fire-after-delay - Debounce (rapid successive calls) - `updateFn` calls the new fn, not the original (stale-closure regression) - `cancel()` prevents firing - Timer reference is null after fire (no stale ID) - Call after cancel works correctly Closes #1225 Closes #1223
1 parent 0d6baf3 commit d74ca75

2 files changed

Lines changed: 109 additions & 5 deletions

File tree

Dechat/dex_with_fiat_frontend/src/lib/chatSearch.test.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { describe, expect, it } from 'vitest';
1+
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
22
import {
3+
debounce,
34
findHighlights,
45
splitByHighlights,
56
searchChatHistory,
@@ -287,3 +288,77 @@ describe('searchChatHistory – date range', () => {
287288
expect(result.matches[0].highlights).toHaveLength(0);
288289
});
289290
});
291+
292+
// ---------------------------------------------------------------------------
293+
// debounce — stale-closure regression tests (#1225)
294+
// ---------------------------------------------------------------------------
295+
296+
describe('debounce', () => {
297+
beforeEach(() => { vi.useFakeTimers(); });
298+
afterEach(() => { vi.useRealTimers(); });
299+
300+
it('calls fn after the delay', () => {
301+
const fn = vi.fn();
302+
const d = debounce(fn, 200);
303+
d('a');
304+
expect(fn).not.toHaveBeenCalled();
305+
vi.advanceTimersByTime(200);
306+
expect(fn).toHaveBeenCalledOnce();
307+
expect(fn).toHaveBeenCalledWith('a');
308+
});
309+
310+
it('resets the timer on rapid successive calls (debounce behaviour)', () => {
311+
const fn = vi.fn();
312+
const d = debounce(fn, 300);
313+
d('first');
314+
vi.advanceTimersByTime(100);
315+
d('second');
316+
vi.advanceTimersByTime(100);
317+
d('third');
318+
vi.advanceTimersByTime(300);
319+
expect(fn).toHaveBeenCalledOnce();
320+
expect(fn).toHaveBeenCalledWith('third');
321+
});
322+
323+
it('regression: calls the latest fn after updateFn (stale-closure fix)', () => {
324+
// Bug: before fix, `fn` was captured at debounce() call time and never
325+
// updated, so a re-rendered callback was never reflected.
326+
const originalFn = vi.fn();
327+
const updatedFn = vi.fn();
328+
const d = debounce(originalFn, 100);
329+
d.updateFn(updatedFn); // simulate caller updating fn after re-render
330+
d('payload');
331+
vi.advanceTimersByTime(100);
332+
expect(originalFn).not.toHaveBeenCalled();
333+
expect(updatedFn).toHaveBeenCalledWith('payload');
334+
});
335+
336+
it('cancel() prevents a pending callback from firing', () => {
337+
const fn = vi.fn();
338+
const d = debounce(fn, 200);
339+
d('x');
340+
d.cancel(); // e.g. called in useEffect cleanup on component unmount
341+
vi.advanceTimersByTime(200);
342+
expect(fn).not.toHaveBeenCalled();
343+
});
344+
345+
it('resets timer reference to null after firing (no stale timer ID)', () => {
346+
const fn = vi.fn();
347+
const d = debounce(fn, 100);
348+
d('first');
349+
vi.advanceTimersByTime(100); // timer fires, should be null now
350+
// cancel on an already-fired timer must be a no-op, not throw
351+
expect(() => d.cancel()).not.toThrow();
352+
});
353+
354+
it('allows a second call after cancel without the first leaking', () => {
355+
const fn = vi.fn();
356+
const d = debounce(fn, 150);
357+
d('a');
358+
d.cancel();
359+
d('b');
360+
vi.advanceTimersByTime(150);
361+
expect(fn).toHaveBeenCalledOnce();
362+
expect(fn).toHaveBeenCalledWith('b');
363+
});
364+
});

Dechat/dex_with_fiat_frontend/src/lib/chatSearch.ts

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,45 @@ export interface SearchResults {
2121
totalMessages: number;
2222
}
2323

24+
export interface DebouncedFn<T extends (...args: unknown[]) => void> {
25+
(...args: Parameters<T>): void;
26+
/** Cancel any pending invocation. Call on component unmount to avoid stale callbacks. */
27+
cancel(): void;
28+
/** Swap the underlying function without resetting the timer. Prevents stale-closure bugs
29+
* when the caller's callback is recreated on each render (e.g. an inline arrow function). */
30+
updateFn(newFn: T): void;
31+
}
32+
2433
/** Debounce helper — returns a debounced version of `fn`. */
2534
export function debounce<T extends (...args: unknown[]) => void>(
2635
fn: T,
2736
delayMs: number,
28-
): (...args: Parameters<T>) => void {
37+
): DebouncedFn<T> {
2938
let timer: ReturnType<typeof setTimeout> | null = null;
30-
return (...args: Parameters<T>) => {
31-
if (timer) clearTimeout(timer);
32-
timer = setTimeout(() => fn(...args), delayMs);
39+
// Store the latest fn so callers can update it via updateFn() without
40+
// recreating the debounced wrapper (fixes stale-closure on re-render).
41+
let latestFn: T = fn;
42+
43+
function debounced(...args: Parameters<T>): void {
44+
if (timer !== null) clearTimeout(timer);
45+
timer = setTimeout(() => {
46+
timer = null; // reset so future clearTimeout checks are accurate
47+
latestFn(...args);
48+
}, delayMs);
49+
}
50+
51+
debounced.cancel = (): void => {
52+
if (timer !== null) {
53+
clearTimeout(timer);
54+
timer = null;
55+
}
56+
};
57+
58+
debounced.updateFn = (newFn: T): void => {
59+
latestFn = newFn;
3360
};
61+
62+
return debounced;
3463
}
3564

3665
/**

0 commit comments

Comments
 (0)