From f68161350a9dbd8494fcedfe4a1392c9ac60d5e1 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 2 Apr 2026 10:07:29 -0800 Subject: [PATCH] fix: render non-typed parameter changes immediately (#23951) This way, if you click a checkbox that is supposed to show a section (for example), you are not stuck waiting half a second. --- site/src/hooks/debounce.test.ts | 59 +++++++++++-------- site/src/hooks/debounce.ts | 22 ++++--- .../CreateWorkspacePageView.tsx | 14 ++++- 3 files changed, 57 insertions(+), 38 deletions(-) diff --git a/site/src/hooks/debounce.test.ts b/site/src/hooks/debounce.test.ts index 37dc01e5cb..86b4d6a50a 100644 --- a/site/src/hooks/debounce.test.ts +++ b/site/src/hooks/debounce.test.ts @@ -105,10 +105,16 @@ describe(useDebouncedValue.name, () => { describe(`${useDebouncedFunction.name}`, () => { function renderDebouncedFunction( callbackArg: (...args: Args) => void | Promise, - time: number, + time: number | ((...args: Args) => number), ) { return renderHook( - ({ callback, time }: { callback: typeof callbackArg; time: number }) => { + ({ + callback, + time, + }: { + callback: typeof callbackArg; + time: number | ((...args: Args) => number); + }) => { return useDebouncedFunction(callback, time); }, { @@ -117,27 +123,6 @@ describe(`${useDebouncedFunction.name}`, () => { ); } - describe("input validation", () => { - it("Should throw for non-nonnegative integer timeouts", () => { - const invalidInputs: readonly number[] = [ - Number.NaN, - Number.NEGATIVE_INFINITY, - Number.POSITIVE_INFINITY, - Math.PI, - -42, - ]; - - const dummyFunction = vi.fn(); - for (const input of invalidInputs) { - expect(() => { - renderDebouncedFunction(dummyFunction, input); - }).toThrow( - `Invalid value ${input} for debounceTimeoutMs. Value must be an integer greater than or equal to zero.`, - ); - } - }); - }); - describe("hook", () => { it("Should provide stable function references across re-renders", () => { const time = 5000; @@ -154,7 +139,22 @@ describe(`${useDebouncedFunction.name}`, () => { expect(oldCancel).toBe(newCancel); }); - it("Resets any pending debounces if the timer argument changes", async () => { + it("Should provide stable references with dynamic debounce", () => { + const time = 5000; + const { result, rerender } = renderDebouncedFunction(vi.fn(), () => time); + + const { debounced: oldDebounced, cancelDebounce: oldCancel } = + result.current; + + rerender({ callback: vi.fn(), time: () => time }); + const { debounced: newDebounced, cancelDebounce: newCancel } = + result.current; + + expect(oldDebounced).toBe(newDebounced); + expect(oldCancel).toBe(newCancel); + }); + + it("Does not reset any pending debounces if the timer argument changes", async () => { const time = 5000; const mockCallback = vi.fn(); const { result, rerender } = renderDebouncedFunction(mockCallback, time); @@ -163,7 +163,7 @@ describe(`${useDebouncedFunction.name}`, () => { rerender({ callback: mockCallback, time: time + 1 }); await vi.runAllTimersAsync(); - expect(mockCallback).not.toBeCalled(); + expect(mockCallback).toBeCalled(); }); }); @@ -177,6 +177,15 @@ describe(`${useDebouncedFunction.name}`, () => { expect(mockCallback).toBeCalledTimes(1); }); + it("Resolve dynamic debounce", async () => { + const mockCallback = vi.fn(); + const { result } = renderDebouncedFunction(mockCallback, () => 100); + result.current.debounced(); + + await vi.runOnlyPendingTimersAsync(); + expect(mockCallback).toBeCalledTimes(1); + }); + it("Always uses the most recent callback argument passed in (even if it switches while a debounce is queued)", async () => { const mockCallback1 = vi.fn(); const mockCallback2 = vi.fn(); diff --git a/site/src/hooks/debounce.ts b/site/src/hooks/debounce.ts index e6a23bcdd3..a059549aba 100644 --- a/site/src/hooks/debounce.ts +++ b/site/src/hooks/debounce.ts @@ -26,8 +26,11 @@ type UseDebouncedFunctionReturn = Readonly<{ * passed into the hook, and use them accordingly. * * If the debounce time changes while a callback has been queued to fire, the - * callback will be canceled completely. You will need to restart the debounce - * process by calling the returned-out function again. + * callback will not be canceled. + * + * Instead of a static debounce time, a function can be passed to enable dynamic + * debounce values (for example to make a checkbox fire immediately but to + * debounce a text input). */ export function useDebouncedFunction< // Parameterizing on the args instead of the whole callback function type to @@ -35,14 +38,8 @@ export function useDebouncedFunction< Args extends unknown[] = unknown[], >( callback: (...args: Args) => void | Promise, - debounceTimeoutMs: number, + debounceTimeoutMs: number | ((...args: Args) => number), ): UseDebouncedFunctionReturn { - if (!Number.isInteger(debounceTimeoutMs) || debounceTimeoutMs < 0) { - throw new Error( - `Invalid value ${debounceTimeoutMs} for debounceTimeoutMs. Value must be an integer greater than or equal to zero.`, - ); - } - const timeoutIdRef = useRef | undefined>( undefined, ); @@ -56,9 +53,8 @@ export function useDebouncedFunction< const debounceTimeRef = useRef(debounceTimeoutMs); useEffect(() => { - cancelDebounce(); debounceTimeRef.current = debounceTimeoutMs; - }, [cancelDebounce, debounceTimeoutMs]); + }, [debounceTimeoutMs]); const callbackRef = useRef(callback); useEffect(() => { @@ -74,7 +70,9 @@ export function useDebouncedFunction< timeoutIdRef.current = setTimeout( () => void callbackRef.current(...args), - debounceTimeRef.current, + typeof debounceTimeRef.current === "function" + ? debounceTimeRef.current(...args) + : debounceTimeRef.current, ); }, [cancelDebounce], diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index fc73bc45b5..3477ca2518 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -241,7 +241,19 @@ export const CreateWorkspacePageView: FC = ({ sendMessage(formInputs, ownerId); }, - 500, + ( + parameters: Array<{ parameter: PreviewParameter; value: string }>, + _ownerId?: string, + ) => { + // Return a debounce for string fields (those that involve typing) and + // zero debounce for all others (so the UI can react immediately). + return parameters.some( + ({ parameter }) => + parameter.form_type === "input" || parameter.form_type === "textarea", + ) + ? 500 + : 0; + }, ); useEffect(() => {