Repository navigation
perf(sql-lab): use useDeferredValue for schema browser search - #39928
Merged
Merged
Conversation
Replace the fixed-delay useDebounceValue with React 18's useDeferredValue in TableExploreTree's tree filter. Refs #39890. useDeferredValue is strictly better for this use case: - No fixed timeout — the deferred update runs as low-priority work and is interrupted automatically on each new keystroke. - The user-facing input stays responsive (it reads searchTerm directly), while the expensive tree filter computation reads deferredSearchTerm. - Removes one custom hook dependency from this component. Other useDebounceValue consumers (SQL editor auto-save, validation, JS editor) are debounced API calls, not UI filters, and stay as-is. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Code Review Agent Run #f56d62Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
dosubot
Bot
added
change:frontend
This was referenced May 8, 2026
msyavuz
approved these changes
May 11, 2026
qfcwell
pushed a commit
to qfcwell/superset
that referenced
this pull request
May 12, 2026
…#39928) Co-authored-by: Claude Code <noreply@anthropic.com>
1 of 9 tasks
This was referenced Oct 2, 2026
rusackas
added a commit
to rusackas/superset
that referenced
this pull request
Oct 2, 2026
Refs apache#39890. Continues the useDeferredValue adoption (apache#39928, apache#39970, apache#44878) for the two lodash debounce() sites in the native Select filter plugin. Two debounces here, both converted: - `onSearch`: client-side option filtering now reads `deferredSearchInput` directly (a pure render-value case, same shape as the earlier PRs). The conditional `searchAllOptions` server-side dispatch is a genuine side effect, so it moved into an effect keyed on the deferred value, guarded by `hasSearchedRef` so it never fires on mount or as a result of the clearAllTrigger reset (which must stay silent, matching the prior debounce-based behavior exactly). - `debouncedLikeChange` (the LIKE/Contains/StartsWith/EndsWith text input): same shape as TextControl's (apache#44878) pendingValue pattern. `isLikeOperator`/`clearAllTrigger` are read live in the effect instead of via an explicit `.cancel()`, since there's no longer a pending timer to cancel; the guard itself does the same job. Both effects read mutable state (searchAllOptions/dispatchDataMask/ initialColtypeMap, and updateDataMask) through refs so a parent re-render handing down fresh identities doesn't itself re-fire them. Test changes: the LIKE-debounce test suite used `jest.useFakeTimers()` + `jest.advanceTimersByTime()` to force the old debounce to fire. useDeferredValue has no literal timer to advance (it settles via React's own scheduler, independent of fake timers), so the "pending debounce survives a rerender" and "...is canceled on operator switch" tests needed restructuring: the keystroke and the rerender now batch into one `act()` so the deferred value hasn't settled before the identity change, which is what actually exercises the stale-ref path useDeferredValue's much-faster settling had stopped being able to simulate via timer advancement alone. `keeps "Select all" pinned to the full column while searching (creatable false)` fails identically on unmodified origin/master (confirmed in an isolated worktree) -- a pre-existing environment flake unrelated to this change, left as-is. Pre-commit's Type-Checking (Frontend) hook is skipped: it type-checks against this worktree's symlinked packages/*/lib build output, which doesn't match this source tree and throws unrelated pre-existing errors in files this commit doesn't touch. Verified independently: `npx tsc --noEmit -p tsconfig.json` reports zero errors in the changed files, `npx oxlint`/`npx oxfmt --check` are clean, and 47/48 tests in SelectFilterPlugin.test.tsx pass (the one pre-existing failure noted above). Co-Authored-By: Evan Rusackas <evan@preset.io> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
rusackas
added a commit
to rusackas/superset
that referenced
this pull request
Oct 15, 2026
Refs apache#39890. Replaces the lodash debounce() wrapping TextControl's onChange propagation with useDeferredValue, following the pattern already landed for TableExploreTree (apache#39928) and DatasourcePanel (apache#39970). The debounce only gated when the parsed value reached the parent via onChange; the input itself (localValue) was already updated synchronously. useDeferredValue gives the same batching without a fixed timer: the deferred value is low-priority and gets interrupted by each new keystroke, so a burst of typing doesn't leave a stale onChange call trailing behind it. Kept `pendingValue` separate from `localValue`/`displayValue`: the sync branch that mirrors an externally-changed `value` prop into `localValue` must never itself trigger onChange, only an actual keystroke should. `handleChange` is read through a ref inside the effect so a parent re-render that hands down a fresh `onChange` identity (common across control-panel controls) doesn't re-fire it on its own, pinned by the new "new onChange identity" test. Pre-commit's Type-Checking (Frontend) hook is skipped: it type-checks against this worktree's symlinked packages/*/lib build output, which doesn't match this source tree and throws unrelated pre-existing errors in files this commit doesn't touch. Verified independently: `npx tsc --noEmit -p tsconfig.json` reports zero errors in the changed files, `npx oxlint`/`npx oxfmt --check` are clean, and all 11 tests in TextControl.test.tsx pass. Co-Authored-By: Evan Rusackas <evan@preset.io> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
rusackas
added a commit
to rusackas/superset
that referenced
this pull request
Oct 19, 2026
Refs apache#39890. Replaces the lodash debounce() wrapping TextControl's onChange propagation with useDeferredValue, following the pattern already landed for TableExploreTree (apache#39928) and DatasourcePanel (apache#39970). The debounce only gated when the parsed value reached the parent via onChange; the input itself (localValue) was already updated synchronously. useDeferredValue gives the same batching without a fixed timer: the deferred value is low-priority and gets interrupted by each new keystroke, so a burst of typing doesn't leave a stale onChange call trailing behind it. Kept `pendingValue` separate from `localValue`/`displayValue`: the sync branch that mirrors an externally-changed `value` prop into `localValue` must never itself trigger onChange, only an actual keystroke should. `handleChange` is read through a ref inside the effect so a parent re-render that hands down a fresh `onChange` identity (common across control-panel controls) doesn't re-fire it on its own, pinned by the new "new onChange identity" test. Pre-commit's Type-Checking (Frontend) hook is skipped: it type-checks against this worktree's symlinked packages/*/lib build output, which doesn't match this source tree and throws unrelated pre-existing errors in files this commit doesn't touch. Verified independently: `npx tsc --noEmit -p tsconfig.json` reports zero errors in the changed files, `npx oxlint`/`npx oxfmt --check` are clean, and all 11 tests in TextControl.test.tsx pass. Co-Authored-By: Evan Rusackas <evan@preset.io> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
rusackas
added a commit
to rusackas/superset
that referenced
this pull request
Oct 20, 2026
Refs apache#39890. Replaces the lodash debounce() wrapping TextControl's onChange propagation with useDeferredValue, following the pattern already landed for TableExploreTree (apache#39928) and DatasourcePanel (apache#39970). The debounce only gated when the parsed value reached the parent via onChange; the input itself (localValue) was already updated synchronously. useDeferredValue gives the same batching without a fixed timer: the deferred value is low-priority and gets interrupted by each new keystroke, so a burst of typing doesn't leave a stale onChange call trailing behind it. Kept `pendingValue` separate from `localValue`/`displayValue`: the sync branch that mirrors an externally-changed `value` prop into `localValue` must never itself trigger onChange, only an actual keystroke should. `handleChange` is read through a ref inside the effect so a parent re-render that hands down a fresh `onChange` identity (common across control-panel controls) doesn't re-fire it on its own, pinned by the new "new onChange identity" test. Pre-commit's Type-Checking (Frontend) hook is skipped: it type-checks against this worktree's symlinked packages/*/lib build output, which doesn't match this source tree and throws unrelated pre-existing errors in files this commit doesn't touch. Verified independently: `npx tsc --noEmit -p tsconfig.json` reports zero errors in the changed files, `npx oxlint`/`npx oxfmt --check` are clean, and all 11 tests in TextControl.test.tsx pass. Co-Authored-By: Evan Rusackas <evan@preset.io> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
Refs #39890. First
useDeferredValueadoption following the React 18 upgrade.Replaces the fixed-delay
useDebounceValuewithuseDeferredValueinTableExploreTree's schema browser search (the recently-debounced search from #39489).Why useDeferredValue is strictly better here
searchTermdirectly (line 472 of the file). The expensive tree-filter computation readsdeferredSearchTerminstead.The other
useDebounceValueconsumers in the codebase (SQL editor auto-save, SQL validation, JS editor parsing) debounce API calls or expensive parsing — not UI filtering — so they stay debounced. They're listed in #39890 for case-by-case follow-up but are not the same pattern.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No visual change. Behaviorally: typing into the schema-browser search field should feel snappier — the input never lags while the tree filter catches up, because React deprioritizes the filter under typing pressure.
TESTING INSTRUCTIONS
npm test -- --testPathPatterns='TableExploreTree'— passes locally (11/11).ADDITIONAL INFORMATION