codeant-ai-for-open-source[bot] commented on code in PR #35218:
URL: https://github.com/apache/superset/pull/35218#discussion_r3624193199
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx:
##########
@@ -471,63 +471,101 @@ const Select = forwardRef(
const handleFilterOption = (search: string, option: AntdLabeledValue) =>
handleFilterOptionHelper(search, option, optionFilterProps,
filterOption);
- const handleOnSearch = debounce((search: string) => {
- const searchValue = search.trim();
- setIsSearching(!!searchValue);
+ const stateRef = useRef({
+ selectOptions,
+ allowNewOptions,
+ fullSelectOptions,
+ selectValue,
+ handleFilterOption,
+ onSearch,
+ });
- let updatedOptions = selectOptions;
+ useEffect(() => {
+ stateRef.current = {
+ selectOptions,
+ allowNewOptions,
+ fullSelectOptions,
+ selectValue,
+ handleFilterOption,
+ onSearch,
+ };
+ });
- if (allowNewOptions) {
- const optionsWithoutTemporary =
ensureIsArray(fullSelectOptions).filter(
- opt => !opt.isNewOption,
- );
- const unquotedSearch = stripSurroundingQuotes(searchValue);
- const shouldCreateNewOption =
- unquotedSearch &&
- !hasOption(unquotedSearch, optionsWithoutTemporary, true);
-
- const newOption = shouldCreateNewOption && {
- label: unquotedSearch,
- value: unquotedSearch,
- isNewOption: true,
- };
- const cleanSelectOptions = ensureIsArray(fullSelectOptions).filter(
- opt => !opt.isNewOption || hasOption(opt.value, selectValue),
- );
- updatedOptions = newOption
- ? [newOption, ...cleanSelectOptions]
- : cleanSelectOptions;
- setSelectOptions(updatedOptions);
- }
+ const handleOnSearch = useMemo(
+ () =>
+ debounce((search: string) => {
+ const {
+ selectOptions,
+ allowNewOptions,
+ fullSelectOptions,
+ selectValue,
+ handleFilterOption,
+ onSearch,
+ } = stateRef.current;
+
+ const searchValue = search.trim();
+ setIsSearching(!!searchValue);
+
+ let updatedOptions = selectOptions;
+
+ if (allowNewOptions) {
+ const optionsWithoutTemporary = ensureIsArray(
+ fullSelectOptions,
+ ).filter(opt => !opt.isNewOption);
+ const unquotedSearch = stripSurroundingQuotes(searchValue);
+ const shouldCreateNewOption =
+ unquotedSearch &&
+ !hasOption(unquotedSearch, optionsWithoutTemporary, true);
+
+ const newOption = shouldCreateNewOption && {
+ label: unquotedSearch,
+ value: unquotedSearch,
+ isNewOption: true,
+ };
+ const cleanSelectOptions = ensureIsArray(fullSelectOptions).filter(
+ opt => !opt.isNewOption || hasOption(opt.value, selectValue),
+ );
+ updatedOptions = newOption
+ ? [newOption, ...cleanSelectOptions]
+ : cleanSelectOptions;
+ setSelectOptions(updatedOptions);
+ }
- const filteredOptions = updatedOptions
- .map((option: any) => {
- /*
+ const filteredOptions = updatedOptions
+ .map((option: DefaultOptionType) => {
+ /*
If it's a group, filter its nested options and only return it
if it has matching options
*/
- if ('options' in option && Array.isArray(option.options)) {
- const filteredGroupOptions = option.options.filter(
- (subOption: AntdLabeledValue) =>
- handleFilterOption(search, subOption),
- );
- return filteredGroupOptions.length > 0
- ? { ...option, options: filteredGroupOptions }
- : null;
- }
-
- return handleFilterOption(search, option as AntdLabeledValue)
- ? option
- : null;
- })
- .filter((option): option is AntdLabeledValue => option !== null);
+ if ('options' in option && Array.isArray(option.options)) {
+ const filteredGroupOptions = option.options.filter(
+ (subOption: AntdLabeledValue) =>
+ handleFilterOption(search, subOption),
+ );
+ return filteredGroupOptions.length > 0
+ ? { ...option, options: filteredGroupOptions }
+ : null;
+ }
- setVisibleOptions(filteredOptions);
- setInputValue(searchValue);
- onSearch?.(searchValue);
- }, Constants.FAST_DEBOUNCE);
+ return handleFilterOption(search, option as AntdLabeledValue)
+ ? option
+ : null;
Review Comment:
**Suggestion:** The filtering logic uses the untrimmed `search` string while
the component state and callbacks use the trimmed `searchValue`, which creates
inconsistent behavior for inputs with leading/trailing whitespace (for example,
options can be filtered out even though the displayed input has been trimmed).
Use the same normalized search term for filtering and state updates. [logic
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Search results inconsistent when user types trailing spaces.
- ⚠️ Dashboard filter selects may hide valid matching options.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render the `Select` component from
`superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx`
with search
enabled (this wires Ant Design's `onSearch` into `handleOnSearch` defined at
lines
494–559).
2. Type a query with trailing whitespace into the search box (e.g., "US ").
Ant Design
passes this raw string as `search` to `handleOnSearch` at line 496.
3. Inside `handleOnSearch`, `searchValue = search.trim()` (line 506) is used
for state
updates: `setInputValue(searchValue)` and `onSearch?.(searchValue)` at lines
557–558, so
the visible input and external callbacks receive the trimmed "US".
4. Still in `handleOnSearch`, filtering uses the original `search` value
(untrimmed "US ")
via `handleFilterOption(search, ...)` at lines 542–543 and 550–551. If the
`filterOption`
/ `handleFilterOptionHelper` logic compares strings without trimming, the
option matching
"US" will not be included in `filteredOptions`, causing the list of visible
options (line
556) to be inconsistent with the displayed trimmed search text.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f05b812117d648f1b51f8a6a53e7789c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f05b812117d648f1b51f8a6a53e7789c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
**Line:** 541:552
**Comment:**
*Logic Error: The filtering logic uses the untrimmed `search` string
while the component state and callbacks use the trimmed `searchValue`, which
creates inconsistent behavior for inputs with leading/trailing whitespace (for
example, options can be filtered out even though the displayed input has been
trimmed). Use the same normalized search term for filtering and state updates.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35218&comment_hash=0de2bd1a7e7c0bc85c99b0168ec7aeaeaf73a813671fc6fafe1706f7a4f60223&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35218&comment_hash=0de2bd1a7e7c0bc85c99b0168ec7aeaeaf73a813671fc6fafe1706f7a4f60223&reaction=dislike'>👎</a>
--
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]
