bito-code-review[bot] commented on code in PR #44880:
URL: https://github.com/apache/superset/pull/44880#discussion_r4162826134
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -280,32 +296,58 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
const isDisabled =
appSection === AppSection.FilterConfigModal && defaultToFirstItem;
- const onSearch = useMemo(
- () =>
- debounce((search: string) => {
- setSearch(search);
- if (searchAllOptions) {
- dispatchDataMask({
- type: 'ownState',
- ownState: {
- coltypeMap: initialColtypeMap,
- // The dropdown offers `stripSurroundingQuotes(search)` as the
- // creatable option, so the server has to be asked for the same
- // string or the two disagree about what was searched for.
- search: stripSurroundingQuotes(search).trim(),
- },
- });
- }
- }, Constants.SLOW_DEBOUNCE),
- [dispatchDataMask, initialColtypeMap, searchAllOptions],
- );
+ const handleSearch = useCallback((value: string) => {
+ hasSearchedRef.current = true;
+ setSearchInput(value);
+ }, []);
+
+ // Read through a ref so a parent re-render that hands down new identities
+ // for these doesn't, by itself, re-fire the dispatch below -- only an
+ // actual search settling (a change to deferredSearchInput) should.
+ const searchDispatchRef = useRef({
+ searchAllOptions,
+ dispatchDataMask,
+ initialColtypeMap,
+ });
+ useEffect(() => {
+ searchDispatchRef.current = {
+ searchAllOptions,
+ dispatchDataMask,
+ initialColtypeMap,
+ };
+ });
+
+ useEffect(() => {
+ if (!hasSearchedRef.current) {
+ return;
+ }
+ const {
+ searchAllOptions: currentSearchAllOptions,
+ dispatchDataMask: currentDispatchDataMask,
+ initialColtypeMap: currentInitialColtypeMap,
+ } = searchDispatchRef.current;
+ if (!currentSearchAllOptions) {
+ return;
+ }
+ currentDispatchDataMask({
+ type: 'ownState',
+ ownState: {
+ coltypeMap: currentInitialColtypeMap,
+ // The dropdown offers `stripSurroundingQuotes(search)` as the
+ // creatable option, so the server has to be asked for the same
+ // string or the two disagree about what was searched for.
+ search: stripSurroundingQuotes(deferredSearchInput).trim(),
+ },
+ });
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [deferredSearchInput]);
Review Comment:
<!-- Bito Reply -->
Yes, you can retain a quiet-period debounce independently of the deferred
rendering. By introducing a separate `debounce` function for the `handleSearch`
logic, you can coalesce the `setSearchInput` updates (and consequently the
`ownState` dispatch) while still allowing `useDeferredValue` to handle the UI
rendering priority independently. This ensures that the server is only queried
after the user has paused typing for the specified duration, preventing
obsolete in-flight requests.
**superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx**
```
const debouncedSearch = useMemo(
() => debounce((value: string) => {
hasSearchedRef.current = true;
setSearchInput(value);
}, Constants.SLOW_DEBOUNCE),
[]
);
const handleSearch = useCallback((value: string) => {
debouncedSearch(value);
}, [debouncedSearch]);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
