[visualizations] Rewrite expression pipeline builder to use AST builder #65396
Conversation
…xpressions plugin.
botelastic
bot
added
the
Feature:ExpressionLanguage
| return expr; | ||
| const visConfig = buildVisConfig.tagcloud(schemas); | ||
|
|
||
| const visdimensionMetric = visConfig.metric |
There was a problem hiding this comment.
metric needs to be preent for tagcloud, so i don't think we need to check for it
| } | ||
| }; | ||
|
|
||
| export const prepareDimension = (variable: string, data: any) => { |
There was a problem hiding this comment.
i think this utility function is still useful to prevent code duplication across all vis types
There was a problem hiding this comment.
lets just update it to return expression function or undefined
| }) | ||
| : undefined; | ||
|
|
||
| const visdimensionBucketFn = |
There was a problem hiding this comment.
a lot of code duplication, the function we had makes sense still
| } | ||
|
|
||
| if (savedSearchId) { | ||
| kibanaContext.addArgument('savedSearchId', savedSearchId); |
There was a problem hiding this comment.
kibanaContext is not added to pipeline
| }) | ||
| ); | ||
| } else if (vis.type.toExpression) { | ||
| pipeline.functions.push(await vis.type.toExpression(vis, params)); |
There was a problem hiding this comment.
we might want to return expression from toExpression, so chart can optionally return more than one function, so we should concat the chains
| pipeline.functions.push(buildExpressionFunction(fnName, args)) | ||
| ); | ||
| } else if (vislibCharts.includes(vis.type.name)) { | ||
| const visConfig = { ...vis.params }; |
There was a problem hiding this comment.
this needs to be moved to vislib charts toExpression function
There was a problem hiding this comment.
(can be done in followup, but lets open an issue for it)
| @@ -258,49 +227,49 @@ const adjustVislibDimensionFormmaters = (vis: Vis, dimensions: { y: any[] }): vo | |||
| }; | |||
|
|
|||
| export const buildPipelineVisFunction: BuildPipelineVisFunction = { | |||
| vega: params => { | |||
| return `vega ${prepareString('spec', params.spec)}`; | |||
| vega: ({ spec }) => { | |||
There was a problem hiding this comment.
all of these should move to specific charts toExpression function, can be done in followup
|
|
||
| // request handler | ||
| if (vis.type.requestHandler === 'courier' && indexPattern?.id) { | ||
| pipeline.functions.push( |
There was a problem hiding this comment.
just a note .... once esaggs function is refactored we'll be able to do pipeline.functions.push(vis.data.aggs.toExpressionAST())
| } else if (vis.type.toExpression) { | ||
| pipeline.functions.push(await vis.type.toExpression(vis, params)); | ||
| } else { | ||
| const visConfig = { ...vis.params }; |
There was a problem hiding this comment.
this case should be removed, same can be achieved by visualizations implementing toExpression function
|
Closing as this is very outdated. As noted in #51246, the app team will revisit whether this effort is even necessary after they have implemented renderers for each vis type. |
WIP
Closes #51246, #61829
Remaining tasks:
buildPipeline& corresponding dependenciesbuildPipelineVisFunctionbuildVisConfigtoExpressionAstto vis object