[7.x] [APM] Add react-hooks lint rules for APM folder, fix dep… (#42178)

* [APM] Add react-hooks lint rules for APM folder, fix dependencies Closes #42128. * Validate useFetcher dependencies as well * Add useFetcher hook; move eslint config to kibana eslint config
elastic · Jul 31, 2019 · 9f83008 · 9f83008
1 parent aacb51e
commit 9f83008
Show file tree

Hide file tree

Showing 11 changed files with 45 additions and 23 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -396,6 +396,14 @@ module.exports = {
         'no-console': ['warn', { allow: ['error'] }],
       },
     },
+    {
+      plugins: ['react-hooks'],
+      files: ['x-pack/legacy/plugins/apm/**/*.{ts,tsx}'],
+      rules: {
+        'react-hooks/rules-of-hooks': 'error', // Checks rules of Hooks
+        'react-hooks/exhaustive-deps': ['error', { additionalHooks: '^useFetcher$' }],
+      },
+    },
 
     /**
      * GIS overrides

diff --git a/x-pack/legacy/plugins/apm/public/components/app/Main/useUpdateBadgeEffect.ts b/x-pack/legacy/plugins/apm/public/components/app/Main/useUpdateBadgeEffect.ts
@@ -11,6 +11,7 @@ import { useCore } from '../../../hooks/useCore';
 
 export const useUpdateBadgeEffect = () => {
   const { chrome } = useCore();
+
   useEffect(() => {
     const uiCapabilities = capabilities.get();
     chrome.setBadge(
@@ -26,5 +27,5 @@ export const useUpdateBadgeEffect = () => {
           }
         : undefined
     );
-  }, []);
+  }, [chrome]);
 };
diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceOverview/index.tsx b/x-pack/legacy/plugins/apm/public/components/app/ServiceOverview/index.tsx
@@ -70,7 +70,7 @@ export function ServiceOverview() {
         )
       });
     }
-  }, [data.hasLegacyData]);
+  }, [data.hasLegacyData, core.http.basePath]);
 
   useTrackPageview({ app: 'apm', path: 'services_overview' });
   useTrackPageview({ app: 'apm', path: 'services_overview', delay: 15000 });

diff --git a/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx b/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx
@@ -47,10 +47,6 @@ export function AddSettingsFlyout({
   onSubmit,
   selectedConfig
 }: Props) {
-  if (!isOpen) {
-    return null;
-  }
-
   const [environment, setEnvironment] = useState<string | undefined>(
     selectedConfig
       ? selectedConfig.service.environment || ENVIRONMENT_NOT_DEFINED
@@ -88,6 +84,11 @@ export function AddSettingsFlyout({
   const hasCorrectDecimals = Number.isInteger(sampleRateFloat * 1000);
   const isSampleRateValid =
     sampleRateFloat >= 0 && sampleRateFloat <= 1 && hasCorrectDecimals;
+
+  if (!isOpen) {
+    return null;
+  }
+
   return (
     <EuiPortal>
       <EuiFlyout size="s" onClose={onClose} ownFocus={true}>

diff --git a/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/List/index.tsx b/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/List/index.tsx
@@ -25,11 +25,10 @@ const TransactionNameLink = styled(TransactionLink)`
 
 interface Props {
   items: ITransactionGroup[];
-  serviceName: string;
   isLoading: boolean;
 }
 
-export function TransactionList({ items, serviceName, isLoading }: Props) {
+export function TransactionList({ items, isLoading }: Props) {
   const columns: Array<ITableColumn<ITransactionGroup>> = useMemo(
     () => [
       {
@@ -39,7 +38,7 @@ export function TransactionList({ items, serviceName, isLoading }: Props) {
         }),
         width: '50%',
         sortable: true,
-        render: (transactionName: string, item: typeof items[0]) => {
+        render: (transactionName: string, item: ITransactionGroup) => {
           return (
             <EuiToolTip
               id="transaction-name-link-tooltip"
@@ -104,7 +103,7 @@ export function TransactionList({ items, serviceName, isLoading }: Props) {
         render: (value: number) => <ImpactBar value={value} />
       }
     ],
-    [serviceName]
+    []
   );
 
   const noItemsMessage = (

diff --git a/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/index.tsx b/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/index.tsx
@@ -152,7 +152,6 @@ export function TransactionOverview({ urlParams }: Props) {
         <TransactionList
           isLoading={transactionListStatus === 'loading'}
           items={transactionListData}
-          serviceName={serviceName}
         />
       </EuiPanel>
     </React.Fragment>

diff --git a/x-pack/legacy/plugins/apm/public/components/shared/KueryBar/index.tsx b/x-pack/legacy/plugins/apm/public/components/shared/KueryBar/index.tsx
@@ -120,15 +120,19 @@ export function KueryBar() {
     let didCancel = false;
 
     async function loadIndexPattern() {
-      setState({ ...state, isLoadingIndexPattern: true });
+      setState(value => ({ ...value, isLoadingIndexPattern: true }));
       const indexPattern = await getAPMIndexPatternForKuery();
       if (didCancel) {
         return;
       }
       if (!indexPattern) {
-        setState({ ...state, isLoadingIndexPattern: false });
+        setState(value => ({ ...value, isLoadingIndexPattern: false }));
       } else {
-        setState({ ...state, indexPattern, isLoadingIndexPattern: false });
+        setState(value => ({
+          ...value,
+          indexPattern,
+          isLoadingIndexPattern: false
+        }));
       }
     }
     loadIndexPattern();

diff --git a/x-pack/legacy/plugins/apm/public/context/UrlParamsContext/index.tsx b/x-pack/legacy/plugins/apm/public/context/UrlParamsContext/index.tsx
@@ -47,17 +47,19 @@ const UrlParamsProvider: React.ComponentClass<{}> = withRouter(
   ({ location, children }) => {
     const refUrlParams = useRef(resolveUrlParams(location, {}));
 
+    const { start, end, rangeFrom, rangeTo } = refUrlParams.current;
+
     const [, forceUpdate] = useState('');
 
     const urlParams = useMemo(
       () =>
         resolveUrlParams(location, {
-          start: refUrlParams.current.start,
-          end: refUrlParams.current.end,
-          rangeFrom: refUrlParams.current.rangeFrom,
-          rangeTo: refUrlParams.current.rangeTo
+          start,
+          end,
+          rangeFrom,
+          rangeTo
         }),
-      [location, refUrlParams.current]
+      [location, start, end, rangeFrom, rangeTo]
     );
 
     refUrlParams.current = urlParams;

diff --git a/x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx b/x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx
@@ -74,14 +74,22 @@ export function useFetcher<Response>(
       dispatchStatus({ id, isLoading: false });
       didCancel = true;
     };
-  }, [...effectKey, counter]);
+    /* eslint-disable react-hooks/exhaustive-deps */
+  }, [
+    counter,
+    id,
+    preservePreviousResponse,
+    dispatchStatus,
+    ...effectKey
+    /* eslint-enable react-hooks/exhaustive-deps */
+  ]);
 
   return useMemo(
     () => ({
       ...result,
       refresh: () => {
         // this will invalidate the effectKey and will result in a new request
-        setCounter(counter + 1);
+        setCounter(count => count + 1);
       }
     }),
     [result]

diff --git a/x-pack/legacy/plugins/apm/public/hooks/useTransactionBreakdown.ts b/x-pack/legacy/plugins/apm/public/hooks/useTransactionBreakdown.ts
@@ -30,7 +30,7 @@ export function useTransactionBreakdown() {
         uiFilters
       });
     }
-  }, [serviceName, start, end, uiFilters]);
+  }, [serviceName, start, end, transactionType, transactionName, uiFilters]);
 
   const receivedDataDuringLifetime = useRef(false);
 

diff --git a/x-pack/legacy/plugins/apm/public/hooks/useTransactionCharts.ts b/x-pack/legacy/plugins/apm/public/hooks/useTransactionCharts.ts
@@ -31,7 +31,7 @@ export function useTransactionCharts() {
 
   const memoizedData = useMemo(
     () => getTransactionCharts({ transactionType }, data),
-    [data]
+    [data, transactionType]
   );
 
   return {