Further improve type checking for actions and triggers (#58765)

* wip * review follow up * make ACTION a prefix, not SUFFIX * fix path * add warnings about casting to ActionType * Make context an object in examples, not a string * require object context, which seems to fix the partial requirement in type and thus the type issue * mistake Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elastic · Mar 4, 2020 · d2cbc59 · d2cbc59
1 parent aea4811
commit d2cbc59
Show file tree

Hide file tree

Showing 62 changed files with 565 additions and 396 deletions.
diff --git a/examples/ui_action_examples/public/hello_world_action.tsx b/examples/ui_action_examples/public/hello_world_action.tsx
@@ -22,15 +22,15 @@ import { OverlayStart } from '../../../src/core/public';
 import { createAction } from '../../../src/plugins/ui_actions/public';
 import { toMountPoint } from '../../../src/plugins/kibana_react/public';
 
-export const HELLO_WORLD_ACTION_TYPE = 'HELLO_WORLD_ACTION_TYPE';
+export const ACTION_HELLO_WORLD = 'ACTION_HELLO_WORLD';
 
 interface StartServices {
   openModal: OverlayStart['openModal'];
 }
 
 export const createHelloWorldAction = (getStartServices: () => Promise<StartServices>) =>
   createAction({
-    type: HELLO_WORLD_ACTION_TYPE,
+    type: ACTION_HELLO_WORLD,
     getDisplayName: () => 'Hello World!',
     execute: async () => {
       const { openModal } = await getStartServices();

diff --git a/examples/ui_action_examples/public/index.ts b/examples/ui_action_examples/public/index.ts
@@ -23,4 +23,4 @@ import { PluginInitializer } from '../../../src/core/public';
 export const plugin: PluginInitializer<void, void> = () => new UiActionExamplesPlugin();
 
 export { HELLO_WORLD_TRIGGER_ID } from './hello_world_trigger';
-export { HELLO_WORLD_ACTION_TYPE } from './hello_world_action';
+export { ACTION_HELLO_WORLD } from './hello_world_action';
diff --git a/examples/ui_action_examples/public/plugin.ts b/examples/ui_action_examples/public/plugin.ts
@@ -19,7 +19,7 @@
 
 import { Plugin, CoreSetup } from '../../../src/core/public';
 import { UiActionsSetup } from '../../../src/plugins/ui_actions/public';
-import { createHelloWorldAction } from './hello_world_action';
+import { createHelloWorldAction, ACTION_HELLO_WORLD } from './hello_world_action';
 import { helloWorldTrigger, HELLO_WORLD_TRIGGER_ID } from './hello_world_trigger';
 
 interface UiActionExamplesSetupDependencies {
@@ -28,7 +28,11 @@ interface UiActionExamplesSetupDependencies {
 
 declare module '../../../src/plugins/ui_actions/public' {
   export interface TriggerContextMapping {
-    [HELLO_WORLD_TRIGGER_ID]: undefined;
+    [HELLO_WORLD_TRIGGER_ID]: {};
+  }
+
+  export interface ActionContextMapping {
+    [ACTION_HELLO_WORLD]: {};
   }
 }
 
@@ -42,7 +46,7 @@ export class UiActionExamplesPlugin
     }));
 
     uiActions.registerAction(helloWorldAction);
-    uiActions.attachAction(helloWorldTrigger.id, helloWorldAction.id);
+    uiActions.attachAction(helloWorldTrigger.id, helloWorldAction);
   }
 
   public start() {}

diff --git a/examples/ui_actions_explorer/public/actions/actions.tsx b/examples/ui_actions_explorer/public/actions/actions.tsx
@@ -27,44 +27,48 @@ export const USER_TRIGGER = 'USER_TRIGGER';
 export const COUNTRY_TRIGGER = 'COUNTRY_TRIGGER';
 export const PHONE_TRIGGER = 'PHONE_TRIGGER';
 
-export const VIEW_IN_MAPS_ACTION = 'VIEW_IN_MAPS_ACTION';
-export const TRAVEL_GUIDE_ACTION = 'TRAVEL_GUIDE_ACTION';
-export const CALL_PHONE_NUMBER_ACTION = 'CALL_PHONE_NUMBER_ACTION';
-export const EDIT_USER_ACTION = 'EDIT_USER_ACTION';
-export const PHONE_USER_ACTION = 'PHONE_USER_ACTION';
-export const SHOWCASE_PLUGGABILITY_ACTION = 'SHOWCASE_PLUGGABILITY_ACTION';
+export const ACTION_VIEW_IN_MAPS = 'ACTION_VIEW_IN_MAPS';
+export const ACTION_TRAVEL_GUIDE = 'ACTION_TRAVEL_GUIDE';
+export const ACTION_CALL_PHONE_NUMBER = 'ACTION_CALL_PHONE_NUMBER';
+export const ACTION_EDIT_USER = 'ACTION_EDIT_USER';
+export const ACTION_PHONE_USER = 'ACTION_PHONE_USER';
+export const ACTION_SHOWCASE_PLUGGABILITY = 'ACTION_SHOWCASE_PLUGGABILITY';
 
-export const showcasePluggability = createAction({
-  type: SHOWCASE_PLUGGABILITY_ACTION,
+export const showcasePluggability = createAction<typeof ACTION_SHOWCASE_PLUGGABILITY>({
+  type: ACTION_SHOWCASE_PLUGGABILITY,
   getDisplayName: () => 'This is pluggable! Any plugin can inject their actions here.',
   execute: async () => alert("Isn't that cool?!"),
 });
 
-export type PhoneContext = string;
+export interface PhoneContext {
+  phone: string;
+}
 
-export const makePhoneCallAction = createAction<PhoneContext>({
-  type: CALL_PHONE_NUMBER_ACTION,
+export const makePhoneCallAction = createAction<typeof ACTION_CALL_PHONE_NUMBER>({
+  type: ACTION_CALL_PHONE_NUMBER,
   getDisplayName: () => 'Call phone number',
-  execute: async phone => alert(`Pretend calling ${phone}...`),
+  execute: async context => alert(`Pretend calling ${context.phone}...`),
 });
 
-export const lookUpWeatherAction = createAction<{ country: string }>({
-  type: TRAVEL_GUIDE_ACTION,
+export const lookUpWeatherAction = createAction<typeof ACTION_TRAVEL_GUIDE>({
+  type: ACTION_TRAVEL_GUIDE,
   getIconType: () => 'popout',
   getDisplayName: () => 'View travel guide',
-  execute: async ({ country }) => {
-    window.open(`https://www.worldtravelguide.net/?s=${country},`, '_blank');
+  execute: async context => {
+    window.open(`https://www.worldtravelguide.net/?s=${context.country}`, '_blank');
   },
 });
 
-export type CountryContext = string;
+export interface CountryContext {
+  country: string;
+}
 
-export const viewInMapsAction = createAction<CountryContext>({
-  type: VIEW_IN_MAPS_ACTION,
+export const viewInMapsAction = createAction<typeof ACTION_VIEW_IN_MAPS>({
+  type: ACTION_VIEW_IN_MAPS,
   getIconType: () => 'popout',
   getDisplayName: () => 'View in maps',
-  execute: async country => {
-    window.open(`https://www.google.com/maps/place/${country}`, '_blank');
+  execute: async context => {
+    window.open(`https://www.google.com/maps/place/${context.country}`, '_blank');
   },
 });
 
@@ -100,11 +104,8 @@ function EditUserModal({
 }
 
 export const createEditUserAction = (getOpenModal: () => Promise<OverlayStart['openModal']>) =>
-  createAction<{
-    user: User;
-    update: (user: User) => void;
-  }>({
-    type: EDIT_USER_ACTION,
+  createAction<typeof ACTION_EDIT_USER>({
+    type: ACTION_EDIT_USER,
     getIconType: () => 'pencil',
     getDisplayName: () => 'Edit user',
     execute: async ({ user, update }) => {
@@ -120,8 +121,8 @@ export interface UserContext {
 }
 
 export const createPhoneUserAction = (getUiActionsApi: () => Promise<UiActionsStart>) =>
-  createAction<UserContext>({
-    type: PHONE_USER_ACTION,
+  createAction<typeof ACTION_PHONE_USER>({
+    type: ACTION_PHONE_USER,
     getDisplayName: () => 'Call phone number',
     isCompatible: async ({ user }) => user.phone !== undefined,
     execute: async ({ user }) => {
@@ -133,7 +134,7 @@ export const createPhoneUserAction = (getUiActionsApi: () => Promise<UiActionsSt
       // TODO: we need to figure out the best way to handle these nested actions however, since
       // we don't want multiple context menu's to pop up.
       if (user.phone !== undefined) {
-        (await getUiActionsApi()).executeTriggerActions(PHONE_TRIGGER, user.phone);
+        (await getUiActionsApi()).executeTriggerActions(PHONE_TRIGGER, { phone: user.phone });
       }
     },
   });
diff --git a/examples/ui_actions_explorer/public/app.tsx b/examples/ui_actions_explorer/public/app.tsx
@@ -35,7 +35,7 @@ import { EuiModalBody } from '@elastic/eui';
 import { toMountPoint } from '../../../src/plugins/kibana_react/public';
 import { UiActionsStart, createAction } from '../../../src/plugins/ui_actions/public';
 import { AppMountParameters, OverlayStart } from '../../../src/core/public';
-import { HELLO_WORLD_TRIGGER_ID, HELLO_WORLD_ACTION_TYPE } from '../../ui_action_examples/public';
+import { HELLO_WORLD_TRIGGER_ID, ACTION_HELLO_WORLD } from '../../ui_action_examples/public';
 import { TriggerContextExample } from './trigger_context_example';
 
 interface Props {
@@ -60,7 +60,7 @@ const ActionsExplorer = ({ uiActionsApi, openModal }: Props) => {
             </EuiText>
             <EuiButton
               data-test-subj="emitHelloWorldTrigger"
-              onClick={() => uiActionsApi.executeTriggerActions(HELLO_WORLD_TRIGGER_ID, undefined)}
+              onClick={() => uiActionsApi.executeTriggerActions(HELLO_WORLD_TRIGGER_ID, {})}
             >
               Say hello world!
             </EuiButton>
@@ -76,8 +76,9 @@ const ActionsExplorer = ({ uiActionsApi, openModal }: Props) => {
               <EuiButton
                 data-test-subj="addDynamicAction"
                 onClick={() => {
-                  const dynamicAction = createAction<{}>({
-                    type: `${HELLO_WORLD_ACTION_TYPE}-${name}`,
+                  const dynamicAction = createAction<typeof ACTION_HELLO_WORLD>({
+                    id: `${ACTION_HELLO_WORLD}-${name}`,
+                    type: ACTION_HELLO_WORLD,
                     getDisplayName: () => `Say hello to ${name}`,
                     execute: async () => {
                       const overlay = openModal(
@@ -95,7 +96,7 @@ const ActionsExplorer = ({ uiActionsApi, openModal }: Props) => {
                     },
                   });
                   uiActionsApi.registerAction(dynamicAction);
-                  uiActionsApi.attachAction(HELLO_WORLD_TRIGGER_ID, dynamicAction.type);
+                  uiActionsApi.attachAction(HELLO_WORLD_TRIGGER_ID, dynamicAction);
                   setConfirmationText(
                     `You've successfully added a new action: ${dynamicAction.getDisplayName(
                       {}

diff --git a/examples/ui_actions_explorer/public/plugin.tsx b/examples/ui_actions_explorer/public/plugin.tsx
@@ -27,17 +27,17 @@ import {
   lookUpWeatherAction,
   viewInMapsAction,
   createEditUserAction,
-  CALL_PHONE_NUMBER_ACTION,
-  VIEW_IN_MAPS_ACTION,
-  TRAVEL_GUIDE_ACTION,
-  PHONE_USER_ACTION,
-  EDIT_USER_ACTION,
   makePhoneCallAction,
   showcasePluggability,
-  SHOWCASE_PLUGGABILITY_ACTION,
   UserContext,
   CountryContext,
   PhoneContext,
+  ACTION_EDIT_USER,
+  ACTION_SHOWCASE_PLUGGABILITY,
+  ACTION_CALL_PHONE_NUMBER,
+  ACTION_TRAVEL_GUIDE,
+  ACTION_VIEW_IN_MAPS,
+  ACTION_PHONE_USER,
 } from './actions/actions';
 
 interface StartDeps {
@@ -54,6 +54,15 @@ declare module '../../../src/plugins/ui_actions/public' {
     [COUNTRY_TRIGGER]: CountryContext;
     [PHONE_TRIGGER]: PhoneContext;
   }
+
+  export interface ActionContextMapping {
+    [ACTION_EDIT_USER]: UserContext;
+    [ACTION_SHOWCASE_PLUGGABILITY]: {};
+    [ACTION_CALL_PHONE_NUMBER]: PhoneContext;
+    [ACTION_TRAVEL_GUIDE]: CountryContext;
+    [ACTION_VIEW_IN_MAPS]: CountryContext;
+    [ACTION_PHONE_USER]: UserContext;
+  }
 }
 
 export class UiActionsExplorerPlugin implements Plugin<void, void, {}, StartDeps> {
@@ -67,29 +76,24 @@ export class UiActionsExplorerPlugin implements Plugin<void, void, {}, StartDeps
     deps.uiActions.registerTrigger({
       id: USER_TRIGGER,
     });
-    deps.uiActions.registerAction(lookUpWeatherAction);
-    deps.uiActions.registerAction(viewInMapsAction);
-    deps.uiActions.registerAction(makePhoneCallAction);
-    deps.uiActions.registerAction(showcasePluggability);
 
     const startServices = core.getStartServices();
-    deps.uiActions.registerAction(
+
+    deps.uiActions.attachAction(
+      USER_TRIGGER,
       createPhoneUserAction(async () => (await startServices)[1].uiActions)
     );
-    deps.uiActions.registerAction(
+    deps.uiActions.attachAction(
+      USER_TRIGGER,
       createEditUserAction(async () => (await startServices)[0].overlays.openModal)
     );
-    deps.uiActions.attachAction(USER_TRIGGER, PHONE_USER_ACTION);
-    deps.uiActions.attachAction(USER_TRIGGER, EDIT_USER_ACTION);
 
-    // What's missing here is type analysis to ensure the context emitted by the trigger
-    // is the same context that the action requires.
-    deps.uiActions.attachAction(COUNTRY_TRIGGER, VIEW_IN_MAPS_ACTION);
-    deps.uiActions.attachAction(COUNTRY_TRIGGER, TRAVEL_GUIDE_ACTION);
-    deps.uiActions.attachAction(COUNTRY_TRIGGER, SHOWCASE_PLUGGABILITY_ACTION);
-    deps.uiActions.attachAction(PHONE_TRIGGER, CALL_PHONE_NUMBER_ACTION);
-    deps.uiActions.attachAction(PHONE_TRIGGER, SHOWCASE_PLUGGABILITY_ACTION);
-    deps.uiActions.attachAction(USER_TRIGGER, SHOWCASE_PLUGGABILITY_ACTION);
+    deps.uiActions.attachAction(COUNTRY_TRIGGER, viewInMapsAction);
+    deps.uiActions.attachAction(COUNTRY_TRIGGER, lookUpWeatherAction);
+    deps.uiActions.attachAction(COUNTRY_TRIGGER, showcasePluggability);
+    deps.uiActions.attachAction(PHONE_TRIGGER, makePhoneCallAction);
+    deps.uiActions.attachAction(PHONE_TRIGGER, showcasePluggability);
+    deps.uiActions.attachAction(USER_TRIGGER, showcasePluggability);
 
     core.application.register({
       id: 'uiActionsExplorer',

diff --git a/examples/ui_actions_explorer/public/trigger_context_example.tsx b/examples/ui_actions_explorer/public/trigger_context_example.tsx
@@ -47,7 +47,7 @@ const createRowData = (
     <Fragment>
       <EuiButtonEmpty
         onClick={() => {
-          uiActionsApi.executeTriggerActions(COUNTRY_TRIGGER, user.countryOfResidence);
+          uiActionsApi.executeTriggerActions(COUNTRY_TRIGGER, { country: user.countryOfResidence });
         }}
       >
         {user.countryOfResidence}
@@ -59,7 +59,7 @@ const createRowData = (
       <EuiButtonEmpty
         disabled={user.phone === undefined}
         onClick={() => {
-          uiActionsApi.executeTriggerActions(PHONE_TRIGGER, user.phone!);
+          uiActionsApi.executeTriggerActions(PHONE_TRIGGER, { phone: user.phone! });
         }}
       >
         {user.phone}

diff --git a/src/legacy/core_plugins/data/public/actions/select_range_action.ts b/src/legacy/core_plugins/data/public/actions/select_range_action.ts
@@ -19,21 +19,21 @@
 
 import { i18n } from '@kbn/i18n';
 import {
-  Action,
   createAction,
   IncompatibleActionError,
+  ActionByType,
 } from '../../../../../plugins/ui_actions/public';
 import { onBrushEvent } from './filters/brush_event';
 import { FilterManager, TimefilterContract, esFilters } from '../../../../../plugins/data/public';
 
-export const SELECT_RANGE_ACTION = 'SELECT_RANGE_ACTION';
+export const ACTION_SELECT_RANGE = 'ACTION_SELECT_RANGE';
 
-interface ActionContext {
+export interface SelectRangeActionContext {
   data: any;
   timeFieldName: string;
 }
 
-async function isCompatible(context: ActionContext) {
+async function isCompatible(context: SelectRangeActionContext) {
   try {
     return Boolean(await onBrushEvent(context.data));
   } catch {
@@ -44,17 +44,17 @@ async function isCompatible(context: ActionContext) {
 export function selectRangeAction(
   filterManager: FilterManager,
   timeFilter: TimefilterContract
-): Action<ActionContext> {
-  return createAction<ActionContext>({
-    type: SELECT_RANGE_ACTION,
-    id: SELECT_RANGE_ACTION,
+): ActionByType<typeof ACTION_SELECT_RANGE> {
+  return createAction<typeof ACTION_SELECT_RANGE>({
+    type: ACTION_SELECT_RANGE,
+    id: ACTION_SELECT_RANGE,
     getDisplayName: () => {
       return i18n.translate('data.filter.applyFilterActionTitle', {
         defaultMessage: 'Apply filter to current view',
       });
     },
     isCompatible,
-    execute: async ({ timeFieldName, data }: ActionContext) => {
+    execute: async ({ timeFieldName, data }: SelectRangeActionContext) => {
       if (!(await isCompatible({ timeFieldName, data }))) {
         throw new IncompatibleActionError();
       }

diff --git a/src/legacy/core_plugins/data/public/actions/value_click_action.ts b/src/legacy/core_plugins/data/public/actions/value_click_action.ts
@@ -20,7 +20,7 @@
 import { i18n } from '@kbn/i18n';
 import { toMountPoint } from '../../../../../plugins/kibana_react/public';
 import {
-  Action,
+  ActionByType,
   createAction,
   IncompatibleActionError,
 } from '../../../../../plugins/ui_actions/public';
@@ -37,14 +37,14 @@ import {
   esFilters,
 } from '../../../../../plugins/data/public';
 
-export const VALUE_CLICK_ACTION = 'VALUE_CLICK_ACTION';
+export const ACTION_VALUE_CLICK = 'ACTION_VALUE_CLICK';
 
-interface ActionContext {
+export interface ValueClickActionContext {
   data: any;
   timeFieldName: string;
 }
 
-async function isCompatible(context: ActionContext) {
+async function isCompatible(context: ValueClickActionContext) {
   try {
     const filters: Filter[] =
       (await createFiltersFromEvent(context.data.data || [context.data], context.data.negate)) ||
@@ -58,17 +58,17 @@ async function isCompatible(context: ActionContext) {
 export function valueClickAction(
   filterManager: FilterManager,
   timeFilter: TimefilterContract
-): Action<ActionContext> {
-  return createAction<ActionContext>({
-    type: VALUE_CLICK_ACTION,
-    id: VALUE_CLICK_ACTION,
+): ActionByType<typeof ACTION_VALUE_CLICK> {
+  return createAction<typeof ACTION_VALUE_CLICK>({
+    type: ACTION_VALUE_CLICK,
+    id: ACTION_VALUE_CLICK,
     getDisplayName: () => {
       return i18n.translate('data.filter.applyFilterActionTitle', {
         defaultMessage: 'Apply filter to current view',
       });
     },
     isCompatible,
-    execute: async ({ timeFieldName, data }: ActionContext) => {
+    execute: async ({ timeFieldName, data }: ValueClickActionContext) => {
       if (!(await isCompatible({ timeFieldName, data }))) {
         throw new IncompatibleActionError();
       }