Debt: Simplifies actionCreatorFactory (grafana#19433)

- Use sets to keep track of previously defined actionCreators
- Remove noPayloadActionCreatorFactory

Author: kaydelaney

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();

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>('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();
     });
   });

public/app/core/redux/actionCreatorFactory.ts

@@ -1,6 +1,6 @@
 import { Action } from 'redux';
 
-const allActionCreators: string[] = [];
+const allActionCreators = new Set<string>();
 
 export interface ActionOf<Payload> extends Action {
   readonly type: string;
@@ -25,33 +25,21 @@ export interface NoPayloadActionCreatorFactory {
   create: () => NoPayloadActionCreator;
 }
 
-export const actionCreatorFactory = <Payload>(type: string): ActionCreatorFactory<Payload> => {
-  const create = (): ActionCreator<Payload> => {
-    return Object.assign((payload: Payload): ActionOf<Payload> => ({ 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}`);
+export function actionCreatorFactory<Payload extends undefined>(type: string): NoPayloadActionCreatorFactory;
+export function actionCreatorFactory<Payload>(type: string): ActionCreatorFactory<Payload>;
+export function actionCreatorFactory<Payload>(type: string): ActionCreatorFactory<Payload> {
+  const upperCaseType = type.toLocaleUpperCase();
+  if (allActionCreators.has(upperCaseType)) {
+    throw new Error(`An actionCreator with type '${type}' has already been defined.`);
   }
 
-  allActionCreators.push(type);
+  allActionCreators.add(upperCaseType);
 
-  return { create };
-};
-
-export const noPayloadActionCreatorFactory = (type: string): NoPayloadActionCreatorFactory => {
-  const create = (): NoPayloadActionCreator => {
-    return Object.assign((): ActionOf<undefined> => ({ type, payload: undefined }), { type });
+  const create = (): ActionCreator<Payload> => {
+    return Object.assign((payload: Payload): ActionOf<Payload> => ({ 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 interface NoPayloadActionCreatorMock extends NoPayloadActionCreator {
   calls: number;
@@ -73,4 +61,4 @@ export const mockActionCreator = (creator: ActionCreator<any>) => {
 };
 
 // Should only be used by tests
-export const resetAllActionCreatorTypes = () => (allActionCreators.length = 0);
+export const resetAllActionCreatorTypes = () => allActionCreators.clear();

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<LdapConnectio
 export const ldapSyncStatusLoadedAction = actionCreatorFactory<SyncInfo>('ldap/SYNC_STATUS_LOADED').create();
 export const userMappingInfoLoadedAction = actionCreatorFactory<LdapUser>('ldap/USER_INFO_LOADED').create();
 export const userMappingInfoFailedAction = actionCreatorFactory<LdapError>('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<LdapError>('ldap/LDAP_FAILED').create();
 
 export const userLoadedAction = actionCreatorFactory<User>('USER_LOADED').create();
 export const userSessionsLoadedAction = actionCreatorFactory<UserSession[]>('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
 

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<PanelEditorInitComp
   'PANEL_EDITOR_INIT_COMPLETED'
 ).create();
 
-export const panelEditorCleanUp = noPayloadActionCreatorFactory('PANEL_EDITOR_CLEAN_UP').create();
+export const panelEditorCleanUp = actionCreatorFactory('PANEL_EDITOR_CLEAN_UP').create();
 
 export const refreshPanelEditor = (props: {
   hasQueriesTab?: boolean;

public/app/features/dashboard/state/actions.ts

@@ -1,6 +1,6 @@
 // Services & Utils
 import { getBackendSrv } from '@grafana/runtime';
-import { actionCreatorFactory, noPayloadActionCreatorFactory } from 'app/core/redux';
+import { actionCreatorFactory } from 'app/core/redux';
 import { createSuccessNotification } from 'app/core/copy/appNotification';
 // Actions
 import { loadPluginDashboards } from '../../plugins/state/actions';
@@ -19,11 +19,11 @@ import {
 
 export const loadDashboardPermissions = actionCreatorFactory<DashboardAclDTO[]>('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<MutableDashboard>('DASHBOARD_INIT_COMLETED').create();
 
@@ -35,7 +35,7 @@ export const dashboardInitFailed = actionCreatorFactory<DashboardInitError>('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<void> {
   return async dispatch => {

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<DataSourceSettings[]>('LOA
 
 export const dataSourceMetaLoaded = actionCreatorFactory<DataSourcePluginMeta>('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<DataSourcePluginMeta[]>('LOADED_DATA_SOURCE_TYPES').create();
 

style_guides/redux.md

@@ -43,31 +43,6 @@ export const someAction = actionCreatorFactory<string>('SOME_ACTION').create();
 export const theAction = actionCreatorFactory<string>('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)