korbit-ai[bot] commented on code in PR #35801:
URL: https://github.com/apache/superset/pull/35801#discussion_r2452758260
##########
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx:
##########
@@ -411,16 +413,22 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> =
props => {
});
}
};
+
if (!datePicker) {
refreshComparatorSuggestions();
}
- }, [props.adhocFilter.subject]);
+ }, [
+ props.adhocFilter.subject,
+ props.adhocFilter.clause,
+ props.datasource,
+ datePicker,
+ ]);
useEffect(() => {
if (isFeatureEnabled(FeatureFlag.EnableAdvancedDataTypes)) {
fetchSubjectAdvancedDataType(props);
}
- }, [props.adhocFilter.subject]);
+ }, [props.adhocFilter.subject, fetchSubjectAdvancedDataType, props]);
Review Comment:
### Overly broad useEffect dependencies causing unnecessary re-renders
<sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The useEffect dependency array includes the entire `props` object, which
will cause the effect to run on every render when any prop changes, not just
when the relevant properties change.
###### Why this matters
This creates unnecessary re-renders and API calls to
fetchSubjectAdvancedDataType, potentially causing performance issues and race
conditions. The effect should only run when specific properties that affect the
advanced data type fetching actually change.
###### Suggested change ∙ *Feature Preview*
Replace the broad `props` dependency with specific properties that actually
affect the advanced data type fetching:
```typescript
useEffect(() => {
if (isFeatureEnabled(FeatureFlag.EnableAdvancedDataTypes)) {
fetchSubjectAdvancedDataType(props);
}
}, [props.adhocFilter.subject, props.datasource,
fetchSubjectAdvancedDataType]);
```
Or if fetchSubjectAdvancedDataType is stable and doesn't need to be in
dependencies:
```typescript
useEffect(() => {
if (isFeatureEnabled(FeatureFlag.EnableAdvancedDataTypes)) {
fetchSubjectAdvancedDataType(props);
}
}, [props.adhocFilter.subject, props.datasource]);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3b976ac7-69da-4f11-9c88-e31f0a6343d2/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3b976ac7-69da-4f11-9c88-e31f0a6343d2?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3b976ac7-69da-4f11-9c88-e31f0a6343d2?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3b976ac7-69da-4f11-9c88-e31f0a6343d2?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3b976ac7-69da-4f11-9c88-e31f0a6343d2)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:46f5364b-b059-45af-aca0-4ab953a12c13 -->
[](46f5364b-b059-45af-aca0-4ab953a12c13)
--
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]