From 989f98efdadb2000423dc672effdc65e6de944e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Fri, 4 Oct 2019 04:37:16 -0700 Subject: [PATCH] Feature: Adds connectWithCleanup HOC (#19392) * Feature: Adds connectWithCleanup HOC * Refactor: Small typings * Refactor: Makes UseEffect run on Mount and UnMount only * Refactor: Adds tests and rootReducer --- public/app/core/actions/cleanUp.ts | 10 ++ .../core/components/connectWithCleanUp.tsx | 39 ++++++++ public/app/core/reducers/root.test.ts | 99 +++++++++++++++++++ public/app/core/reducers/root.ts | 70 +++++++++++++ public/app/features/teams/TeamList.tsx | 13 +-- public/app/store/configureStore.ts | 39 +------- public/app/types/store.ts | 2 + public/test/core/redux/reducerTester.ts | 26 ++++- 8 files changed, 250 insertions(+), 48 deletions(-) create mode 100644 public/app/core/actions/cleanUp.ts create mode 100644 public/app/core/components/connectWithCleanUp.tsx create mode 100644 public/app/core/reducers/root.test.ts create mode 100644 public/app/core/reducers/root.ts diff --git a/public/app/core/actions/cleanUp.ts b/public/app/core/actions/cleanUp.ts new file mode 100644 index 00000000000..991fd6e8d75 --- /dev/null +++ b/public/app/core/actions/cleanUp.ts @@ -0,0 +1,10 @@ +import { StoreState } from '../../types'; +import { actionCreatorFactory } from '../redux'; + +export type StateSelector = (state: StoreState) => T; + +export interface CleanUp { + stateSelector: StateSelector; +} + +export const cleanUpAction = actionCreatorFactory>('CORE_CLEAN_UP_STATE').create(); diff --git a/public/app/core/components/connectWithCleanUp.tsx b/public/app/core/components/connectWithCleanUp.tsx new file mode 100644 index 00000000000..c6b4561cee7 --- /dev/null +++ b/public/app/core/components/connectWithCleanUp.tsx @@ -0,0 +1,39 @@ +import { MapStateToPropsParam, MapDispatchToPropsParam, connect, useDispatch } from 'react-redux'; +import { StateSelector, cleanUpAction } from '../actions/cleanUp'; +import React, { ComponentType, FunctionComponent, useEffect } from 'react'; +import hoistNonReactStatics from 'hoist-non-react-statics'; + +export const connectWithCleanUp = < + TStateProps extends {} = {}, + TDispatchProps = {}, + TOwnProps = {}, + State = {}, + TSelector extends object = {}, + Statics = {} +>( + mapStateToProps: MapStateToPropsParam, + mapDispatchToProps: MapDispatchToPropsParam, + stateSelector: StateSelector +) => (Component: ComponentType) => { + const ConnectedComponent = connect( + mapStateToProps, + mapDispatchToProps + )(Component); + + const ConnectedComponentWithCleanUp: FunctionComponent = props => { + const dispatch = useDispatch(); + useEffect(() => { + return function cleanUp() { + dispatch(cleanUpAction({ stateSelector })); + }; + }, []); + // @ts-ignore + return ; + }; + + ConnectedComponentWithCleanUp.displayName = `ConnectWithCleanUp(${ConnectedComponent.displayName})`; + hoistNonReactStatics(ConnectedComponentWithCleanUp, Component); + type Hoisted = typeof ConnectedComponentWithCleanUp & Statics; + + return ConnectedComponentWithCleanUp as Hoisted; +}; diff --git a/public/app/core/reducers/root.test.ts b/public/app/core/reducers/root.test.ts new file mode 100644 index 00000000000..761b9062e97 --- /dev/null +++ b/public/app/core/reducers/root.test.ts @@ -0,0 +1,99 @@ +import { recursiveCleanState, rootReducer } from './root'; +import { describe, expect } from '../../../test/lib/common'; +import { NavModelItem } from '@grafana/data'; +import { reducerTester } from '../../../test/core/redux/reducerTester'; +import { StoreState } from '../../types/store'; +import { ActionTypes } from '../../features/teams/state/actions'; +import { Team } from '../../types'; +import { cleanUpAction } from '../actions/cleanUp'; +import { initialTeamsState } from '../../features/teams/state/reducers'; + +jest.mock('@grafana/runtime', () => ({ + config: { + bootData: { + navTree: [] as NavModelItem[], + user: {}, + }, + }, +})); + +describe('recursiveCleanState', () => { + describe('when called with an existing state selector', () => { + it('then it should clear that state slice in state', () => { + const state = { + teams: { teams: [{ id: 1 }, { id: 2 }] }, + }; + // Choosing a deeper state selector here just to test recursive behaviour + // This should be same state slice that matches the state slice of a reducer like state.teams + const stateSelector = state.teams.teams[0]; + + recursiveCleanState(state, stateSelector); + + expect(state.teams.teams[0]).not.toBeDefined(); + expect(state.teams.teams[1]).toBeDefined(); + }); + }); + + describe('when called with a non existing state selector', () => { + it('then it should not clear that state slice in state', () => { + const state = { + teams: { teams: [{ id: 1 }, { id: 2 }] }, + }; + // Choosing a deeper state selector here just to test recursive behaviour + // This should be same state slice that matches the state slice of a reducer like state.teams + const stateSelector = state.teams.teams[2]; + + recursiveCleanState(state, stateSelector); + + expect(state.teams.teams[0]).toBeDefined(); + expect(state.teams.teams[1]).toBeDefined(); + }); + }); +}); + +describe('rootReducer', () => { + describe('when called with any action except cleanUpAction', () => { + it('then it should not clean state', () => { + const teams = [{ id: 1 }]; + const state = { + teams: { ...initialTeamsState }, + } as StoreState; + + reducerTester() + .givenReducer(rootReducer, state) + .whenActionIsDispatched({ + type: ActionTypes.LoadTeams, + payload: teams, + }) + .thenStatePredicateShouldEqual(resultingState => { + expect(resultingState.teams).toEqual({ + hasFetched: true, + searchQuery: '', + teams, + }); + return true; + }); + }); + }); + + describe('when called with cleanUpAction', () => { + it('then it should clean state', () => { + const teams = [{ id: 1 }] as Team[]; + const state: StoreState = { + teams: { + hasFetched: true, + searchQuery: '', + teams, + }, + } as StoreState; + + reducerTester() + .givenReducer(rootReducer, state, true) + .whenActionIsDispatched(cleanUpAction({ stateSelector: storeState => storeState.teams })) + .thenStatePredicateShouldEqual(resultingState => { + expect(resultingState.teams).toEqual({ ...initialTeamsState }); + return true; + }); + }); + }); +}); diff --git a/public/app/core/reducers/root.ts b/public/app/core/reducers/root.ts new file mode 100644 index 00000000000..4f6a4153c4a --- /dev/null +++ b/public/app/core/reducers/root.ts @@ -0,0 +1,70 @@ +import { combineReducers } from 'redux'; + +import { StoreState } from '../../types'; +import { ActionOf } from '../redux'; +import { CleanUp, cleanUpAction } from '../actions/cleanUp'; +import sharedReducers from 'app/core/reducers'; +import alertingReducers from 'app/features/alerting/state/reducers'; +import teamsReducers from 'app/features/teams/state/reducers'; +import apiKeysReducers from 'app/features/api-keys/state/reducers'; +import foldersReducers from 'app/features/folders/state/reducers'; +import dashboardReducers from 'app/features/dashboard/state/reducers'; +import exploreReducers from 'app/features/explore/state/reducers'; +import pluginReducers from 'app/features/plugins/state/reducers'; +import dataSourcesReducers from 'app/features/datasources/state/reducers'; +import usersReducers from 'app/features/users/state/reducers'; +import userReducers from 'app/features/profile/state/reducers'; +import organizationReducers from 'app/features/org/state/reducers'; +import ldapReducers from 'app/features/admin/state/reducers'; + +const rootReducers = { + ...sharedReducers, + ...alertingReducers, + ...teamsReducers, + ...apiKeysReducers, + ...foldersReducers, + ...dashboardReducers, + ...exploreReducers, + ...pluginReducers, + ...dataSourcesReducers, + ...usersReducers, + ...userReducers, + ...organizationReducers, + ...ldapReducers, +}; + +export function addRootReducer(reducers: any) { + Object.assign(rootReducers, ...reducers); +} + +export const recursiveCleanState = (state: any, stateSlice: any): boolean => { + for (const stateKey in state) { + if (state[stateKey] === stateSlice) { + state[stateKey] = undefined; + return true; + } + + if (typeof state[stateKey] === 'object') { + const cleaned = recursiveCleanState(state[stateKey], stateSlice); + if (cleaned) { + return true; + } + } + } + + return false; +}; + +const appReducer = combineReducers(rootReducers); + +export const rootReducer = (state: StoreState, action: ActionOf): StoreState => { + if (action.type !== cleanUpAction.type) { + return appReducer(state, action); + } + + const { stateSelector } = action.payload as CleanUp; + const stateSlice = stateSelector(state); + recursiveCleanState(state, stateSlice); + + return appReducer(state, action); +}; diff --git a/public/app/features/teams/TeamList.tsx b/public/app/features/teams/TeamList.tsx index 445c039d3a3..fde2d9028f0 100644 --- a/public/app/features/teams/TeamList.tsx +++ b/public/app/features/teams/TeamList.tsx @@ -1,17 +1,17 @@ import React, { PureComponent } from 'react'; -import { connect } from 'react-redux'; import { hot } from 'react-hot-loader'; import Page from 'app/core/components/Page/Page'; import { DeleteButton } from '@grafana/ui'; import { NavModel } from '@grafana/data'; import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA'; -import { Team, OrgRole } from 'app/types'; +import { Team, OrgRole, StoreState } from 'app/types'; import { loadTeams, deleteTeam, setSearchQuery } from './state/actions'; import { getSearchQuery, getTeams, getTeamsCount, isPermissionTeamAdmin } from './state/selectors'; import { getNavModel } from 'app/core/selectors/navModel'; import { FilterInput } from 'app/core/components/FilterInput/FilterInput'; import { config } from 'app/core/config'; import { contextSrv, User } from 'app/core/services/context_srv'; +import { connectWithCleanUp } from '../../core/components/connectWithCleanUp'; export interface Props { navModel: NavModel; @@ -152,7 +152,7 @@ export class TeamList extends PureComponent { } } -function mapStateToProps(state: any) { +function mapStateToProps(state: StoreState) { return { navModel: getNavModel(state.navIndex, 'teams'), teams: getTeams(state.teams), @@ -170,9 +170,4 @@ const mapDispatchToProps = { setSearchQuery, }; -export default hot(module)( - connect( - mapStateToProps, - mapDispatchToProps - )(TeamList) -); +export default hot(module)(connectWithCleanUp(mapStateToProps, mapDispatchToProps, state => state.teams)(TeamList)); diff --git a/public/app/store/configureStore.ts b/public/app/store/configureStore.ts index 7c210cb81eb..ffeb309aaa4 100644 --- a/public/app/store/configureStore.ts +++ b/public/app/store/configureStore.ts @@ -1,46 +1,15 @@ -import { createStore, applyMiddleware, compose, combineReducers } from 'redux'; +import { applyMiddleware, compose, createStore } from 'redux'; import thunk from 'redux-thunk'; import { createLogger } from 'redux-logger'; -import sharedReducers from 'app/core/reducers'; -import alertingReducers from 'app/features/alerting/state/reducers'; -import teamsReducers from 'app/features/teams/state/reducers'; -import apiKeysReducers from 'app/features/api-keys/state/reducers'; -import foldersReducers from 'app/features/folders/state/reducers'; -import dashboardReducers from 'app/features/dashboard/state/reducers'; -import exploreReducers from 'app/features/explore/state/reducers'; -import pluginReducers from 'app/features/plugins/state/reducers'; -import dataSourcesReducers from 'app/features/datasources/state/reducers'; -import usersReducers from 'app/features/users/state/reducers'; -import userReducers from 'app/features/profile/state/reducers'; -import organizationReducers from 'app/features/org/state/reducers'; -import ldapReducers from 'app/features/admin/state/reducers'; + import { setStore } from './store'; import { StoreState } from 'app/types/store'; import { toggleLogActionsMiddleware } from 'app/core/middlewares/application'; - -const rootReducers = { - ...sharedReducers, - ...alertingReducers, - ...teamsReducers, - ...apiKeysReducers, - ...foldersReducers, - ...dashboardReducers, - ...exploreReducers, - ...pluginReducers, - ...dataSourcesReducers, - ...usersReducers, - ...userReducers, - ...organizationReducers, - ...ldapReducers, -}; - -export function addRootReducer(reducers: any) { - Object.assign(rootReducers, ...reducers); -} +import { rootReducer } from '../core/reducers/root'; export function configureStore() { const composeEnhancers = (window as any).__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose; - const rootReducer = combineReducers(rootReducers); + const logger = createLogger({ predicate: (getState: () => StoreState) => { return getState().application.logActions; diff --git a/public/app/types/store.ts b/public/app/types/store.ts index e0973704d41..fd4358a8c82 100644 --- a/public/app/types/store.ts +++ b/public/app/types/store.ts @@ -16,6 +16,7 @@ import { NavIndex } from '@grafana/data'; import { ApplicationState } from './application'; import { LdapState, LdapUserState } from './ldap'; import { PanelEditorState } from '../features/dashboard/panel_editor/state/reducers'; +import { ApiKeysState } from './apiKeys'; export interface StoreState { navIndex: NavIndex; @@ -36,6 +37,7 @@ export interface StoreState { application: ApplicationState; ldap: LdapState; ldapUser: LdapUserState; + apiKeys: ApiKeysState; } /* diff --git a/public/test/core/redux/reducerTester.ts b/public/test/core/redux/reducerTester.ts index 5a0cf7f7b69..32ce4e84fc6 100644 --- a/public/test/core/redux/reducerTester.ts +++ b/public/test/core/redux/reducerTester.ts @@ -3,7 +3,7 @@ import { Reducer } from 'redux'; import { ActionOf } from 'app/core/redux/actionCreatorFactory'; export interface Given { - givenReducer: (reducer: Reducer>, state: State) => When; + givenReducer: (reducer: Reducer>, state: State, disableDeepFreeze?: boolean) => When; } export interface When { @@ -12,6 +12,7 @@ export interface When { export interface Then { thenStateShouldEqual: (state: State) => When; + thenStatePredicateShouldEqual: (predicate: (resultingState: State) => boolean) => When; } interface ObjectType extends Object { @@ -53,10 +54,16 @@ export const reducerTester = (): Given => { let resultingState: State; let initialState: State; - const givenReducer = (reducer: Reducer>, state: State): When => { + const givenReducer = ( + reducer: Reducer>, + state: State, + disableDeepFreeze = false + ): When => { reducerUnderTest = reducer; initialState = { ...state }; - initialState = deepFreeze(initialState); + if (!disableDeepFreeze) { + initialState = deepFreeze(initialState); + } return instance; }; @@ -73,7 +80,18 @@ export const reducerTester = (): Given => { return instance; }; - const instance: ReducerTester = { thenStateShouldEqual, givenReducer, whenActionIsDispatched }; + const thenStatePredicateShouldEqual = (predicate: (resultingState: State) => boolean): When => { + expect(predicate(resultingState)).toBe(true); + + return instance; + }; + + const instance: ReducerTester = { + thenStateShouldEqual, + thenStatePredicateShouldEqual, + givenReducer, + whenActionIsDispatched, + }; return instance; };