[GitHub] [superset] eric-briscoe commented on a diff in pull request #21667: fix(sqllab): async query broken due to #21320

[GitBox <gi...@apache.org>]

eric-briscoe commented on code in PR #21667:
URL: https://github.com/apache/superset/pull/21667#discussion_r986072314


##########
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:
   @EugeneTorap `useEffect` is intended to replace all of the Class lifecycle methods so with functional components React is saying you no longer would try to differentiate between componentDidMount, componentDidUpdate, and componentWillUnmount, just use `useEffect`.
   
   `Tip If you’re familiar with React class lifecycle methods, you can think of useEffect Hook as componentDidMount, componentDidUpdate, and componentWillUnmount combined.` https://reactjs.org/docs/hooks-effect.html 
   
   useMemo is for a very specific purpose and even React recommends not using it until you have a working component and start experience performance issues. useMemo runs during render and is not intended to produce side effects.
   
   `Remember that the function passed to useMemo runs during rendering. Don’t do anything there that you wouldn’t normally do while rendering. For example, side effects belong in useEffect, not useMemo.` https://reactjs.org/docs/hooks-reference.html#usememo



-- 
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