From 0991032fefadac68205675b4ca03e4e4ad32f4d5 Mon Sep 17 00:00:00 2001 From: kay delaney <45561153+kaydelaney@users.noreply.github.com> Date: Thu, 26 Sep 2019 11:41:37 +0100 Subject: [PATCH] Debt: Simplifies actionCreatorFactory (#19433) - Use sets to keep track of previously defined actionCreators - Remove noPayloadActionCreatorFactory --- public/app/core/actions/application.ts | 4 +- .../core/redux/actionCreatorFactory.test.ts | 10 ++--- public/app/core/redux/actionCreatorFactory.ts | 38 +++++++------------ public/app/features/admin/state/actions.ts | 12 +++--- .../dashboard/panel_editor/state/actions.ts | 4 +- .../app/features/dashboard/state/actions.ts | 10 ++--- .../app/features/datasources/state/actions.ts | 4 +- style_guides/redux.md | 25 ------------ 8 files changed, 33 insertions(+), 74 deletions(-) diff --git a/public/app/core/actions/application.ts b/public/app/core/actions/application.ts index 9bde989e8ca..8345b34c0f0 100644 --- a/public/app/core/actions/application.ts +++ b/public/app/core/actions/application.ts @@ -1,3 +1,3 @@ -import { noPayloadActionCreatorFactory } from 'app/core/redux'; +import { actionCreatorFactory } from 'app/core/redux'; -export const toggleLogActions = noPayloadActionCreatorFactory('TOGGLE_LOG_ACTIONS').create(); +export const toggleLogActions = actionCreatorFactory('TOGGLE_LOG_ACTIONS').create(); diff --git a/public/app/core/redux/actionCreatorFactory.test.ts b/public/app/core/redux/actionCreatorFactory.test.ts index 274079311b3..6683eb040b8 100644 --- a/public/app/core/redux/actionCreatorFactory.test.ts +++ b/public/app/core/redux/actionCreatorFactory.test.ts @@ -1,8 +1,4 @@ -import { - actionCreatorFactory, - resetAllActionCreatorTypes, - noPayloadActionCreatorFactory, -} from './actionCreatorFactory'; +import { actionCreatorFactory, resetAllActionCreatorTypes } from './actionCreatorFactory'; interface Dummy { n: number; @@ -18,7 +14,7 @@ interface Dummy { const setup = (payload?: Dummy) => { resetAllActionCreatorTypes(); const actionCreator = actionCreatorFactory('dummy').create(); - const noPayloadactionCreator = noPayloadActionCreatorFactory('NoPayload').create(); + const noPayloadactionCreator = actionCreatorFactory('NoPayload').create(); const result = actionCreator(payload); const noPayloadResult = noPayloadactionCreator(); @@ -49,7 +45,7 @@ describe('actionCreatorFactory', () => { setup(payload); expect(() => { - noPayloadActionCreatorFactory('DuMmY').create(); + actionCreatorFactory('DuMmY').create(); }).toThrow(); }); }); diff --git a/public/app/core/redux/actionCreatorFactory.ts b/public/app/core/redux/actionCreatorFactory.ts index 1205d1bcfd9..7061facd203 100644 --- a/public/app/core/redux/actionCreatorFactory.ts +++ b/public/app/core/redux/actionCreatorFactory.ts @@ -1,6 +1,6 @@ import { Action } from 'redux'; -const allActionCreators: string[] = []; +const allActionCreators = new Set(); export interface ActionOf extends Action { readonly type: string; @@ -25,33 +25,21 @@ export interface NoPayloadActionCreatorFactory { create: () => NoPayloadActionCreator; } -export const actionCreatorFactory = (type: string): ActionCreatorFactory => { +export function actionCreatorFactory(type: string): NoPayloadActionCreatorFactory; +export function actionCreatorFactory(type: string): ActionCreatorFactory; +export function actionCreatorFactory(type: string): ActionCreatorFactory { + const upperCaseType = type.toLocaleUpperCase(); + if (allActionCreators.has(upperCaseType)) { + throw new Error(`An actionCreator with type '${type}' has already been defined.`); + } + + allActionCreators.add(upperCaseType); + const create = (): ActionCreator => { return Object.assign((payload: Payload): ActionOf => ({ type, payload }), { type }); }; - - if (allActionCreators.some(t => (t && type ? t.toLocaleUpperCase() === type.toLocaleUpperCase() : false))) { - throw new Error(`There is already an actionCreator defined with the type ${type}`); - } - - allActionCreators.push(type); - return { create }; -}; - -export const noPayloadActionCreatorFactory = (type: string): NoPayloadActionCreatorFactory => { - const create = (): NoPayloadActionCreator => { - return Object.assign((): ActionOf => ({ type, payload: undefined }), { type }); - }; - - if (allActionCreators.some(t => (t && type ? t.toLocaleUpperCase() === type.toLocaleUpperCase() : false))) { - throw new Error(`There is already an actionCreator defined with the type ${type}`); - } - - allActionCreators.push(type); - - return { create }; -}; +} export interface NoPayloadActionCreatorMock extends NoPayloadActionCreator { calls: number; @@ -73,4 +61,4 @@ export const mockActionCreator = (creator: ActionCreator) => { }; // Should only be used by tests -export const resetAllActionCreatorTypes = () => (allActionCreators.length = 0); +export const resetAllActionCreatorTypes = () => allActionCreators.clear(); diff --git a/public/app/features/admin/state/actions.ts b/public/app/features/admin/state/actions.ts index 06fda51b6af..68f22c0ba73 100644 --- a/public/app/features/admin/state/actions.ts +++ b/public/app/features/admin/state/actions.ts @@ -1,4 +1,4 @@ -import { actionCreatorFactory, noPayloadActionCreatorFactory } from 'app/core/redux'; +import { actionCreatorFactory } from 'app/core/redux'; import config from 'app/core/config'; import { ThunkResult, SyncInfo, LdapUser, LdapConnectionInfo, LdapError, UserSession, User } from 'app/types'; import { @@ -20,15 +20,15 @@ export const ldapConnectionInfoLoadedAction = actionCreatorFactory('ldap/SYNC_STATUS_LOADED').create(); export const userMappingInfoLoadedAction = actionCreatorFactory('ldap/USER_INFO_LOADED').create(); export const userMappingInfoFailedAction = actionCreatorFactory('ldap/USER_INFO_FAILED').create(); -export const clearUserMappingInfoAction = noPayloadActionCreatorFactory('ldap/CLEAR_USER_MAPPING_INFO').create(); -export const clearUserErrorAction = noPayloadActionCreatorFactory('ldap/CLEAR_USER_ERROR').create(); +export const clearUserMappingInfoAction = actionCreatorFactory('ldap/CLEAR_USER_MAPPING_INFO').create(); +export const clearUserErrorAction = actionCreatorFactory('ldap/CLEAR_USER_ERROR').create(); export const ldapFailedAction = actionCreatorFactory('ldap/LDAP_FAILED').create(); export const userLoadedAction = actionCreatorFactory('USER_LOADED').create(); export const userSessionsLoadedAction = actionCreatorFactory('USER_SESSIONS_LOADED').create(); -export const userSyncFailedAction = noPayloadActionCreatorFactory('USER_SYNC_FAILED').create(); -export const revokeUserSessionAction = noPayloadActionCreatorFactory('REVOKE_USER_SESSION').create(); -export const revokeAllUserSessionsAction = noPayloadActionCreatorFactory('REVOKE_ALL_USER_SESSIONS').create(); +export const userSyncFailedAction = actionCreatorFactory('USER_SYNC_FAILED').create(); +export const revokeUserSessionAction = actionCreatorFactory('REVOKE_USER_SESSION').create(); +export const revokeAllUserSessionsAction = actionCreatorFactory('REVOKE_ALL_USER_SESSIONS').create(); // Actions diff --git a/public/app/features/dashboard/panel_editor/state/actions.ts b/public/app/features/dashboard/panel_editor/state/actions.ts index 8e15749bf6b..7b07f04cc2d 100644 --- a/public/app/features/dashboard/panel_editor/state/actions.ts +++ b/public/app/features/dashboard/panel_editor/state/actions.ts @@ -1,4 +1,4 @@ -import { actionCreatorFactory, noPayloadActionCreatorFactory } from '../../../../core/redux'; +import { actionCreatorFactory } from '../../../../core/redux'; import { PanelEditorTabIds, PanelEditorTab, getPanelEditorTab } from './reducers'; import { ThunkResult } from '../../../../types'; import { updateLocation } from '../../../../core/actions'; @@ -12,7 +12,7 @@ export const panelEditorInitCompleted = actionCreatorFactory('LOAD_DASHBOARD_PERMISSIONS').create(); -export const dashboardInitFetching = noPayloadActionCreatorFactory('DASHBOARD_INIT_FETCHING').create(); +export const dashboardInitFetching = actionCreatorFactory('DASHBOARD_INIT_FETCHING').create(); -export const dashboardInitServices = noPayloadActionCreatorFactory('DASHBOARD_INIT_SERVICES').create(); +export const dashboardInitServices = actionCreatorFactory('DASHBOARD_INIT_SERVICES').create(); -export const dashboardInitSlow = noPayloadActionCreatorFactory('SET_DASHBOARD_INIT_SLOW').create(); +export const dashboardInitSlow = actionCreatorFactory('SET_DASHBOARD_INIT_SLOW').create(); export const dashboardInitCompleted = actionCreatorFactory('DASHBOARD_INIT_COMLETED').create(); @@ -35,7 +35,7 @@ export const dashboardInitFailed = actionCreatorFactory('DAS /* * When leaving dashboard, resets state * */ -export const cleanUpDashboard = noPayloadActionCreatorFactory('DASHBOARD_CLEAN_UP').create(); +export const cleanUpDashboard = actionCreatorFactory('DASHBOARD_CLEAN_UP').create(); export function getDashboardPermissions(id: number): ThunkResult { return async dispatch => { diff --git a/public/app/features/datasources/state/actions.ts b/public/app/features/datasources/state/actions.ts index 3df2c445443..673d321184f 100644 --- a/public/app/features/datasources/state/actions.ts +++ b/public/app/features/datasources/state/actions.ts @@ -9,7 +9,7 @@ import { DataSourceSettings, DataSourcePluginMeta } from '@grafana/ui'; import { StoreState } from 'app/types'; import { LocationUpdate } from '@grafana/runtime'; import { actionCreatorFactory } from 'app/core/redux'; -import { ActionOf, noPayloadActionCreatorFactory } from 'app/core/redux/actionCreatorFactory'; +import { ActionOf } from 'app/core/redux/actionCreatorFactory'; import { getPluginSettings } from 'app/features/plugins/PluginSettingsCache'; import { importDataSourcePlugin } from 'app/features/plugins/plugin_loader'; @@ -19,7 +19,7 @@ export const dataSourcesLoaded = actionCreatorFactory('LOA export const dataSourceMetaLoaded = actionCreatorFactory('LOAD_DATA_SOURCE_META').create(); -export const dataSourceTypesLoad = noPayloadActionCreatorFactory('LOAD_DATA_SOURCE_TYPES').create(); +export const dataSourceTypesLoad = actionCreatorFactory('LOAD_DATA_SOURCE_TYPES').create(); export const dataSourceTypesLoaded = actionCreatorFactory('LOADED_DATA_SOURCE_TYPES').create(); diff --git a/style_guides/redux.md b/style_guides/redux.md index ff64fe400f3..53ce27b193c 100644 --- a/style_guides/redux.md +++ b/style_guides/redux.md @@ -43,31 +43,6 @@ export const someAction = actionCreatorFactory('SOME_ACTION').create(); export const theAction = actionCreatorFactory('SOME_ACTION').create(); // will throw ``` -### noPayloadActionCreatorFactory - -Used when you don't need to supply a payload for your action. Will create an action creator with the following signature - -```typescript -{ type: string , (): {type: string; payload: undefined;} } -``` - -where the `type` string will be ensured to be unique. - -#### Example - -```typescript -export const noPayloadAction = noPayloadActionCreatorFactory('NO_PAYLOAD').create(); - -// later when dispatched -noPayloadAction(); -``` - -```typescript -// declaring an action creator with a type string that has already been defined will throw -export const noPayloadAction = noPayloadActionCreatorFactory('NO_PAYLOAD').create(); -export const noAction = noPayloadActionCreatorFactory('NO_PAYLOAD').create(); // will throw -``` - ### reducerFactory Fluent API used to create a reducer. (same as implementing the standard switch statement in Redux)