Re: [PR] fix(native-filters): prevent circular dependencies and improve dependency handling [superset]

Sat, 18 Oct 2025 07:21:03 -0700 


korbit-ai[bot] commented on code in PR #35317:
URL: https://github.com/apache/superset/pull/35317#discussion_r2442441964


##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx:
##########
@@ -584,7 +584,10 @@ const FiltersConfigForm = (
     return Promise.reject(new Error(t('Pre-filter is required')));
   };
 
-  const availableFilters = getAvailableFilters(filterId);
+  const availableFilters = useMemo(
+    () => getAvailableFilters(filterId),
+    [getAvailableFilters, filterId, filters],
+  );

Review Comment:
   ### Memoization dependency mismatch with function parameters <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The useMemo dependency array includes `filters` but the memoized function 
`getAvailableFilters(filterId)` only takes `filterId` as a parameter, creating 
a potential mismatch between dependencies and actual function inputs.
   
   
   ###### Why this matters
   This could lead to unnecessary re-computations when `filters` changes but 
`getAvailableFilters` doesn't actually use the updated filters data, or 
conversely, stale memoized results if `getAvailableFilters` internally depends 
on filters data that isn't reflected in its parameters.
   
   ###### Suggested change ∙ *Feature Preview*
   Verify that `getAvailableFilters` function signature matches its actual 
dependencies. If it internally uses `filters` data, it should accept it as a 
parameter:
   
   ```typescript
   const availableFilters = useMemo(
     () => getAvailableFilters(filterId, filters),
     [getAvailableFilters, filterId, filters],
   );
   ```
   
   Or if `filters` is not needed, remove it from dependencies:
   
   ```typescript
   const availableFilters = useMemo(
     () => getAvailableFilters(filterId),
     [getAvailableFilters, filterId],
   );
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c18b0ece-49a4-4ee5-bf09-f8b0ee90da58/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c18b0ece-49a4-4ee5-bf09-f8b0ee90da58?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c18b0ece-49a4-4ee5-bf09-f8b0ee90da58?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c18b0ece-49a4-4ee5-bf09-f8b0ee90da58?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c18b0ece-49a4-4ee5-bf09-f8b0ee90da58)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:e0c66099-e590-47ee-921a-c6fce21b37e4 -->
   
   
   [](e0c66099-e590-47ee-921a-c6fce21b37e4)



-- 
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]

Reply via email to