fix: useDebounce race condition (#139) (#146)

Co-authored-by: Hunter Johnston <[email protected]>

.changeset/three-owls-matter.md

@@ -0,0 +1,5 @@
+---
+"runed": patch
+---
+
+fix: useDebounce race condition (#139)

packages/runed/src/lib/utilities/useDebounce/useDebounce.svelte.ts

@@ -8,6 +8,13 @@ type UseDebounceReturn<Args extends unknown[], Return> = ((
 	pending: boolean;
 };
 
+type DebounceContext<Return> = {
+	timeout: ReturnType<typeof setTimeout> | null;
+	resolve: (value: Return) => void;
+	reject: (reason: unknown) => void;
+	promise: Promise<Return>;
+};
+
 /**
  * Function that takes a callback, and returns a debounced version of it.
  * When calling the debounced function, it will wait for the specified time
@@ -29,62 +36,68 @@ export function useDebounce<Args extends unknown[], Return>(
 	callback: (...args: Args) => Return,
 	wait: MaybeGetter<number> = 250
 ): UseDebounceReturn<Args, Return> {
-	let timeout = $state<ReturnType<typeof setTimeout>>();
-	let resolve: null | ((value: Return) => void) = null;
-	let reject: null | ((reason: unknown) => void) = null;
-	let promise: Promise<Return> | null = null;
-
-	function reset() {
-		timeout = undefined;
-		promise = null;
-		resolve = null;
-		reject = null;
-	}
+	let context = $state<DebounceContext<Return> | null>(null);
 
 	function debounced(this: unknown, ...args: Args) {
-		if (timeout) {
-			clearTimeout(timeout);
-		}
-
-		if (!promise) {
-			promise = new Promise((res, rej) => {
+		if (context) {
+			// Old context will be reused so callers awaiting the promise will get the
+			// new value
+			if (context.timeout) {
+				clearTimeout(context.timeout);
+			}
+		} else {
+			// No old context, create a new one
+			let resolve: (value: Return) => void;
+			let reject: (reason: unknown) => void;
+			let promise = new Promise<Return>((res, rej) => {
 				resolve = res;
 				reject = rej;
 			});
+
+			context = {
+				timeout: null,
+				promise,
+				resolve: resolve!,
+				reject: reject!,
+			};
 		}
 
-		timeout = setTimeout(
+		context.timeout = setTimeout(
 			async () => {
+				// Grab the context and reset it
+				// -> new debounced calls will create a new context
+				if (!context) return;
+				const ctx = context;
+				context = null;
+
 				try {
-					resolve?.(await callback.apply(this, args));
+					ctx.resolve(await callback.apply(this, args));
 				} catch (error) {
-					reject?.(error);
-				} finally {
-					reset();
+					ctx.reject(error);
 				}
 			},
 			typeof wait === "function" ? wait() : wait
 		);
 
-		return promise;
+		return context.promise;
 	}
 
 	debounced.cancel = async () => {
-		if (timeout === undefined) {
+		if (!context || context.timeout === null) {
 			// Wait one event loop to see if something triggered the debounced function
 			await new Promise((resolve) => setTimeout(resolve, 0));
-			if (timeout === undefined) return;
+			if (!context || context.timeout === null) return;
 		}
 
-		clearTimeout(timeout);
-		reject?.("Cancelled");
-		reset();
+		clearTimeout(context.timeout);
+		context.reject("Cancelled");
+		context = null;
 	};
 
 	Object.defineProperty(debounced, "pending", {
 		enumerable: true,
 		get() {
-			return !!timeout;
+			return !!context?.timeout;
 		},
 	});
 

packages/runed/src/lib/utilities/useDebounce/useDebounce.test.svelte.ts

@@ -28,4 +28,54 @@ describe("useDebounce", () => {
 		await new Promise((resolve) => setTimeout(resolve, 200));
 		expect(fn).not.toHaveBeenCalled();
 	});
+
+	testWithEffect("Doesn't reuse promise after cancel", async () => {
+		// Same as above
+		const fn = vi.fn();
+		const debounced = useDebounce(fn, 100);
+
+		expect(fn).not.toHaveBeenCalled();
+		debounced().catch(() => {});
+		expect(fn).not.toHaveBeenCalled();
+		expect(debounced.pending).toBe(true);
+		debounced.cancel();
+		expect(debounced.pending).toBe(false);
+		await new Promise((resolve) => setTimeout(resolve, 200));
+		expect(fn).not.toHaveBeenCalled();
+
+		// New test
+		let wasCatchCalled = false;
+		debounced().catch(() => (wasCatchCalled = true));
+		expect(wasCatchCalled).toBe(false);
+		await new Promise((resolve) => setTimeout(resolve, 110));
+		expect(wasCatchCalled).toBe(false);
+		expect(fn).toHaveBeenCalledTimes(1);
+	});
+
+	testWithEffect("No race condition with running callback", async () => {
+		let calledNTimes = 0;
+
+		const slowFunction = async () => {
+			calledNTimes++;
+
+			await new Promise((resolve) => setTimeout(resolve, 100));
+		};
+		const debounced = useDebounce(slowFunction, 100);
+
+		expect(calledNTimes).toBe(0);
+		debounced();
+		expect(calledNTimes).toBe(0);
+		expect(debounced.pending).toBe(true);
+
+		await new Promise((resolve) => setTimeout(resolve, 110));
+		expect(calledNTimes).toBe(1);
+		expect(debounced.pending).toBe(false);
+		debounced();
+		expect(calledNTimes).toBe(1);
+		expect(debounced.pending).toBe(true);
+
+		await new Promise((resolve) => setTimeout(resolve, 110));
+		expect(debounced.pending).toBe(false);
+		expect(calledNTimes).toBe(2);
+	});
 });