elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.76k stars 8.16k forks source link

[Response Ops] `ActionType` Typing #127152

Open ymao1 opened 2 years ago

ymao1 commented 2 years ago

With the latest Typescript version bump PR,

in x-pack/plugins/actions/server/action_type_registry.ts

this.actionTypes.set(actionType.id, { ...actionType } as ActionType);

caused this Typescript error:

proc [tsc] x-pack/plugins/actions/server/action_type_registry.ts:125:41 - error TS2678: Type 'ExecutorResultData' is not comparable to type 'void'. 
proc [tsc] 
proc [tsc] 125 this.actionTypes.set(actionType.id, { ...actionType } as ActionType); proc [tsc] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 
proc [tsc] 
proc [tsc] x-pack/plugins/actions/server/action_type_registry.ts:144:39 - error TS2678: Type 'ExecutorResultData' is not comparable to type 'void'. 
proc [tsc] 
proc [tsc] 144 getActionTypeFeatureUsageName(actionType as ActionType), 
proc [tsc] ~~~~~~~~~~~~~~~~~~~~~~~~ 
proc [tsc]

Temporary workaround was to do this:

this.actionTypes.set(actionType.id, { ...actionType } as unknown as ActionType);

but we should fix it for real

elasticmachine commented 2 years ago

Pinging @elastic/response-ops (Team:ResponseOps)

spalger commented 2 years ago

It seems like more APIs should accept some sort of T extends ActionType<any, any, any, any> rather than having four generic parameters everywhere with defaults assigned... That being said there are tons of APIs and my knowledge about why this direction was chosen is basically zero. Normally when I design APIs like this I do something like this:

type AnyActionType = ActionType<any, any, any, any>
type ActionTypeSecrets<T extends AnyActionType> = T extends ActionType<any, infer X, any, any> ? X : never

and then extract the Secrets type as necessary from the single generic, in a method like:

async getSecrets<A extends AnyActionType>(type: A): Promise<ActionTypeSecrets<A>> {
  ...
}