Merge pull request #326 from chromaui/avoid-excessive-polling

Replace API polling with browser-native "online" status check

src/Panel.tsx

@@ -1,14 +1,14 @@
 import type { API } from "@storybook/manager-api";
 import { useChannel, useStorybookState } from "@storybook/manager-api";
-import React, { useCallback } from "react";
+import React, { useCallback, useEffect, useState } from "react";
 
 import { AuthProvider } from "./AuthContext";
 import { Spinner } from "./components/design-system";
 import {
   ADDON_ID,
-  API_INFO,
   GIT_INFO,
   GIT_INFO_ERROR,
+  IS_OFFLINE,
   IS_OUTDATED,
   LOCAL_BUILD_PROGRESS,
   PANEL_ID,
@@ -27,7 +27,7 @@ import { Uninstalled } from "./screens/Uninstalled/Uninstalled";
 import { ControlsProvider } from "./screens/VisualTests/ControlsContext";
 import { RunBuildProvider } from "./screens/VisualTests/RunBuildContext";
 import { VisualTests } from "./screens/VisualTests/VisualTests";
-import { APIInfoPayload, GitInfoPayload, LocalBuildProgress, UpdateStatusFunction } from "./types";
+import { GitInfoPayload, LocalBuildProgress, UpdateStatusFunction } from "./types";
 import { client, Provider, useAccessToken } from "./utils/graphQLClient";
 import { TelemetryProvider } from "./utils/TelemetryContext";
 import { useBuildEvents } from "./utils/useBuildEvents";
@@ -51,9 +51,21 @@ export const Panel = ({ active, api }: PanelProps) => {
   );
   const { storyId } = useStorybookState();
 
-  const [apiInfo] = useSharedState<APIInfoPayload>(API_INFO);
+  const [isOnline, setOnline] = useState<boolean>(window.navigator.onLine);
+  useEffect(() => {
+    const online = () => setOnline(true);
+    const offline = () => setOnline(false);
+    window.addEventListener("online", online);
+    window.addEventListener("offline", offline);
+    return () => {
+      window.removeEventListener("online", online);
+      window.removeEventListener("offline", offline);
+    };
+  }, []);
+
   const [gitInfo] = useSharedState<GitInfoPayload>(GIT_INFO);
   const [gitInfoError] = useSharedState<Error>(GIT_INFO_ERROR);
+  const [isOffline] = useSharedState<boolean>(IS_OFFLINE);
   const [isOutdated] = useSharedState<boolean>(IS_OUTDATED);
   const [localBuildProgress, setLocalBuildProgress] =
     useSharedState<LocalBuildProgress>(LOCAL_BUILD_PROGRESS);
@@ -114,8 +126,8 @@ export const Panel = ({ active, api }: PanelProps) => {
     return withProviders(<Uninstalled />);
   }
 
-  if (apiInfo?.connected === false) {
-    return withProviders(<NoNetwork aborted={apiInfo.aborted} />);
+  if (isOffline) {
+    return withProviders(<NoNetwork offline={isOffline} />);
   }
 
   // Render the Authentication flow if the user is not signed in.

src/components/SidebarTop.tsx

@@ -6,14 +6,14 @@ import React, { useCallback, useContext, useEffect, useRef } from "react";
 
 import {
   ADDON_ID,
-  API_INFO,
   CONFIG_INFO,
   GIT_INFO_ERROR,
+  IS_OFFLINE,
   IS_OUTDATED,
   LOCAL_BUILD_PROGRESS,
   PANEL_ID,
 } from "../constants";
-import { APIInfoPayload, ConfigInfoPayload, LocalBuildProgress } from "../types";
+import { ConfigInfoPayload, LocalBuildProgress } from "../types";
 import { useAccessToken } from "../utils/graphQLClient";
 import { TelemetryContext } from "../utils/TelemetryContext";
 import { useBuildEvents } from "../utils/useBuildEvents";
@@ -33,10 +33,10 @@ export const SidebarTop = ({ api }: SidebarTopProps) => {
   const [accessToken] = useAccessToken();
   const isLoggedIn = !!accessToken;
 
+  const [isOffline, setOffline] = useSharedState<boolean>(IS_OFFLINE);
   const [isOutdated] = useSharedState<boolean>(IS_OUTDATED);
   const [localBuildProgress] = useSharedState<LocalBuildProgress>(LOCAL_BUILD_PROGRESS);
 
-  const [apiInfo] = useSharedState<APIInfoPayload>(API_INFO);
   const [configInfo] = useSharedState<ConfigInfoPayload>(CONFIG_INFO);
   const hasConfigProblem = Object.keys(configInfo?.problems || {}).length > 0;
 
@@ -74,6 +74,17 @@ export const SidebarTop = ({ api }: SidebarTopProps) => {
     [openVisualTestsPanel]
   );
 
+  useEffect(() => {
+    const offline = () => setOffline(true);
+    const online = () => setOffline(false);
+    window.addEventListener("offline", offline);
+    window.addEventListener("online", online);
+    return () => {
+      window.removeEventListener("offline", offline);
+      window.removeEventListener("online", online);
+    };
+  }, [setOffline]);
+
   useEffect(() => {
     if (localBuildProgress?.currentStep === lastStep.current) return;
     lastStep.current = localBuildProgress?.currentStep;
@@ -179,11 +190,11 @@ export const SidebarTop = ({ api }: SidebarTopProps) => {
   });
 
   let warning: string | undefined;
-  if (apiInfo?.connected === false) warning = "Visual tests locked while waiting for network.";
   if (!projectId) warning = "Visual tests locked until a project is selected.";
   if (!isLoggedIn) warning = "Visual tests locked until you are logged in.";
   if (gitInfoError) warning = "Visual tests locked due to Git synchronization problem.";
   if (hasConfigProblem) warning = "Visual tests locked due to configuration problem.";
+  if (isOffline) warning = "Visual tests locked while offline.";
 
   const clickWarning = useCallback(
     () => openVisualTestsPanel(warning),

src/constants.ts

@@ -12,12 +12,12 @@ export const SIDEBAR_TOP_ID = `${ADDON_ID}/sidebarTop`;
 export const SIDEBAR_BOTTOM_ID = `${ADDON_ID}/sidebarBottom`;
 export const ACCESS_TOKEN_KEY = `${ADDON_ID}/access-token/${CHROMATIC_BASE_URL}`;
 export const DEV_BUILD_ID_KEY = `${ADDON_ID}/dev-build-id`;
-export const API_INFO = `${ADDON_ID}/apiInfo`;
 export const CONFIG_INFO = `${ADDON_ID}/configInfo`;
 export const CONFIG_INFO_DISMISSED = `${ADDON_ID}/configInfoDismissed`;
 export const GIT_INFO = `${ADDON_ID}/gitInfo`;
 export const GIT_INFO_ERROR = `${ADDON_ID}/gitInfoError`;
 export const PROJECT_INFO = `${ADDON_ID}/projectInfo`;
+export const IS_OFFLINE = `${ADDON_ID}/isOffline`;
 export const IS_OUTDATED = `${ADDON_ID}/isOutdated`;
 export const START_BUILD = `${ADDON_ID}/startBuild`;
 export const STOP_BUILD = `${ADDON_ID}/stopBuild`;
@@ -27,7 +27,6 @@ export const SELECTED_BROWSER_ID = `${ADDON_ID}/selectedBrowserId`;
 export const TELEMETRY = `${ADDON_ID}/telemetry`;
 export const ENABLE_FILTER = `${ADDON_ID}/enableFilter`;
 export const REMOVE_ADDON = `${ADDON_ID}/removeAddon`;
-export const RETRY_CONNECTION = `${ADDON_ID}/retryConnection`;
 
 export const CONFIG_OVERRIDES = {
   // Local changes should never be auto-accepted

src/preset.ts

@@ -11,8 +11,6 @@ import { type Configuration, getConfiguration, getGitInfo, type GitInfo } from "
 
 import {
   ADDON_ID,
-  API_INFO,
-  CHROMATIC_API_URL,
   CHROMATIC_BASE_URL,
   CONFIG_INFO,
   GIT_INFO,
@@ -21,14 +19,12 @@ import {
   PACKAGE_NAME,
   PROJECT_INFO,
   REMOVE_ADDON,
-  RETRY_CONNECTION,
   START_BUILD,
   STOP_BUILD,
   TELEMETRY,
 } from "./constants";
 import { runChromaticBuild, stopChromaticBuild } from "./runChromaticBuild";
 import {
-  APIInfoPayload,
   ConfigInfoPayload,
   ConfigurationUpdate,
   GitInfoPayload,
@@ -112,35 +108,6 @@ const getConfigInfo = async (
   };
 };
 
-// Polls for a connection to the Chromatic API.
-// Uses a recursive setTimeout instead of setInterval to avoid overlapping async calls.
-// Two consecutive failures are needed before considering the connection as lost.
-// Retries with an increasing delay after the first failure and aborts after 10 attempts.
-const observeAPIInfo = (interval: number, callback: (apiInfo: APIInfoPayload) => void) => {
-  let timer: NodeJS.Timeout | undefined;
-  const act = async (attempt = 1) => {
-    if (attempt > 10) {
-      callback({ aborted: true, connected: false });
-      return;
-    }
-    const ok = await fetch(CHROMATIC_API_URL, {
-      method: "POST",
-      headers: { "Content-Type": "application/json" },
-      body: JSON.stringify({ query: `{ viewer { id } }` }),
-    }).then(
-      (res) => res.ok,
-      () => false
-    );
-    if (ok || attempt > 1) {
-      callback({ aborted: false, connected: ok });
-    }
-    timer = ok ? setTimeout(act, interval) : setTimeout(act, attempt * 1000, attempt + 1);
-  };
-  act();
-
-  return { cancel: () => clearTimeout(timer) };
-};
-
 // Polls for changes to the Git state and invokes the callback when it changes.
 // Uses a recursive setTimeout instead of setInterval to avoid overlapping async calls.
 const observeGitInfo = (
@@ -260,15 +227,10 @@ async function serverChannel(channel: Channel, options: Options & { configFile?:
     telemetry("addon-visual-tests" as any, { ...event, addonVersion: await getAddonVersion() });
   });
 
-  const apiInfoState = SharedState.subscribe<APIInfoPayload>(API_INFO, channel);
   const configInfoState = SharedState.subscribe<ConfigInfoPayload>(CONFIG_INFO, channel);
   const gitInfoState = SharedState.subscribe<GitInfoPayload>(GIT_INFO, channel);
   const gitInfoError = SharedState.subscribe<Error>(GIT_INFO_ERROR, channel);
 
-  let apiInfoObserver = observeAPIInfo(5000, (info: APIInfoPayload) => {
-    apiInfoState.value = info;
-  });
-
   const gitInfoObserver = observeGitInfo(
     5000,
     (info) => {
@@ -289,17 +251,9 @@ async function serverChannel(channel: Channel, options: Options & { configFile?:
 
   channel.on(REMOVE_ADDON, () => {
     apiPromise.then((api) => api.removeAddon(PACKAGE_NAME)).catch((e) => console.error(e));
-    apiInfoObserver.cancel();
     gitInfoObserver.cancel();
   });
 
-  channel.on(RETRY_CONNECTION, () => {
-    apiInfoObserver.cancel();
-    apiInfoObserver = observeAPIInfo(5000, (info: APIInfoPayload) => {
-      apiInfoState.value = info;
-    });
-  });
-
   return channel;
 }
 

src/screens/NoNetwork/NoNetwork.stories.tsx

@@ -4,17 +4,14 @@ import { NoNetwork } from "./NoNetwork";
 
 const meta = {
   component: NoNetwork,
-  args: {
-    aborted: false,
-  },
 } satisfies Meta<typeof NoNetwork>;
 
 export default meta;
 
 export const Default = {} satisfies StoryObj<typeof meta>;
 
-export const Aborted = {
+export const Offline = {
   args: {
-    aborted: true,
+    offline: true,
   },
 } satisfies StoryObj<typeof meta>;

src/screens/NoNetwork/NoNetwork.tsx

@@ -1,59 +1,31 @@
-import { SyncIcon } from "@storybook/icons";
-import { useChannel } from "@storybook/manager-api";
-import { styled } from "@storybook/theming";
-import React, { useEffect, useState } from "react";
+import React from "react";
 
-import { Button } from "../../components/Button";
 import { Container } from "../../components/Container";
 import { Link } from "../../components/design-system";
-import { rotate360 } from "../../components/design-system/shared/animation";
 import { Heading } from "../../components/Heading";
 import { Screen } from "../../components/Screen";
 import { Stack } from "../../components/Stack";
 import { Text } from "../../components/Text";
-import { RETRY_CONNECTION } from "../../constants";
-
-const SpinIcon = styled(SyncIcon)({
-  animation: `${rotate360} 1s linear infinite`,
-});
-
-export const NoNetwork = ({ aborted }: { aborted: boolean }) => {
-  const [retried, setRetried] = useState(false);
-  const emit = useChannel({});
-
-  const retry = () => {
-    setRetried(true);
-    emit(RETRY_CONNECTION);
-  };
-
-  useEffect(() => {
-    setRetried(false);
-  }, [aborted]);
 
+export const NoNetwork = ({ offline = false }: { offline?: boolean }) => {
   return (
     <Screen footer={null}>
       <Container>
         <Stack>
           <div>
             <Heading>Can't connect to Chromatic</Heading>
             <Text center muted>
-              Double check your internet connection and firewall settings.
+              {offline
+                ? "You're offline. Double check your internet connection."
+                : "We're having trouble connecting to the Chromatic API."}
             </Text>
           </div>
-          {aborted ? (
-            <Button size="medium" variant="solid" onClick={retry} disabled={retried}>
-              <SyncIcon />
-              Retry
-            </Button>
-          ) : (
-            <Button size="medium" variant="ghost" onClick={retry} disabled={retried}>
-              <SpinIcon />
-              Connecting...
-            </Button>
+
+          {!offline && (
+            <Link href="https://status.chromatic.com" target="_blank" rel="noreferrer" withArrow>
+              Chromatic API status
+            </Link>
           )}
-          <Link href="https://status.chromatic.com" target="_blank" rel="noreferrer" withArrow>
-            Chromatic API status
-          </Link>
         </Stack>
       </Container>
     </Screen>

src/types.ts

@@ -26,10 +26,6 @@ export type ConfigurationUpdate = {
   [Property in keyof Configuration]: Configuration[Property] | null;
 };
 
-export type APIInfoPayload = {
-  aborted: boolean;
-  connected: boolean;
-};
 export type ConfigInfoPayload = {
   configuration: Awaited<ReturnType<typeof getConfiguration>>;
   problems?: ConfigurationUpdate;