Skip to content

fix(site): speed up state syncs and validate input for debounce hook logic #18877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 17, 2025

Conversation

Parkreiner
Copy link
Member

No issue to link – I'm basically pushing some updates upstream from the version of the hook I copied over for the Registry website.

Changes made

  • Updated debounce functions to have input validation for timeouts
  • Updated useDebouncedValue to flush state syncs immediately if timeout value is 0
  • Updated tests to reflect changes
  • Cleaned up some comments and parameter names to make things more clear

@Parkreiner Parkreiner self-assigned this Jul 15, 2025
value: T,
debounceTimeMs: number,
): T {
export function useDebouncedValue<T>(value: T, debounceTimeoutMs: number): T {
Copy link
Member Author

@Parkreiner Parkreiner Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the default parameter here because if you, the user of the hook, don't know the type of a value you're trying to debounce, you're probably doing something really wrong

);
}

const timeoutIdRef = useRef<number | undefined>(undefined);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated ref type to match the type used by the browser's setTimeout

@@ -12,7 +12,7 @@ afterAll(() => {
});

describe(`${useDebouncedValue.name}`, () => {
function renderDebouncedValue<T = unknown>(value: T, time: number) {
function renderDebouncedValue<T>(value: T, time: number) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default type parameter removed to match new type signature for the main hook

@Parkreiner Parkreiner requested a review from aslilac July 15, 2025 14:28
@@ -12,7 +12,7 @@ afterAll(() => {
});

describe(`${useDebouncedValue.name}`, () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
describe(`${useDebouncedValue.name}`, () => {
describe(useDebouncedValue.name, () => {

I know you didn't change this line, but just noticed

): UseDebouncedFunctionReturn<Args> {
if (!Number.isInteger(debounceTimeoutMs) || debounceTimeoutMs < 0) {
throw new Error(
`Provided debounce value ${debounceTimeoutMs} must be a non-negative integer`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provided debounce value -1 must be a non-negative integer

I'm not sure this wording makes much sense. how about something like...

Suggested change
`Provided debounce value ${debounceTimeoutMs} must be a non-negative integer`,
`Invalid value ${debounceTimeoutMS} for debounceTimeoutMs. Value must be greater than or equal to zero.`,

export function useDebouncedValue<T>(value: T, debounceTimeoutMs: number): T {
if (!Number.isInteger(debounceTimeoutMs) || debounceTimeoutMs < 0) {
throw new Error(
`Provided debounce value ${debounceTimeoutMs} must be a non-negative integer`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here

@Parkreiner Parkreiner changed the title fix(site): speed up state syncs and add input validation for debounce hook logic fix(site): speed up state syncs and validate input for debounce hook logic Jul 17, 2025
@Parkreiner Parkreiner merged commit f47efc6 into main Jul 17, 2025
33 checks passed
@Parkreiner Parkreiner deleted the mes/debounce-fix branch July 17, 2025 22:15
@github-actions github-actions bot locked and limited conversation to collaborators Jul 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants