villebro commented on a change in pull request #13484:
URL: https://github.com/apache/superset/pull/13484#discussion_r590377767
##########
File path:
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
##########
@@ -89,18 +89,16 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
useEffect(() => {
handleChange(currentValue ?? []);
- }, [JSON.stringify(currentValue)]);
+ }, [JSON.stringify(currentValue), multiSelect, enableEmptyFilter,
inverseSelection]);
useEffect(() => {
handleChange(defaultValue ?? []);
- // I think after Config Modal update some filter it re-creates default
value for all other filters
- // so we can process it like this `JSON.stringify` or start to use `Immer`
- }, [JSON.stringify(defaultValue)]);
+ }, [JSON.stringify(defaultValue), multiSelect, enableEmptyFilter,
inverseSelection]);
Review comment:
This looks really promising - however, reading this I think we should
consider if there are alternative ways of doing this:
> WARNING: Please only use this if you really can't find a way to use
React.useEffect. There's often a better way to do what you're trying to do than
a deep comparison.
I'll leave this as-is to not put more refactoring into this PR (there's
multiple instances of this logic here from before), but I'll keep this in mind
in future PRs that touch this code. FYI @simcha90
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
