refactor: move state

Co-authored-by: Patrik Oldsberg <poldsberg@gmail.com>
Co-authored-by: Nikita Dudnik <nikdudnik@gmail.com>

plugins/circleci/src/components/App.tsx

@@ -3,10 +3,10 @@ import { Switch, Route } from 'react-router';
 import { BuildsPage } from '../pages/BuildsPage';
 import { SettingsPage } from '../pages/SettingsPage';
 import { DetailedViewPage } from '../pages/DetailedViewPage';
-import { Store } from './Store';
+import { AppStateProvider } from '../state';
 
 export const App = () => (
-  <Store>
+  <AppStateProvider>
     <Switch>
       <Route path="/circleci" exact component={BuildsPage} />
       <Route path="/circleci/settings" exact component={SettingsPage} />
@@ -16,5 +16,5 @@ export const App = () => (
         component={DetailedViewPage}
       />
     </Switch>
-  </Store>
+  </AppStateProvider>
 );

plugins/circleci/src/components/Store/types.ts

@@ -1,62 +0,0 @@
-import { BuildSummary, BuildWithSteps } from '../../api';
-
-export type SettingsState = {
-  owner: string;
-  repo: string;
-  token: string;
-};
-
-export enum PollingState {
-  Polling,
-  Idle,
-}
-
-export type BuildsState = {
-  builds: BuildSummary[];
-  pollingIntervalId: number | null;
-  pollingState: PollingState;
-};
-
-export type State = {
-  settings: SettingsState;
-  builds: BuildsState;
-  buildsWithSteps: BuildsWithStepsState;
-};
-
-type SettingsAction = {
-  type: 'setCredentials';
-  payload: {
-    repo: string;
-    owner: string;
-    token: string;
-  };
-};
-
-type BuildsAction =
-  | {
-      type: 'setBuilds';
-      payload: BuildSummary[];
-    }
-  | {
-      type: 'setPollingIntervalId';
-      payload: number | null;
-    };
-
-type BuildsWithStepsAction =
-  | {
-      type: 'setBuildWithSteps';
-      payload: BuildWithSteps;
-    }
-  | {
-      type: 'setPollingIntervalIdForBuildsWithSteps';
-      payload: number | null;
-    };
-
-export type BuildsWithStepsState = {
-  builds: Record<number, BuildWithSteps>;
-  pollingIntervalId: number | null;
-  pollingState: PollingState;
-  getBuildError: Error | null;
-};
-
-export type Action = SettingsAction | BuildsAction | BuildsWithStepsAction;

plugins/circleci/src/pages/BuildsPage/lib/Builds/Builds.tsx

@@ -13,11 +13,10 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import React, { FC } from 'react';
+import React, { FC, useEffect } from 'react';
 import { CITableBuildInfo, CITable } from '../CITable';
 import { BuildSummary } from '../../../../api';
-import { useBuilds } from '../../builds';
-import { useSettings } from '../../../SettingsPage/settings';
+import { useSettings, useBuilds } from '../../../../state';
 
 const makeReadableStatus = (status: string | undefined) => {
   if (!status) return '';
@@ -49,7 +48,7 @@ const transform = (
         ? buildData.subject +
           (buildData.retry_of ? ` (retry of #${buildData.retry_of})` : '')
         : '',
-      onRetryClick: () =>
+      onRestartClick: () =>
         typeof buildData.build_num !== 'undefined' &&
         restartBuild(buildData.build_num),
       source: {
@@ -67,9 +66,18 @@ const transform = (
 };
 
 export const Builds: FC<{}> = () => {
-  const [{ builds }, { restartBuild }] = useBuilds();
+  const [
+    builds,
+    { restartBuild: handleRestartBuild, startPolling, stopPolling },
+  ] = useBuilds();
   const [{ repo, owner }] = useSettings();
-  const transformedBuilds = transform(builds, restartBuild);
+
+  useEffect(() => {
+    startPolling();
+    return () => stopPolling();
+  }, [repo, owner]);
+
+  const transformedBuilds = transform(builds, handleRestartBuild);
 
   return (
     <CITable builds={transformedBuilds} projectName={`${owner}/${repo}`} />

plugins/circleci/src/pages/BuildsPage/lib/CITable/CITable.tsx

@@ -46,7 +46,7 @@ export type CITableBuildInfo = {
     failed: number;
     testUrl: string; // fixme better name
   };
-  onRetryClick: () => void;
+  onRestartClick: () => void;
 };
 
 // retried, canceled, infrastructure_fail, timedout, not_run, running, failed, queued, scheduled, not_running, no_tests, fixed, success
@@ -106,7 +106,7 @@ const generatedColumns: TableColumn[] = [
   {
     title: 'Actions',
     render: (row: Partial<CITableBuildInfo>) => (
-      <IconButton onClick={row.onRetryClick}>
+      <IconButton onClick={row.onRestartClick}>
         <RetryIcon />
       </IconButton>
     ),

plugins/circleci/src/pages/DetailedViewPage/DetailedViewPage.tsx

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import React, { FC } from 'react';
+import React, { FC, useEffect } from 'react';
 import { useParams } from 'react-router-dom';
 import { Content, InfoCard } from '@backstage/core';
 import { BuildWithSteps, BuildStepAction } from '../../api';
@@ -22,7 +22,7 @@ import { makeStyles } from '@material-ui/core/styles';
 import { PluginHeader } from '../../components/PluginHeader';
 import { ActionOutput } from './lib/ActionOutput/ActionOutput';
 import { Layout } from '../../components/Layout';
-import { useBuildWithSteps } from './hooks';
+import { useBuildWithSteps, useSettings } from '../../state';
 
 const BuildName: FC<{ build: BuildWithSteps | null }> = ({ build }) => (
   <>
@@ -89,8 +89,15 @@ const pickClassName = (
 const DetailedViewPage: FC<{}> = () => {
   const { buildId = '' } = useParams();
   const classes = useStyles();
+  const [settings] = useSettings();
+  const [build, { startPolling, stopPolling }] = useBuildWithSteps(
+    parseInt(buildId, 10),
+  );
 
-  const [build] = useBuildWithSteps(parseInt(buildId, 10));
+  useEffect(() => {
+    startPolling();
+    return () => stopPolling();
+  }, [buildId, settings]);
 
   return (
     <Layout>

plugins/circleci/src/pages/SettingsPage/SettingsPage.tsx

@@ -27,7 +27,7 @@ import { Alert } from '@material-ui/lab';
 import { InfoCard, Content } from '@backstage/core';
 import { Layout } from '../../components/Layout';
 import { PluginHeader } from '../../components/PluginHeader';
-import { useSettings } from './settings';
+import { useSettings } from '../../state';
 
 const SettingsPage = () => {
   const [

plugins/circleci/src/state/AppState.tsx

@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 import React, { FC, useReducer, Dispatch, Reducer } from 'react';
-import { circleCIApiRef } from '../../api';
-import { State, PollingState, Action, SettingsState } from './types';
+import { circleCIApiRef } from '../api';
+import { State, Action, SettingsState } from './types';
 export { SettingsState };
 
 export const AppContext = React.createContext<[State, Dispatch<Action>]>(
@@ -29,17 +29,8 @@ const initialState: State = {
     repo: '',
     token: '',
   },
-  builds: {
-    builds: [],
-    pollingIntervalId: null,
-    pollingState: PollingState.Idle,
-  },
-  buildsWithSteps: {
-    builds: {},
-    pollingIntervalId: null,
-    pollingState: PollingState.Idle,
-    getBuildError: null,
-  },
+  builds: [],
+  buildsWithSteps: {},
 };
 
 const reducer: Reducer<State, Action> = (state, action) => {
@@ -47,55 +38,28 @@ const reducer: Reducer<State, Action> = (state, action) => {
     case 'setCredentials':
       return {
         ...state,
-        settings: { ...state.settings, ...(action.payload as {}) },
+        settings: { ...state.settings, ...action.payload },
       };
     case 'setBuilds':
       return {
         ...state,
-        builds: { ...state.builds, builds: action.payload },
+        builds: action.payload,
       };
     case 'setBuildWithSteps': {
-      if (state.buildsWithSteps.pollingState !== PollingState.Polling) {
-        return state;
-      }
       return {
         ...state,
         buildsWithSteps: {
           ...state.buildsWithSteps,
-          builds: {
-            ...state.buildsWithSteps.builds,
-            [action.payload.build_num!]: action.payload,
-          },
+          [action.payload.build_num!]: action.payload,
         },
       };
     }
-    case 'setPollingIntervalId':
-      return {
-        ...state,
-        builds: {
-          ...state.builds,
-          pollingIntervalId: action.payload,
-          pollingState:
-            action.payload === null ? PollingState.Idle : PollingState.Polling,
-        },
-      };
-    case 'setPollingIntervalIdForBuildsWithSteps':
-      return {
-        ...state,
-        buildsWithSteps: {
-          ...state.buildsWithSteps,
-
-          pollingIntervalId: action.payload,
-          pollingState:
-            action.payload === null ? PollingState.Idle : PollingState.Polling,
-        },
-      };
     default:
       return state;
   }
 };
 
-export const Store: FC = ({ children }) => {
+export const AppStateProvider: FC = ({ children }) => {
   const [state, dispatch] = useReducer(reducer, initialState);
 
   return (

plugins/circleci/src/state/index.ts

@@ -13,4 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-export * from './Store';
+export * from './AppState';
+export * from './useBuildWithSteps';
+export * from './useBuilds';
+export * from './useSettings';

plugins/circleci/src/state/types.ts

@@ -0,0 +1,38 @@
+import { BuildSummary, BuildWithSteps } from '../api';
+
+export type SettingsState = {
+  owner: string;
+  repo: string;
+  token: string;
+};
+
+export type BuildsState = BuildSummary[];
+
+export type State = {
+  settings: SettingsState;
+  builds: BuildsState;
+  buildsWithSteps: BuildsWithStepsState;
+};
+
+type SettingsAction = {
+  type: 'setCredentials';
+  payload: {
+    repo: string;
+    owner: string;
+    token: string;
+  };
+};
+
+type BuildsAction = {
+  type: 'setBuilds';
+  payload: BuildSummary[];
+};
+
+type BuildsWithStepsAction = {
+  type: 'setBuildWithSteps';
+  payload: BuildWithSteps;
+};
+
+export type BuildsWithStepsState = Record<number, BuildWithSteps>;
+
+export type Action = SettingsAction | BuildsAction | BuildsWithStepsAction;

plugins/circleci/src/state/useBuildWithSteps.ts

@@ -14,16 +14,18 @@
  * limitations under the License.
  */
 import { errorApiRef, useApi } from '@backstage/core';
-import { useContext, useEffect } from 'react';
-import { circleCIApiRef, GitType } from '../../api/index';
-import { AppContext } from '../../components/Store';
-import { useSettings } from '../SettingsPage/settings';
+import { useContext, useRef } from 'react';
+import { circleCIApiRef, GitType } from '../api/index';
+import { AppContext } from '.';
+import { useSettings } from './useSettings';
 
 const INTERVAL_AMOUNT = 3000;
 
 export function useBuildWithSteps(buildId: number) {
   const [settings] = useSettings();
   const [{ buildsWithSteps }, dispatch] = useContext(AppContext);
+  const intervalId = useRef<number | null>(null);
+  const isPolling = intervalId !== null;
   const api = useApi(circleCIApiRef);
   const errorApi = useApi(errorApiRef);
 
@@ -38,7 +40,7 @@ export function useBuildWithSteps(buildId: number) {
         },
       };
       const build = await api.getBuild(buildId, options);
-      dispatch({ type: 'setBuildWithSteps', payload: build });
+      if (isPolling) dispatch({ type: 'setBuildWithSteps', payload: build });
     } catch (e) {
       errorApi.post(e);
     }
@@ -61,33 +63,18 @@ export function useBuildWithSteps(buildId: number) {
 
   const startPolling = () => {
     stopPolling();
-    const intervalId = (setInterval(
+    intervalId.current = (setInterval(
       () => getBuildWithSteps(),
       INTERVAL_AMOUNT,
     ) as any) as number;
-    dispatch({
-      type: 'setPollingIntervalIdForBuildsWithSteps',
-      payload: intervalId,
-    });
   };
 
   const stopPolling = () => {
-    const currentIntervalId = buildsWithSteps.pollingIntervalId;
+    const currentIntervalId = intervalId.current;
     if (currentIntervalId) clearInterval(currentIntervalId);
-    dispatch({
-      type: 'setPollingIntervalIdForBuildsWithSteps',
-      payload: null,
-    });
   };
 
-  useEffect(() => {
-    startPolling();
-    return () => {
-      stopPolling();
-    };
-  }, [buildId, settings]);
-
-  const build = buildsWithSteps.builds[buildId];
+  const build = buildsWithSteps[buildId];
 
   return [
     build,

plugins/circleci/src/state/useBuilds.tsx

@@ -15,18 +15,15 @@
  */
 import { errorApiRef, useApi } from '@backstage/core';
 import { GitType } from 'circleci-api';
-import { useContext, useEffect } from 'react';
-import { circleCIApiRef } from '../../api/index';
-import { AppContext } from '../../components/Store';
-
-export type BuildsDispatch = {
-  restartBuild: (buildId: number) => Promise<void>;
-};
+import { useContext, useRef } from 'react';
+import { circleCIApiRef } from '../api/index';
+import { AppContext } from '.';
 
 const INTERVAL_AMOUNT = 3000;
 
 export function useBuilds() {
   const [{ builds, settings }, dispatch] = useContext(AppContext);
+  const intervalId = useRef<number | null>(null);
   const api = useApi(circleCIApiRef);
   const errorApi = useApi(errorApiRef);
 
@@ -67,36 +64,23 @@ export function useBuilds() {
 
   const startPolling = () => {
     stopPolling();
-    const intervalId = (setInterval(
+    intervalId.current = (setInterval(
       () => getBuilds(),
       INTERVAL_AMOUNT,
-    ) as any) as number;
-    dispatch({
-      type: 'setPollingIntervalId',
-      payload: intervalId,
-    });
+    ) as unknown) as number;
   };
 
   const stopPolling = () => {
-    const currentIntervalId = builds.pollingIntervalId;
+    const currentIntervalId = intervalId.current;
     if (currentIntervalId) clearInterval(currentIntervalId);
-    dispatch({
-      type: 'setPollingIntervalId',
-      payload: null,
-    });
   };
 
-  useEffect(() => {
-    startPolling();
-    return () => {
-      stopPolling();
-    };
-  }, [settings]);
-
   return [
     builds,
     {
       restartBuild,
+      startPolling,
+      stopPolling,
     },
   ] as const;
 }

plugins/circleci/src/state/useSettings.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 import { useContext, useEffect } from 'react';
-import { AppContext, STORAGE_KEY, SettingsState } from '../../components/Store';
+import { AppContext, STORAGE_KEY, SettingsState } from '.';
 import { useApi, errorApiRef } from '@backstage/core';
 
 // type Effect = {