eric-briscoe commented on code in PR #21667:
URL: https://github.com/apache/superset/pull/21667#discussion_r984971207
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.jsx:
##########
@@ -229,13 +229,20 @@ const SqlEditor = ({
}
};
- useState(() => {
+ const runQuery = () => {
+ if (database) {
+ startQuery();
+ }
+ };
+
+ // hack the useMemo hook to imitate componentWillMount
+ useMemo(() => {
Review Comment:
I think the `useEffect` hook be more appropriate here and not be considered
a hack, but recommended practice from React.
When using a `
`
useEffect() => {
}, [])
`
With empty array it tells the functional component to only run the effect
one time.
Notes section of:
https://reactjs.org/docs/hooks-effect.html#tip-optimizing-performance-by-skipping-effects
"If you want to run an effect and clean it up only once (on mount and
unmount), you can pass an empty array ([]) as a second argument. This tells
React that your effect doesn’t depend on any values from props or state, so it
never needs to re-run. This isn’t handled as a special case — it follows
directly from how the dependencies array always works."
Basically same exact code just swapping out the `useMemo` with `useEffect`
to align with React's best practices and remove the hack comment
--
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]
