Re: [PR] fix: dashboard filter regress [superset]

["rusackas (via GitHub)" <gi...@apache.org>]

rusackas commented on PR #35218:
URL: https://github.com/apache/superset/pull/35218#issuecomment-4753633843

   @SBIN2010 this still has my prior changes outstanding, and the core issue stands: dropping the `() =>` means `handleOnSearch.cancel()` now fires in the effect body on every dep change instead of in the cleanup on unmount — that doesn't fix #28174 and likely makes the flicker worse. 
   
   The new test also asserts `onSearch` WAS called after `unmount()`, which is the broken behavior — it should assert it was NOT. And `pre-commit` + `sharded-jest-tests (6)` are failing on your own code, not flakiness. The shape I'd expect is `useEffect(() => () => handleOnSearch?.cancel(), [handleOnSearch])`, ideally with `handleOnSearch` wrapped in `useMemo` so cleanup only runs on unmount, and the same fix mirrored in `AsyncSelect.tsx`.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org