From d64d50a99f70e85e010fc6160d93fe74a6fe01eb Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Tue, 23 Feb 2021 15:26:43 -0800 Subject: [PATCH] implement state management in redux --- .../common/hooks/apiResources/apiResources.ts | 140 +++++++++++++----- .../src/common/hooks/apiResources/index.ts | 6 +- .../src/dashboard/reducers/getInitialState.js | 2 + .../src/dashboard/reducers/index.js | 2 + 4 files changed, 110 insertions(+), 40 deletions(-) diff --git a/superset-frontend/src/common/hooks/apiResources/apiResources.ts b/superset-frontend/src/common/hooks/apiResources/apiResources.ts index 99504c7f31..2e42e0bf0b 100644 --- a/superset-frontend/src/common/hooks/apiResources/apiResources.ts +++ b/superset-frontend/src/common/hooks/apiResources/apiResources.ts @@ -17,8 +17,10 @@ * under the License. */ +import { createContext, useEffect, useMemo, useRef, useState } from 'react'; +import { Action, Dispatch } from 'redux'; import { makeApi } from '@superset-ui/core'; -import { useEffect, useMemo, useRef, useState } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; export enum ResourceStatus { LOADING = 'loading', @@ -32,6 +34,10 @@ export enum ResourceStatus { */ export type Resource = LoadingState | CompleteState | ErrorState; +export type ReduxState = { + [url: string]: Resource; +}; + // Trying out something a little different: a separate type per status. // This should let Typescript know whether a Resource has a result or error. // It's possible that I'm expecting too much from Typescript here. @@ -61,12 +67,87 @@ type ErrorState = { error: Error; }; -const initialState: LoadingState = { +const loadingState: LoadingState = { status: ResourceStatus.LOADING, result: null, error: null, }; +const RESOURCE_FETCH_START = 'RESOURCE_FETCH_START'; +const RESOURCE_FETCH_COMPLETE = 'RESOURCE_FETCH_COMPLETE'; +const RESOURCE_FETCH_ERROR = 'RESROUCE_FETCH_ERROR'; + +type FetchStart = { + type: typeof RESOURCE_FETCH_START; + endpoint: string; +}; + +type FetchComplete = { + type: typeof RESOURCE_FETCH_COMPLETE; + endpoint: string; + result: T; +}; + +type FetchError = { + type: typeof RESOURCE_FETCH_ERROR; + endpoint: string; + error: Error; +}; + +type ResourceAction = FetchStart | FetchComplete | FetchError; + +export type ResourcesState = Record>; + +export const initialResourcesState: ResourcesState = {}; + +export function resourcesReducer( + state: ResourcesState = initialResourcesState, + action: ResourceAction, +): ResourcesState { + switch (action.type) { + case RESOURCE_FETCH_START: { + return { + ...state, + [action.endpoint]: { + status: ResourceStatus.LOADING, + result: null, + error: null, + }, + }; + } + case RESOURCE_FETCH_COMPLETE: { + const { endpoint, result } = action; + return { + ...state, + [endpoint]: { + status: ResourceStatus.COMPLETE, + result, + error: null, + }, + }; + } + case RESOURCE_FETCH_ERROR: { + const { endpoint, error } = action as FetchError; + return { + ...state, + [endpoint]: { + status: ResourceStatus.ERROR, + result: null, + error, + }, + }; + } + default: + return state; + } +} + +type ReduxRootState = { apiResources: ResourcesState }; + +const selectResourceForEndpoint = (endpoint: string) => ( + state: ReduxRootState, +): Resource | null => state.apiResources[endpoint] ?? null; + /** * A general-purpose hook to fetch the response from an endpoint. * Returns the full response body from the API, including metadata. @@ -87,21 +168,19 @@ const initialState: LoadingState = { export function useApiResourceFullBody( endpoint: string, ): Resource { - const [resource, setResource] = useState>(initialState); - const cancelRef = useRef<() => void>(() => {}); + const dispatch = useDispatch>(); + const resource = useSelector(selectResourceForEndpoint(endpoint)); useEffect(() => { - // If refresh is implemented, this will need to change. - // The previous values should stay during refresh. - setResource(initialState); + if (resource != null) { + // fetching already underway/complete, don't duplicate work. + return; + } - // when this effect runs, the endpoint has changed. - // cancel any current calls so that state doesn't get messed up. - cancelRef.current(); - let cancelled = false; - cancelRef.current = () => { - cancelled = true; - }; + dispatch({ + type: RESOURCE_FETCH_START, + endpoint, + }); const fetchResource = makeApi<{}, RESULT>({ method: 'GET', @@ -110,31 +189,22 @@ export function useApiResourceFullBody( fetchResource({}) .then(result => { - if (!cancelled) { - setResource({ - status: ResourceStatus.COMPLETE, - result, - error: null, - }); - } + dispatch({ + type: RESOURCE_FETCH_COMPLETE, + endpoint, + result, + }); }) .catch(error => { - if (!cancelled) { - setResource({ - status: ResourceStatus.ERROR, - result: null, - error, - }); - } + dispatch({ + type: RESOURCE_FETCH_ERROR, + endpoint, + error, + }); }); + }, [endpoint, resource, dispatch]); - // Cancel the request when the component un-mounts - return () => { - cancelled = true; - }; - }, [endpoint]); - - return resource; + return resource ?? loadingState; } /** diff --git a/superset-frontend/src/common/hooks/apiResources/index.ts b/superset-frontend/src/common/hooks/apiResources/index.ts index 8befc73735..9c6f159bf7 100644 --- a/superset-frontend/src/common/hooks/apiResources/index.ts +++ b/superset-frontend/src/common/hooks/apiResources/index.ts @@ -17,11 +17,7 @@ * under the License. */ -export { - useApiResourceFullBody, - useApiV1Resource, - useTransformedResource, -} from './apiResources'; +export * from './apiResources'; // A central catalog of API Resource hooks. // Add new API hooks here, organized under diff --git a/superset-frontend/src/dashboard/reducers/getInitialState.js b/superset-frontend/src/dashboard/reducers/getInitialState.js index 1157997606..76b66bb34b 100644 --- a/superset-frontend/src/dashboard/reducers/getInitialState.js +++ b/superset-frontend/src/dashboard/reducers/getInitialState.js @@ -47,6 +47,7 @@ import getFilterConfigsFromFormdata from '../util/getFilterConfigsFromFormdata'; import getLocationHash from '../util/getLocationHash'; import newComponentFactory from '../util/newComponentFactory'; import { TIME_RANGE } from '../../visualizations/FilterBox/FilterBox'; +import { initialResourcesState } from 'src/common/hooks/apiResources/apiResources'; export default function getInitialState(bootstrapData) { const { user_id, datasources, common, editMode, urlParams } = bootstrapData; @@ -308,5 +309,6 @@ export default function getInitialState(bootstrapData) { dashboardLayout, messageToasts: [], impressionId: shortid.generate(), + apiResources: initialResourcesState, }; } diff --git a/superset-frontend/src/dashboard/reducers/index.js b/superset-frontend/src/dashboard/reducers/index.js index 61964de92e..95b25f47b8 100644 --- a/superset-frontend/src/dashboard/reducers/index.js +++ b/superset-frontend/src/dashboard/reducers/index.js @@ -18,6 +18,7 @@ */ import { combineReducers } from 'redux'; +import { resourcesReducer } from 'src/common/hooks/apiResources'; import charts from '../../chart/chartReducer'; import dashboardInfo from './dashboardInfo'; import dashboardState from './dashboardState'; @@ -41,4 +42,5 @@ export default combineReducers({ impressionId, messageToasts, sliceEntities, + apiResources: resourcesReducer, });