refactor: extract shared useAsync hook

plugins/aks-desktop/src/components/Deployments/hooks/usePipelineRuns.ts

@@ -3,7 +3,8 @@
 
 import { useTranslation } from '@kinvolk/headlamp-plugin/lib';
 import type { Octokit } from '@octokit/rest';
-import { useCallback, useEffect, useMemo, useState } from 'react';
+import { useCallback, useEffect, useMemo } from 'react';
+import { useAsync } from '../../../hooks/useAsync';
 import type { GitHubRepo, WorkflowRunConclusion, WorkflowRunStatus } from '../../../types/github';
 import { listWorkflowRuns } from '../../../utils/github/github-api';
 import { PIPELINE_WORKFLOW_FILENAME } from '../../GitHubPipeline/constants';
@@ -32,57 +33,54 @@ export const usePipelineRuns = (
   repos: GitHubRepo[]
 ): UsePipelineRunsResult => {
   const { t } = useTranslation();
-  const [runs, setRuns] = useState<PipelineRun[]>([]);
-  const [loading, setLoading] = useState(false);
-  const [error, setError] = useState<string | null>(null);
 
   // Stable repos reference — only changes when the repo list actually changes
   const repoKey = JSON.stringify(repos.map(r => [r.owner, r.repo]));
   // eslint-disable-next-line react-hooks/exhaustive-deps
   const stableRepos = useMemo(() => repos, [repoKey]);
 
-  const fetchRuns = useCallback(
-    async (signal: { cancelled: boolean }) => {
-      if (!octokit || stableRepos.length === 0) return;
+  const fetchRuns = useCallback(async (): Promise<PipelineRun[]> => {
+    if (!octokit) return [];
 
-      setLoading(true);
-      setError(null);
-      const results = await Promise.allSettled(
-        stableRepos.map(repo =>
-          listWorkflowRuns(octokit, repo.owner, repo.repo, {
-            workflowFileName: PIPELINE_WORKFLOW_FILENAME,
-            per_page: 5,
-          })
-        )
-      );
-      if (signal.cancelled) return;
-      const allRuns: PipelineRun[] = [];
-      for (const result of results) {
-        if (result.status === 'fulfilled') {
-          allRuns.push(...result.value);
-        }
-      }
-      const failures = results.filter(r => r.status === 'rejected');
-      if (failures.length === results.length && results.length > 0) {
-        setError(t('Failed to load pipeline runs'));
-      } else if (failures.length > 0) {
-        console.warn(`Pipeline runs: ${failures.length}/${results.length} repos failed to load`);
-        setError(null); // Partial success — show what we have
+    const results = await Promise.allSettled(
+      stableRepos.map(repo =>
+        listWorkflowRuns(octokit, repo.owner, repo.repo, {
+          workflowFileName: PIPELINE_WORKFLOW_FILENAME,
+          per_page: 5,
+        })
+      )
+    );
+
+    const allRuns: PipelineRun[] = [];
+    for (const result of results) {
+      if (result.status === 'fulfilled') {
+        allRuns.push(...result.value);
       }
-      allRuns.sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime());
-      setRuns(allRuns.slice(0, 10));
-      setLoading(false);
-    },
-    [octokit, stableRepos]
-  );
+    }
+
+    const failures = results.filter((r): r is PromiseRejectedResult => r.status === 'rejected');
+    if (failures.length === results.length && results.length > 0) {
+      throw new Error(t('Failed to load pipeline runs'));
+    } else if (failures.length > 0) {
+      console.warn(
+        `Pipeline runs: ${failures.length}/${results.length} repos failed to load`,
+        failures.map(f => f.reason)
+      );
+    }
+
+    allRuns.sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime());
+    return allRuns.slice(0, 10);
+  }, [octokit, stableRepos, t]);
+
+  const { data, loading, error, execute, reset } = useAsync(fetchRuns, { immediate: false });
 
   useEffect(() => {
-    const signal = { cancelled: false };
-    fetchRuns(signal);
-    return () => {
-      signal.cancelled = true;
-    };
-  }, [fetchRuns]);
+    if (!octokit || stableRepos.length === 0) {
+      reset();
+      return;
+    }
+    execute();
+  }, [execute, reset, octokit, stableRepos, fetchRuns]);
 
-  return { runs, loading, error };
+  return { runs: data ?? [], loading, error };
 };

plugins/aks-desktop/src/components/Scaling/hooks/useChartData.test.ts

@@ -271,7 +271,7 @@ describe('useChartData', () => {
     });
 
     expect(result.current.chartData).toHaveLength(0);
