From bd2ac2c68357cfdf10998d12474b73334babf0b5 Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Thu, 14 May 2020 16:05:22 +0200 Subject: [PATCH] refactor: move state Co-authored-by: Patrik Oldsberg Co-authored-by: Nikita Dudnik --- plugins/circleci/src/components/App.tsx | 6 +- .../circleci/src/components/Store/types.ts | 62 ------------------- .../pages/BuildsPage/lib/Builds/Builds.tsx | 20 ++++-- .../pages/BuildsPage/lib/CITable/CITable.tsx | 4 +- .../DetailedViewPage/DetailedViewPage.tsx | 13 +++- .../src/pages/SettingsPage/SettingsPage.tsx | 2 +- .../Store/Store.tsx => state/AppState.tsx} | 52 +++------------- .../src/{components/Store => state}/index.ts | 5 +- plugins/circleci/src/state/types.ts | 38 ++++++++++++ .../hooks.ts => state/useBuildWithSteps.ts} | 33 +++------- .../builds.tsx => state/useBuilds.tsx} | 34 +++------- .../settings.ts => state/useSettings.ts} | 2 +- 12 files changed, 100 insertions(+), 171 deletions(-) delete mode 100644 plugins/circleci/src/components/Store/types.ts rename plugins/circleci/src/{components/Store/Store.tsx => state/AppState.tsx} (53%) rename plugins/circleci/src/{components/Store => state}/index.ts (82%) create mode 100644 plugins/circleci/src/state/types.ts rename plugins/circleci/src/{pages/DetailedViewPage/hooks.ts => state/useBuildWithSteps.ts} (72%) rename plugins/circleci/src/{pages/BuildsPage/builds.tsx => state/useBuilds.tsx} (75%) rename plugins/circleci/src/{pages/SettingsPage/settings.ts => state/useSettings.ts} (96%) diff --git a/plugins/circleci/src/components/App.tsx b/plugins/circleci/src/components/App.tsx index 68a02b70be..21c710e2b1 100644 --- a/plugins/circleci/src/components/App.tsx +++ b/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 = () => ( - + @@ -16,5 +16,5 @@ export const App = () => ( component={DetailedViewPage} /> - + ); diff --git a/plugins/circleci/src/components/Store/types.ts b/plugins/circleci/src/components/Store/types.ts deleted file mode 100644 index d01bcaf391..0000000000 --- a/plugins/circleci/src/components/Store/types.ts +++ /dev/null @@ -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; - pollingIntervalId: number | null; - pollingState: PollingState; - getBuildError: Error | null; -}; - -export type Action = SettingsAction | BuildsAction | BuildsWithStepsAction; diff --git a/plugins/circleci/src/pages/BuildsPage/lib/Builds/Builds.tsx b/plugins/circleci/src/pages/BuildsPage/lib/Builds/Builds.tsx index f52dff30ad..41001c91dd 100644 --- a/plugins/circleci/src/pages/BuildsPage/lib/Builds/Builds.tsx +++ b/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 ( diff --git a/plugins/circleci/src/pages/BuildsPage/lib/CITable/CITable.tsx b/plugins/circleci/src/pages/BuildsPage/lib/CITable/CITable.tsx index 5524d9985c..962b3b46b2 100644 --- a/plugins/circleci/src/pages/BuildsPage/lib/CITable/CITable.tsx +++ b/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) => ( - + ), diff --git a/plugins/circleci/src/pages/DetailedViewPage/DetailedViewPage.tsx b/plugins/circleci/src/pages/DetailedViewPage/DetailedViewPage.tsx index 19509b32d1..6e6996af58 100644 --- a/plugins/circleci/src/pages/DetailedViewPage/DetailedViewPage.tsx +++ b/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 ( diff --git a/plugins/circleci/src/pages/SettingsPage/SettingsPage.tsx b/plugins/circleci/src/pages/SettingsPage/SettingsPage.tsx index 76f37b955b..62b874d70e 100644 --- a/plugins/circleci/src/pages/SettingsPage/SettingsPage.tsx +++ b/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 [ diff --git a/plugins/circleci/src/components/Store/Store.tsx b/plugins/circleci/src/state/AppState.tsx similarity index 53% rename from plugins/circleci/src/components/Store/Store.tsx rename to plugins/circleci/src/state/AppState.tsx index f6df1897a0..8c2e46d68b 100644 --- a/plugins/circleci/src/components/Store/Store.tsx +++ b/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]>( @@ -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) => { @@ -47,55 +38,28 @@ const reducer: Reducer = (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 ( diff --git a/plugins/circleci/src/components/Store/index.ts b/plugins/circleci/src/state/index.ts similarity index 82% rename from plugins/circleci/src/components/Store/index.ts rename to plugins/circleci/src/state/index.ts index 8ddb059053..24db02aff4 100644 --- a/plugins/circleci/src/components/Store/index.ts +++ b/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'; diff --git a/plugins/circleci/src/state/types.ts b/plugins/circleci/src/state/types.ts new file mode 100644 index 0000000000..8f4849748f --- /dev/null +++ b/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; + +export type Action = SettingsAction | BuildsAction | BuildsWithStepsAction; diff --git a/plugins/circleci/src/pages/DetailedViewPage/hooks.ts b/plugins/circleci/src/state/useBuildWithSteps.ts similarity index 72% rename from plugins/circleci/src/pages/DetailedViewPage/hooks.ts rename to plugins/circleci/src/state/useBuildWithSteps.ts index 78ac6d8c7a..22f5309963 100644 --- a/plugins/circleci/src/pages/DetailedViewPage/hooks.ts +++ b/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(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, diff --git a/plugins/circleci/src/pages/BuildsPage/builds.tsx b/plugins/circleci/src/state/useBuilds.tsx similarity index 75% rename from plugins/circleci/src/pages/BuildsPage/builds.tsx rename to plugins/circleci/src/state/useBuilds.tsx index 9f23e78b31..72e8830042 100644 --- a/plugins/circleci/src/pages/BuildsPage/builds.tsx +++ b/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; -}; +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(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; } diff --git a/plugins/circleci/src/pages/SettingsPage/settings.ts b/plugins/circleci/src/state/useSettings.ts similarity index 96% rename from plugins/circleci/src/pages/SettingsPage/settings.ts rename to plugins/circleci/src/state/useSettings.ts index 8c6de913e9..68f19ee3b9 100644 --- a/plugins/circleci/src/pages/SettingsPage/settings.ts +++ b/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 = {