[ML] Improve useEffects deps.

elastic · Aug 26, 2021 · 34e882c · 34e882c
1 parent acc4caa
commit 34e882c
Show file tree

Hide file tree

Showing 6 changed files with 209 additions and 178 deletions.
diff --git a/x-pack/plugins/apm/public/components/app/correlations/failed_transactions_correlations.tsx b/x-pack/plugins/apm/public/components/app/correlations/failed_transactions_correlations.tsx
@@ -41,7 +41,6 @@ import { CorrelationsLog } from './correlations_log';
 import { CorrelationsEmptyStatePrompt } from './empty_state_prompt';
 import { CrossClusterSearchCompatibilityWarning } from './cross_cluster_search_warning';
 import { CorrelationsProgressControls } from './progress_controls';
-import type { SearchServiceParams } from '../../../../common/search_strategies/correlations/types';
 import type { FailedTransactionsCorrelationValue } from '../../../../common/search_strategies/failure_correlations/types';
 import { Summary } from '../../shared/Summary';
 import { asPercent } from '../../../../common/utils/formatters';
@@ -70,16 +69,6 @@ export function FailedTransactionsCorrelations({
 
   const inspectEnabled = uiSettings.get<boolean>(enableInspectEsQueries);
 
-  const searchServiceParams: SearchServiceParams = {
-    environment,
-    kuery,
-    serviceName,
-    transactionName,
-    transactionType,
-    start,
-    end,
-  };
-
   const result = useFailedTransactionsCorrelationsFetcher();
 
   const {
@@ -93,12 +82,27 @@ export function FailedTransactionsCorrelations({
   } = result;
 
   const startFetchHandler = useCallback(() => {
-    startFetch(searchServiceParams);
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [environment, serviceName, transactionType, kuery, start, end]);
+    startFetch({
+      environment,
+      kuery,
+      serviceName,
+      transactionName,
+      transactionType,
+      start,
+      end,
+    });
+  }, [
+    startFetch,
+    environment,
+    serviceName,
+    transactionName,
+    transactionType,
+    kuery,
+    start,
+    end,
+  ]);
 
-  // start fetching on load
-  // we want this effect to execute exactly once after the component mounts
+  // start fetching on load and on changing environment
   useEffect(() => {
     if (isRunning) {
       cancelFetch();
@@ -108,11 +112,11 @@ export function FailedTransactionsCorrelations({
 
     return () => {
       // cancel any running async partial request when unmounting the component
-      // we want this effect to execute exactly once after the component mounts
       cancelFetch();
     };
+    // `isRunning` must not be part of the deps check
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [startFetchHandler]);
+  }, [cancelFetch, startFetchHandler]);
 
   const [
     selectedSignificantTerm,

diff --git a/x-pack/plugins/apm/public/components/app/correlations/latency_correlations.tsx b/x-pack/plugins/apm/public/components/app/correlations/latency_correlations.tsx
@@ -95,11 +95,18 @@ export function LatencyCorrelations({ onFilter }: { onFilter: () => void }) {
       end,
       percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD,
     });
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [environment, serviceName, transactionType, kuery, start, end]);
-
-  // start fetching on load
-  // we want this effect to execute exactly once after the component mounts
+  }, [
+    startFetch,
+    environment,
+    serviceName,
+    transactionName,
+    transactionType,
+    kuery,
+    start,
+    end,
+  ]);
+
+  // start fetching on load and on changing environment
   useEffect(() => {
     if (isRunning) {
       cancelFetch();
@@ -109,11 +116,11 @@ export function LatencyCorrelations({ onFilter }: { onFilter: () => void }) {
 
     return () => {
       // cancel any running async partial request when unmounting the component
-      // we want this effect to execute exactly once after the component mounts
       cancelFetch();
     };
+    // `isRunning` must not be part of the deps check
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [startFetchHandler]);
+  }, [cancelFetch, startFetchHandler]);
 
   useEffect(() => {
     if (isErrorMessage(error)) {

diff --git a/x-pack/plugins/apm/public/components/app/transaction_details/distribution/index.tsx b/x-pack/plugins/apm/public/components/app/transaction_details/distribution/index.tsx
@@ -114,11 +114,7 @@ export function TransactionDistribution({
     transactionDistribution,
   } = useTransactionDistributionFetcher();
 
-  useEffect(() => {
-    if (isRunning) {
-      cancelFetch();
-    }
-
+  const startFetchHandler = useCallback(() => {
     startFetch({
       environment,
       kuery,
@@ -129,13 +125,32 @@ export function TransactionDistribution({
       end,
       percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD,
     });
+  }, [
+    startFetch,
+    environment,
+    serviceName,
+    transactionName,
+    transactionType,
+    kuery,
+    start,
+    end,
+  ]);
+
+  // start fetching on load and on changing environment
+  useEffect(() => {
+    if (isRunning) {
+      cancelFetch();
+    }
+
+    startFetchHandler();
 
     return () => {
       // cancel any running async partial request when unmounting the component
       cancelFetch();
     };
+    // `isRunning` must not be part of the deps check
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [environment, serviceName, transactionType, kuery, start, end]);
+  }, [cancelFetch, startFetchHandler]);
 
   useEffect(() => {
     if (isErrorMessage(error)) {

diff --git a/x-pack/plugins/apm/public/hooks/use_failed_transactions_correlations_fetcher.ts b/x-pack/plugins/apm/public/hooks/use_failed_transactions_correlations_fetcher.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { useRef, useState } from 'react';
+import { useCallback, useRef, useState } from 'react';
 import type { Subscription } from 'rxjs';
 import {
   IKibanaSearchRequest,
@@ -72,62 +72,65 @@ export const useFailedTransactionsCorrelationsFetcher = () => {
     }));
   }
 
-  const startFetch = (params: SearchServiceParams) => {
-    setFetchState((prevState) => ({
-      ...prevState,
-      error: undefined,
-      isComplete: false,
-    }));
-    searchSubscription$.current?.unsubscribe();
-    abortCtrl.current.abort();
-    abortCtrl.current = new AbortController();
+  const startFetch = useCallback(
+    (params: SearchServiceParams) => {
+      setFetchState((prevState) => ({
+        ...prevState,
+        error: undefined,
+        isComplete: false,
+      }));
+      searchSubscription$.current?.unsubscribe();
+      abortCtrl.current.abort();
+      abortCtrl.current = new AbortController();
 
-    const req = { params };
+      const req = { params };
 
-    // Submit the search request using the `data.search` service.
-    searchSubscription$.current = data.search
-      .search<IKibanaSearchRequest, IKibanaSearchResponse<RawResponse>>(req, {
-        strategy: FAILED_TRANSACTIONS_CORRELATION_SEARCH_STRATEGY,
-        abortSignal: abortCtrl.current.signal,
-      })
-      .subscribe({
-        next: (res: IKibanaSearchResponse<RawResponse>) => {
-          setResponse(res);
-          if (isCompleteResponse(res)) {
-            searchSubscription$.current?.unsubscribe();
+      // Submit the search request using the `data.search` service.
+      searchSubscription$.current = data.search
+        .search<IKibanaSearchRequest, IKibanaSearchResponse<RawResponse>>(req, {
+          strategy: FAILED_TRANSACTIONS_CORRELATION_SEARCH_STRATEGY,
+          abortSignal: abortCtrl.current.signal,
+        })
+        .subscribe({
+          next: (res: IKibanaSearchResponse<RawResponse>) => {
+            setResponse(res);
+            if (isCompleteResponse(res)) {
+              searchSubscription$.current?.unsubscribe();
+              setFetchState((prevState) => ({
+                ...prevState,
+                isRunnning: false,
+                isComplete: true,
+              }));
+            } else if (isErrorResponse(res)) {
+              searchSubscription$.current?.unsubscribe();
+              setFetchState((prevState) => ({
+                ...prevState,
+                error: (res as unknown) as Error,
+                setIsRunning: false,
+              }));
+            }
+          },
+          error: (error: Error) => {
             setFetchState((prevState) => ({
               ...prevState,
-              isRunnning: false,
-              isComplete: true,
-            }));
-          } else if (isErrorResponse(res)) {
-            searchSubscription$.current?.unsubscribe();
-            setFetchState((prevState) => ({
-              ...prevState,
-              error: (res as unknown) as Error,
+              error,
               setIsRunning: false,
             }));
-          }
-        },
-        error: (error: Error) => {
-          setFetchState((prevState) => ({
-            ...prevState,
-            error,
-            setIsRunning: false,
-          }));
-        },
-      });
-  };
+          },
+        });
+    },
+    [data.search]
+  );
 
-  const cancelFetch = () => {
+  const cancelFetch = useCallback(() => {
     searchSubscription$.current?.unsubscribe();
     searchSubscription$.current = undefined;
     abortCtrl.current.abort();
     setFetchState((prevState) => ({
       ...prevState,
       setIsRunning: false,
     }));
-  };
+  }, []);
 
   return {
     ...fetchState,