-    expect(result.current.error).toBe('Failed to fetch scaling data');
+    expect(result.current.error).toBe('string error');
   });
 
   test('handles null result from getClusterResourceIdAndGroup', async () => {

plugins/aks-desktop/src/components/Scaling/hooks/useChartData.ts

@@ -1,7 +1,8 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the Apache 2.0.
 
-import { useCallback, useEffect, useRef, useState } from 'react';
+import { useCallback, useEffect } from 'react';
+import { useAsync } from '../../../hooks/useAsync';
 import { getClusterResourceIdAndGroup } from '../../../utils/azure/az-clusters';
 import { getPrometheusEndpoint } from '../../MetricsTab/getPrometheusEndpoint';
 import { queryPrometheus } from '../../MetricsTab/queryPrometheus';
@@ -73,122 +74,82 @@ export const useChartData = (
   timeRangeSecs: number,
   step: number
 ): UseChartDataResult => {
-  const [chartData, setChartData] = useState<ChartDataPoint[]>([]);
-  const [loading, setLoading] = useState<boolean>(false);
-  const [error, setError] = useState<string | null>(null);
-  const latestRequestIdRef = useRef(0);
-
-  const fetchChartData = useCallback(async () => {
-    const requestId = ++latestRequestIdRef.current;
-    const isLatestRequest = () => latestRequestIdRef.current === requestId;
-    const applyIfLatest = (callback: () => void) => {
-      if (isLatestRequest()) {
-        callback();
-      }
-    };
+  const hasRequiredParams = !!(namespace && selectedDeployment && cluster && subscription);
 
-    if (!namespace || !selectedDeployment || !cluster || !subscription) {
-      applyIfLatest(() => {
-        setChartData([]);
-        setError(null);
-        setLoading(false);
-      });
-      return;
-    }
-
-    try {
-      applyIfLatest(() => {
-        setLoading(true);
-        setError(null);
-      });
+  const asyncFn = useCallback(async (): Promise<ChartDataPoint[]> => {
+    // Extract resource group from label if available, otherwise fetch
+    let resourceGroup = resourceGroupLabel;
 
-      // Extract resource group from label if available, otherwise fetch
-      let resourceGroup = resourceGroupLabel;
+    if (!resourceGroup) {
+      const result = await getClusterResourceIdAndGroup(cluster, subscription);
+      resourceGroup = result?.resourceGroup;
 
       if (!resourceGroup) {
-        const result = await getClusterResourceIdAndGroup(cluster, subscription);
-        resourceGroup = result?.resourceGroup;
-
-        if (!resourceGroup) {
-          throw new Error('Could not find resource group for cluster');
-        }
-      }
-
-      // Return cached data if the same parameters were queried recently.
-      // Include resolved resource group and time range to avoid cache collisions.
-      const cacheKey = `${selectedDeployment}:${namespace}:${cluster}:${subscription}:${resourceGroup}:${timeRangeSecs}:${step}`;
-      const cachedChartData = getCachedChartData(cacheKey);
-      if (cachedChartData) {
-        applyIfLatest(() => {
-          setChartData(cachedChartData);
-          setError(null);
-        });
-        return;
+        throw new Error('Could not find resource group for cluster');
       }
+    }
 
-      const endpointKey = `${resourceGroup}:${cluster}:${subscription}`;
-      let promEndpoint = promEndpointCache.get(endpointKey);
-      if (!promEndpoint) {
-        promEndpoint = await getPrometheusEndpoint(resourceGroup, cluster, subscription);
-        promEndpointCache.set(endpointKey, promEndpoint);
-      }
+    // Return cached data if the same parameters were queried recently.
+    // Include resolved resource group and time range to avoid cache collisions.
+    const cacheKey = `${selectedDeployment}:${namespace}:${cluster}:${subscription}:${resourceGroup}:${timeRangeSecs}:${step}`;
+    const cachedChartData = getCachedChartData(cacheKey);
+    if (cachedChartData) {
+      return cachedChartData;
+    }
 
-      const end = Math.floor(Date.now() / 1000);
-      const start = end - timeRangeSecs;
-
-      // Query replica count and CPU usage in parallel
-      const replicaQuery = `kube_deployment_spec_replicas{deployment="${selectedDeployment}",namespace="${namespace}"}`;
-      const cpuQuery = `100 * (sum by (namespace) (rate(container_cpu_usage_seconds_total{namespace="${namespace}", pod=~"${selectedDeployment}-.*", container!=""}[5m])) / sum by (namespace) (kube_pod_container_resource_limits{namespace="${namespace}", pod=~"${selectedDeployment}-.*", resource="cpu"}))`;
-
-      const [replicaResults, cpuResults] = await Promise.all([
-        queryPrometheus(promEndpoint, replicaQuery, start, end, step, subscription),
-        queryPrometheus(promEndpoint, cpuQuery, start, end, step, subscription),
-      ]);
-
-      // Merge replica and CPU data by timestamp
-      const mergedData: ChartDataPoint[] = [];
-      const replicaValues = replicaResults[0]?.values || [];
-      const cpuValues = cpuResults[0]?.values || [];
-      const dayFormatter = new Intl.DateTimeFormat([], { weekday: 'short' });
-      const timeFormatter = new Intl.DateTimeFormat([], { hour: '2-digit', minute: '2-digit' });
-
-      // Create a map of timestamps to CPU values for easier lookup
-      const cpuMap = new Map<number, number>();
-      cpuValues.forEach(([timestamp, value]: [number, string]) => {
-        const parsedCpu = parseFloat(value);
-        cpuMap.set(timestamp, Number.isFinite(parsedCpu) ? parsedCpu : 0);
-      });
+    const endpointKey = `${resourceGroup}:${cluster}:${subscription}`;
+    let promEndpoint = promEndpointCache.get(endpointKey);
+    if (!promEndpoint) {
+      promEndpoint = await getPrometheusEndpoint(resourceGroup, cluster, subscription);
+      promEndpointCache.set(endpointKey, promEndpoint);
+    }
 
-      // Iterate through replica values and match with CPU
-      replicaValues.forEach(([timestamp, replicaValue]: [number, string]) => {
-        const date = new Date(timestamp * 1000);
-        const day = dayFormatter.format(date);
-        const time = timeFormatter.format(date);
-        const timeString = `${day}, ${time}`;
-
-        const parsedReplicas = parseInt(replicaValue, 10);
-        const replicas = Number.isFinite(parsedReplicas) ? parsedReplicas : 0;
-        const cpu = cpuMap.get(timestamp) || 0;
-
-        mergedData.push({
-          time: timeString,
-          Replicas: replicas,
-          CPU: Math.round(cpu),
-        });
+    const end = Math.floor(Date.now() / 1000);
+    const start = end - timeRangeSecs;
+
+    // Query replica count and CPU usage in parallel
+    const replicaQuery = `kube_deployment_spec_replicas{deployment="${selectedDeployment}",namespace="${namespace}"}`;
+    const cpuQuery = `100 * (sum by (namespace) (rate(container_cpu_usage_seconds_total{namespace="${namespace}", pod=~"${selectedDeployment}-.*", container!=""}[5m])) / sum by (namespace) (kube_pod_container_resource_limits{namespace="${namespace}", pod=~"${selectedDeployment}-.*", resource="cpu"}))`;
+
+    const [replicaResults, cpuResults] = await Promise.all([
+      queryPrometheus(promEndpoint, replicaQuery, start, end, step, subscription),
+      queryPrometheus(promEndpoint, cpuQuery, start, end, step, subscription),
+    ]);
+
+    // Merge replica and CPU data by timestamp
+    const mergedData: ChartDataPoint[] = [];
+    const replicaValues = replicaResults[0]?.values || [];
+    const cpuValues = cpuResults[0]?.values || [];
+    const dayFormatter = new Intl.DateTimeFormat([], { weekday: 'short' });
+    const timeFormatter = new Intl.DateTimeFormat([], { hour: '2-digit', minute: '2-digit' });
+
+    // Create a map of timestamps to CPU values for easier lookup
+    const cpuMap = new Map<number, number>();
+    cpuValues.forEach(([timestamp, value]: [number, string]) => {
+      const parsedCpu = parseFloat(value);
+      cpuMap.set(timestamp, Number.isFinite(parsedCpu) ? parsedCpu : 0);
+    });
+
+    // Iterate through replica values and match with CPU
+    replicaValues.forEach(([timestamp, replicaValue]: [number, string]) => {
+      const date = new Date(timestamp * 1000);
+      const day = dayFormatter.format(date);
+      const time = timeFormatter.format(date);
+      const timeString = `${day}, ${time}`;
+
+      const parsedReplicas = parseInt(replicaValue, 10);
+      const replicas = Number.isFinite(parsedReplicas) ? parsedReplicas : 0;
+      const cpu = cpuMap.get(timestamp) || 0;
+
+      mergedData.push({
+        time: timeString,
+        Replicas: replicas,
+        CPU: Math.round(cpu),
       });
+    });
 
-      chartDataCache.set(cacheKey, { data: mergedData, timestamp: Date.now() });
-      applyIfLatest(() => setChartData(mergedData));
-    } catch (err) {
-      console.error('Failed to fetch chart data from Prometheus:', err);
-      applyIfLatest(() => {
-        const nextError = err instanceof Error ? err.message : 'Failed to fetch scaling data';
-        setError(nextError);
-        setChartData([]);
-      });
-    } finally {
-      applyIfLatest(() => setLoading(false));
-    }
+    chartDataCache.set(cacheKey, { data: mergedData, timestamp: Date.now() });
+    return mergedData;
   }, [
     namespace,
     selectedDeployment,
@@ -199,16 +160,15 @@ export const useChartData = (
     step,
   ]);
 
-  useEffect(() => {
-    fetchChartData();
-  }, [fetchChartData]);
+  const { data, loading, error, execute, reset } = useAsync(asyncFn, { immediate: false });
 
   useEffect(() => {
-    return () => {
-      // Invalidate any in-flight async work for this hook instance.
-      latestRequestIdRef.current += 1;
-    };
-  }, []);
+    if (hasRequiredParams) {
+      execute();
+    } else {
+      reset();
+    }
+  }, [execute, reset, hasRequiredParams, asyncFn]);
 
-  return { chartData, loading, error };
+  return { chartData: data ?? [], loading, error };
 };