justinpark opened a new pull request, #23871: URL: https://github.com/apache/superset/pull/23871
### SUMMARY We often to update(add) the useEffect dependencies to fix the dependency warnings (i.e. #22974) This work has potential performance degrade issue. For example, #22974 updated the following useEffect dependancies. https://github.com/apache/superset/blob/097f47519433aa7892f1993a33afb3e0a12296a4/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx#L382-L397 This means `setHeight` and window event allocation can be triggered anytime handleWindowResizeWithThrottle/onBeforeUnload or/and updated. The problem is `onBeforeUnload` only has dependancies by useCallback https://github.com/apache/superset/blob/097f47519433aa7892f1993a33afb3e0a12296a4/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx#L365-L380 So whenever database is updated or latestQuery state updated, the above useEffect triggers the additional setHeight(which triggers re-rendering) and new event allocation (no memory leaked but unnecessary addEventListener/removeEventListener operation triggered) This is common issue on react useEffect usage, so there's an official solution (useEffectEvent) is introduced. For more detail, please check [the blog link /w video clip](https://morioh.com/p/994ce3afc1d3) or [official proposal document](https://github.com/reactjs/rfcs/blob/useevent/text/0000-useevent.md) In addition to introduce the useEffectEvent, this commit also tune up L365-L380 to avoid the unnecessary setHeight triggered during database is updated. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A ### TESTING INSTRUCTIONS Resize sqllab editor to verify the query panel height adjusted accordingly. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [x] Introduces new feature or API - [ ] Removes existing feature or API -- 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]
