refactor: useEffectEvent for efficient deps (#23871)

superset-frontend/package-lock.json

@@ -146,6 +146,7 @@
         "shortid": "^2.2.6",
         "tinycolor2": "^1.4.2",
         "urijs": "^1.19.8",
+        "use-event-callback": "^0.1.0",
         "use-immer": "^0.8.1",
         "use-query-params": "^1.1.9",
         "yargs": "^15.4.1"
@@ -56791,6 +56792,14 @@
         "react": "^16.8.0 || ^17.0.0"
       }
     },
+    "node_modules/use-event-callback": {
+      "version": "0.1.0",
+      "resolved": "https://registry.npmjs.org/use-event-callback/-/use-event-callback-0.1.0.tgz",
+      "integrity": "sha512-5fTzY5UEXHMK5UR0NRkUz6TPfWmmX9fO8Tx3SnHrfMPdrQ7Rna0gDBy0r56SP68TwsP9DgwSBzeysCu3A/Z2NA==",
+      "peerDependencies": {
+        "react": ">=16.8"
+      }
+    },
     "node_modules/use-immer": {
       "version": "0.8.1",
       "resolved": "https://registry.npmjs.org/use-immer/-/use-immer-0.8.1.tgz",
@@ -106393,6 +106402,12 @@
         "ts-essentials": "^2.0.3"
       }
     },
+    "use-event-callback": {
+      "version": "0.1.0",
+      "resolved": "https://registry.npmjs.org/use-event-callback/-/use-event-callback-0.1.0.tgz",
+      "integrity": "sha512-5fTzY5UEXHMK5UR0NRkUz6TPfWmmX9fO8Tx3SnHrfMPdrQ7Rna0gDBy0r56SP68TwsP9DgwSBzeysCu3A/Z2NA==",
+      "requires": {}
+    },
     "use-immer": {
       "version": "0.8.1",
       "resolved": "https://registry.npmjs.org/use-immer/-/use-immer-0.8.1.tgz",

superset-frontend/package.json

@@ -211,6 +211,7 @@
     "shortid": "^2.2.6",
     "tinycolor2": "^1.4.2",
     "urijs": "^1.19.8",
+    "use-event-callback": "^0.1.0",
     "use-immer": "^0.8.1",
     "use-query-params": "^1.1.9",
     "yargs": "^15.4.1"

superset-frontend/src/SqlLab/components/SqlEditor/index.jsx

@@ -25,6 +25,7 @@ import React, {
   useRef,
   useCallback,
 } from 'react';
+import useEffectEvent from 'src/hooks/useEffectEvent';
 import { CSSTransition } from 'react-transition-group';
 import { shallowEqual, useDispatch, useSelector } from 'react-redux';
 import PropTypes from 'prop-types';
@@ -354,39 +355,24 @@ const SqlEditor = ({
     return base;
   }, [dispatch, queryEditor.sql, startQuery, stopQuery]);
 
-  const handleWindowResize = useCallback(() => {
-    setHeight(getSqlEditorHeight());
-  }, []);
-
-  const handleWindowResizeWithThrottle = useMemo(
-    () => throttle(handleWindowResize, WINDOW_RESIZE_THROTTLE_MS),
-    [handleWindowResize],
-  );
-
-  const onBeforeUnload = useCallback(
-    event => {
-      if (
-        database?.extra_json?.cancel_query_on_windows_unload &&
-        latestQuery?.state === 'running'
-      ) {
-        event.preventDefault();
-        stopQuery();
-      }
-    },
-    [
-      database?.extra_json?.cancel_query_on_windows_unload,
-      latestQuery?.state,
-      stopQuery,
-    ],
-  );
+  const onBeforeUnload = useEffectEvent(event => {
+    if (
+      database?.extra_json?.cancel_query_on_windows_unload &&
+      latestQuery?.state === 'running'
+    ) {
+      event.preventDefault();
+      stopQuery();
+    }
+  });
 
   useEffect(() => {
     // We need to measure the height of the sql editor post render to figure the height of
     // the south pane so it gets rendered properly
     setHeight(getSqlEditorHeight());
-    if (!database || isEmpty(database)) {
-      setShowEmptyState(true);
-    }
+    const handleWindowResizeWithThrottle = throttle(
+      () => setHeight(getSqlEditorHeight()),
+      WINDOW_RESIZE_THROTTLE_MS,
+    );
 
     window.addEventListener('resize', handleWindowResizeWithThrottle);
     window.addEventListener('beforeunload', onBeforeUnload);
@@ -395,7 +381,14 @@ const SqlEditor = ({
       window.removeEventListener('resize', handleWindowResizeWithThrottle);
       window.removeEventListener('beforeunload', onBeforeUnload);
     };
-  }, [database, handleWindowResizeWithThrottle, onBeforeUnload]);
+    // TODO: Remove useEffectEvent deps once https://github.com/facebook/react/pull/25881 is released
+  }, [onBeforeUnload]);
+
+  useEffect(() => {
+    if (!database || isEmpty(database)) {
+      setShowEmptyState(true);
+    }
+  }, [database]);
 
   useEffect(() => {
     // setup hotkeys

superset-frontend/src/hooks/useEffectEvent.ts

@@ -0,0 +1,40 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+// TODO: Replace to react-use-event-hook once https://github.com/facebook/react/pull/25881 is released
+import useEventCallback from 'use-event-callback';
+
+declare type Fn<ARGS extends any[], R> = (...args: ARGS) => R;
+
+/**
+ * Similar to useCallback, with a few subtle differences:
+ * @external
+ * https://github.com/reactjs/rfcs/blob/useevent/text/0000-useevent.md#internal-implementation
+ * @example
+ * const onStateChanged = useEffectEvent((state: T) => log(['clicked', state]));
+ *
+ * useEffect(() => {
+ *   onStateChanged(state);
+ * }, [onStateChanged, state]);
+ * // ^ onStateChanged is guaranteed to never change and always be up to date!
+ */
+export default function useEffectEvent<A extends any[], R>(
+  fn: Fn<A, R>,
+): Fn<A, R> {
+  return useEventCallback(fn);
+}