[Expressions Plugin] Allow render definitions with extended IInterpreterRenderHandlers

[crob611]

Canvas was trying to type all of our render definitions, but we have some extra handlers that we define to extend IInterpreterRenderHandlers.

Defining our renderers with a render function with the handlers typed to our HandlerType causes a type error when trying to add to the registry due to those handlers being mismatched with the IInterpreterRenderHandlers.

Is there a way to relax the type on ExpressionRenderDefinition to allow additional handlers? Or is there a different way we should be going about adding additional handlers?

[elasticmachine]

Pinging @elastic/kibana-app-arch (Team:AppArch)

[lukeelmers]

We could consider adding an optional type parameter to ExpressionRenderDefinition to take in custom render handlers:

interface MyHandlers extends IInterpreterRenderHandlers {
  hello: () => void;
}

const myRenderer: ExpressionRenderDefinition<Config, MyHandlers> = {};

The generic ExpressionRenderDefinition would enforce that the provided handlers extend IInterpreterRenderHandlers.

Would that work for you @crob611? AFAICT this shouldn't be a problem, but would need to test it.

[crob611]

I experimented with that. The problem I hit then was at the ExpressionRenderRegistry level. register function is expecting a definition of type AnyExpressionRenderDefinition, so hits the same problem. If you don't give it a generic and it defaults to IInterpreterRenderHandlers then the Canvas Handlers don't match the expected signature.

I didn't dig much after that and just bailed things on our end and explicitly casted our stuff to AnyExpressionDefinition.

I'm starting to think maybe handlers isn't the correct place for us to add additional handler functionality?

[lukeelmers]

Ah, I had missed that. So in that case we'd also need to make AnyExpressionRenderDefinition be ExpressionRenderDefinition<any, any>. This would be consistent with how we have handled the typings for registering functions & types.

[lukeelmers]

I'm starting to think maybe handlers isn't the correct place for us to add additional handler functionality?

Yeah, @ppisljar and I talked about this a bit and I believe he's planning to follow up with some thoughts on how to address what you are looking to do.

[crob611]

Just for reference: We create the handlers that we pass into the render functions in create_handlers.ts We just have empty stubs for most of the methods defined by IInterpreterRegisterHandlers

We pass them through to the render function in render_with_fn

[ppisljar]

The reason we didn't allow extending the handlers definition is that we need a fixed (agreed upon) set of handlers that every renderer consumes as else the renderers are not usable outside of specific context. If we don't know what handlers the specific renderer requires its impossible to consume it.

I opened an issue for addressing the current extra handlers that canvas is passing into expressions here: #74644

i think we should try to address that as soon as possible. I think reusability is one of the main advantages of expressions and we should really try hard to make all functions and renderers reusable.