Chore: Fix eslint react hook warnings in grafana-ui (#31092)

* Chore: Fix eslint react hook warnings in grafana-ui

* Remove unneeded exlcudes

* Address Andrej's review

.eslintrc

@@ -11,8 +11,7 @@
   "overrides": [
     {
       "files": [
-        "packages/grafana-ui/**/*/!(*.story).{ts,tsx}",
-        "packages/jaeger-ui-components/**/*.{ts,tsx,js}",
+        "packages/grafana-ui/src/components/uPlot/**/*.{ts,tsx}",
         "public/app/**/*.{ts,tsx}"
       ],
       "rules": {

packages/grafana-ui/src/components/CustomScrollbar/CustomScrollbar.tsx

@@ -54,16 +54,18 @@ export const CustomScrollbar: FC<Props> = ({
    * Special logic for doing a update a few milliseconds after mount to check for
    * updated height due to dynamic content
    */
-  if (updateAfterMountMs) {
-    useEffect(() => {
-      setTimeout(() => {
-        const scrollbar = ref.current as any;
-        if (scrollbar?.update) {
-          scrollbar.update();
-        }
-      }, updateAfterMountMs);
-    }, []);
-  }
+
+  useEffect(() => {
+    if (!updateAfterMountMs) {
+      return;
+    }
+    setTimeout(() => {
+      const scrollbar = ref.current as any;
+      if (scrollbar?.update) {
+        scrollbar.update();
+      }
+    }, updateAfterMountMs);
+  }, [updateAfterMountMs]);
 
   function renderTrack(className: string, hideTrack: boolean | undefined, passedProps: any) {
     if (passedProps.style && hideTrack) {

packages/grafana-ui/src/components/DataSourceSettings/SigV4AuthSettings.tsx

@@ -46,6 +46,7 @@ export const SigV4AuthSettings: React.FC<HttpSettingsProps> = (props) => {
   useEffect(() => {
     const sigV4AuthType = dataSourceConfig.jsonData.sigV4AuthType || 'default';
     onJsonDataChange('sigV4AuthType', sigV4AuthType);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
   const onSecureJsonDataReset = (fieldName: string) => {

packages/grafana-ui/src/components/MatchersUI/FieldNamesMatcherEditor.tsx

@@ -17,11 +17,6 @@ export const FieldNamesMatcherEditor = memo<MatcherUIProps<ByNamesMatcherOptions
   const names = useFieldDisplayNames(data);
   const selectOptions = useSelectOptions(names);
 
-  if (readOnly) {
-    const displayNames = (options.names ?? []).join(', ');
-    return <Input value={displayNames} readOnly={true} disabled={true} prefix={prefix} />;
-  }
-
   const onChange = useCallback(
     (selections: Array<SelectableValue<string>>) => {
       if (!Array.isArray(selections)) {
@@ -39,9 +34,14 @@ export const FieldNamesMatcherEditor = memo<MatcherUIProps<ByNamesMatcherOptions
         }, []),
       });
     },
-    [names, onChangeFromProps]
+    [names, onChangeFromProps, options]
   );
 
+  if (readOnly) {
+    const displayNames = (options.names ?? []).join(', ');
+    return <Input value={displayNames} readOnly={true} disabled={true} prefix={prefix} />;
+  }
+
   return <MultiSelect value={options.names} options={selectOptions} onChange={onChange} />;
 });
 FieldNamesMatcherEditor.displayName = 'FieldNameMatcherEditor';

packages/grafana-ui/src/components/Modal/Modal.tsx

@@ -42,13 +42,12 @@ export function Modal(props: PropsWithChildren<Props>): ReturnType<FC<Props>> {
     }
   }, [propsOnDismiss]);
 
-  const onEscKey = (ev: KeyboardEvent) => {
-    if (ev.key === 'Esc' || ev.key === 'Escape') {
-      onDismiss();
-    }
-  };
-
   useEffect(() => {
+    const onEscKey = (ev: KeyboardEvent) => {
+      if (ev.key === 'Esc' || ev.key === 'Escape') {
+        onDismiss();
+      }
+    };
     if (isOpen && closeOnEscape) {
       document.addEventListener('keydown', onEscKey, false);
     } else {
@@ -57,7 +56,7 @@ export function Modal(props: PropsWithChildren<Props>): ReturnType<FC<Props>> {
     return () => {
       document.removeEventListener('keydown', onEscKey, false);
     };
-  }, [closeOnEscape, isOpen]);
+  }, [closeOnEscape, isOpen, onDismiss]);
 
   if (!isOpen) {
     return null;

packages/grafana-ui/src/components/NodeGraph/NodeGraph.tsx

@@ -84,11 +84,14 @@ export function NodeGraph({ getLinks, dataFrames, nodeLimit }: Props) {
   const [edgeHover, setEdgeHover] = useState<string | undefined>(undefined);
   const clearEdgeHover = useCallback(() => setEdgeHover(undefined), [setEdgeHover]);
 
+  const firstNodesDataFrame = nodesDataFrames[0];
+  const firstEdgesDataFrame = edgesDataFrames[0];
+
   // TODO we should be able to allow multiple dataframes for both edges and nodes, could be issue with node ids which in
   //  that case should be unique or figure a way to link edges and nodes dataframes together.
-  const processed = useMemo(() => processNodes(nodesDataFrames[0], edgesDataFrames[0]), [
-    nodesDataFrames[0],
-    edgesDataFrames[0],
+  const processed = useMemo(() => processNodes(firstNodesDataFrame, firstEdgesDataFrame), [
+    firstEdgesDataFrame,
+    firstNodesDataFrame,
   ]);
 
   const { nodes: rawNodes, edges: rawEdges } = useNodeLimit(processed.nodes, processed.edges, nodeCountLimit);

packages/grafana-ui/src/components/NodeGraph/useNodeLimit.ts

@@ -46,5 +46,5 @@ export function useNodeLimit(
     const newEdges = edges.filter((e) => newNodes[e.source as string] && newNodes[e.target as string]);
 
     return { nodes: Object.values(newNodes), edges: newEdges };
-  }, [edges, nodes]);
+  }, [edges, limit, nodes]);
 }

packages/grafana-ui/src/components/NodeGraph/usePanning.ts

@@ -18,7 +18,7 @@ interface Options {
  * Based on https://github.com/streamich/react-use/blob/master/src/useSlider.ts
  * Returns position x/y coordinates which can be directly used in transform: translate().
  * @param scale Can be used when we want to scale the movement if we are moving a scaled element. We need to do it
- *   here because we don't wont to change the pos when scale changes.
+ *   here because we don't want to change the pos when scale changes.
  * @param bounds If set the panning cannot go outside of those bounds.
  */
 export function usePanning<T extends Element>(
@@ -105,17 +105,18 @@ export function usePanning<T extends Element>(
       });
     };
 
-    if (panRef.current) {
-      panRef.current.addEventListener('mousedown', onPanStart);
-      panRef.current.addEventListener('touchstart', onPanStart);
+    const ref = panRef.current;
+    if (ref) {
+      ref.addEventListener('mousedown', onPanStart);
+      ref.addEventListener('touchstart', onPanStart);
     }
     return () => {
-      if (panRef.current) {
-        panRef.current.removeEventListener('mousedown', onPanStart);
-        panRef.current.removeEventListener('touchstart', onPanStart);
+      if (ref) {
+        ref.removeEventListener('mousedown', onPanStart);
+        ref.removeEventListener('touchstart', onPanStart);
       }
     };
-  }, [scale, bounds?.left, bounds?.right, bounds?.top, bounds?.bottom]);
+  }, [scale, bounds?.left, bounds?.right, bounds?.top, bounds?.bottom, isMounted]);
 
   return { state, ref: panRef };
 }

packages/grafana-ui/src/components/NodeGraph/useZoom.ts

@@ -64,18 +64,21 @@ export function useZoom({ stepUp, stepDown, min, max } = defaultOptions) {
   );
 
   useEffect(() => {
-    if (ref.current) {
-      // Adds listener for wheel event, we need the passive: false to be able to prevent default otherwise that
-      // cannot be used with passive listeners.
-      ref.current.addEventListener('wheel', onWheel, { passive: false });
-      return () => {
-        if (ref.current) {
-          ref.current.removeEventListener('wheel', onWheel);
-        }
-      };
+    if (!ref.current) {
+      return;
     }
-    return undefined;
-  }, [ref.current, onWheel]);
+
+    const zoomRef = ref.current;
+
+    // Adds listener for wheel event, we need the passive: false to be able to prevent default otherwise that
+    // cannot be used with passive listeners.
+    zoomRef.addEventListener('wheel', onWheel, { passive: false });
+    return () => {
+      if (zoomRef) {
+        zoomRef.removeEventListener('wheel', onWheel);
+      }
+    };
+  }, [onWheel]);
 
   return {
     onStepUp,

packages/grafana-ui/src/components/Slider/Slider.tsx

@@ -24,7 +24,7 @@ export const Slider: FunctionComponent<SliderProps> = ({
   const theme = useTheme();
   const styles = getStyles(theme, isHorizontal);
   const SliderWithTooltip = SliderComponent;
-  const [slidervalue, setSliderValue] = useState<number>(value || min);
+  const [sliderValue, setSliderValue] = useState<number>(value || min);
 
   const onSliderChange = useCallback(
     (v: number) => {
@@ -58,7 +58,7 @@ export const Slider: FunctionComponent<SliderProps> = ({
         onAfterChange(v);
       }
     },
-    [setSliderValue, onAfterChange]
+    [max, min, onChange, onAfterChange]
   );
 
   const sliderInputClassNames = !isHorizontal ? [styles.sliderInputVertical] : [];
@@ -74,7 +74,7 @@ export const Slider: FunctionComponent<SliderProps> = ({
           max={max}
           step={step}
           defaultValue={value}
-          value={slidervalue}
+          value={sliderValue}
           onChange={onSliderChange}
           onAfterChange={onAfterChange}
           vertical={!isHorizontal}
@@ -84,7 +84,7 @@ export const Slider: FunctionComponent<SliderProps> = ({
         <Input
           type="text"
           className={cx(styles.sliderInputField, ...sliderInputFieldClassNames)}
-          value={`${slidervalue}`} // to fix the react leading zero issue
+          value={`${sliderValue}`} // to fix the react leading zero issue
           onChange={onSliderInputChange}
           min={min}
           max={max}

packages/grafana-ui/src/components/VizLayout/VizLayout.tsx

@@ -29,13 +29,14 @@ export const VizLayout: VizLayoutComponentType = ({ width, height, legend, child
     width: `${width}px`,
     height: `${height}px`,
   };
+  const [legendRef, legendMeasure] = useMeasure();
 
   if (!legend) {
     return <div style={containerStyle}>{children(width, height)}</div>;
   }
 
   const { placement, maxHeight, maxWidth } = legend.props;
-  const [legendRef, legendMeasure] = useMeasure();
+
   let size: VizSize | null = null;
 
   const vizStyle: CSSProperties = {
@@ -64,7 +65,7 @@ export const VizLayout: VizLayoutComponentType = ({ width, height, legend, child
   }
 
   // This happens when position is switched from bottom to right
-  // Then we preserve old with for one render cycle until lenged is measured in it's new position
+  // Then we preserve old with for one render cycle until legend is measured in it's new position
   if (size?.width === 0) {
     size.width = width;
   }