[7.3] [APM] Add react-hooks lint rules for APM folder, fix dep… (#42182)

* [APM] Add react-hooks lint rules for APM folder, fix dependencies (#42129) * [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 * Fix type errors
elastic · Aug 2, 2019 · 9903479 · 9903479
1 parent ede2e46
commit 9903479
Show file tree

Hide file tree

Showing 9 changed files with 50 additions and 28 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -381,6 +381,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/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>> = [
     {
       field: 'name',

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
@@ -72,21 +72,22 @@ export function TransactionOverview({
 
   const { data: transactionCharts } = useTransactionCharts();
 
+  const {
+    data: transactionListData,
+    status: transactionListStatus
+  } = useTransactionList(urlParams);
+  const { data: hasMLJob = false } = useFetcher(() => {
+    return serviceName && transactionType
+      ? getHasMLJob({ serviceName, transactionType })
+      : undefined;
+  }, [serviceName, transactionType]);
+
   // TODO: improve urlParams typings.
   // `serviceName` or `transactionType` will never be undefined here, and this check should not be needed
   if (!serviceName || !transactionType) {
     return null;
   }
 
-  const {
-    data: transactionListData,
-    status: transactionListStatus
-  } = useTransactionList(urlParams);
-  const { data: hasMLJob = false } = useFetcher(
-    () => getHasMLJob({ serviceName, transactionType }),
-    [serviceName, transactionType]
-  );
-
   return (
     <React.Fragment>
       {serviceTransactionTypes.length > 1 ? (
@@ -139,7 +140,6 @@ export function TransactionOverview({
         <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
@@ -72,15 +72,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
@@ -39,17 +39,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 {