Improve visualization typings (#79128) (#79636)

* Improve visualization typings * Fix vis selection dialog * Fix broken getInfoMessage type Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Oct 6, 2020 · e754fb7 · e754fb7
1 parent d851039
commit e754fb7
Show file tree

Hide file tree

Showing 25 changed files with 696 additions and 196 deletions.
diff --git a/src/plugins/tile_map/public/components/tile_map_options.tsx b/src/plugins/tile_map/public/components/tile_map_options.tsx
@@ -76,7 +76,7 @@ function TileMapOptions(props: TileMapOptionsProps) {
         <BasicOptions {...props} />
 
         <SwitchOption
-          disabled={!vis.type.visConfig.canDesaturate}
+          disabled={!vis.type.visConfig?.canDesaturate}
           label={i18n.translate('tileMap.visParams.desaturateTilesLabel', {
             defaultMessage: 'Desaturate tiles',
           })}

diff --git a/src/plugins/tile_map/public/tile_map_type.js b/src/plugins/tile_map/public/tile_map_type.js
@@ -55,7 +55,6 @@ export function createTileMapTypeDefinition(dependencies) {
         wms: uiSettings.get('visualization:tileMap:WMSdefaults'),
       },
     },
-    requiresPartialRows: true,
     visualization: CoordinateMapsVisualization,
     responseHandler: convertToGeoJson,
     editorConfig: {

diff --git a/src/plugins/vis_default_editor/public/components/sidebar/sidebar.tsx b/src/plugins/vis_default_editor/public/components/sidebar/sidebar.tsx
@@ -18,7 +18,7 @@
  */
 
 import React, { useMemo, useState, useCallback, KeyboardEventHandler, useEffect } from 'react';
-import { get, isEqual } from 'lodash';
+import { isEqual } from 'lodash';
 import { i18n } from '@kbn/i18n';
 import { keys, EuiButtonIcon, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
 import { EventEmitter } from 'events';
@@ -71,7 +71,7 @@ function DefaultEditorSideBar({
   ]);
   const metricSchemas = (vis.type.schemas.metrics || []).map((s: Schema) => s.name);
   const metricAggs = useMemo(
-    () => responseAggs.filter((agg) => metricSchemas.includes(get(agg, 'schema'))),
+    () => responseAggs.filter((agg) => agg.schema && metricSchemas.includes(agg.schema)),
     [responseAggs, metricSchemas]
   );
   const hasHistogramAgg = useMemo(() => responseAggs.some((agg) => agg.type.name === 'histogram'), [

diff --git a/src/plugins/vis_type_metric/public/to_ast.ts b/src/plugins/vis_type_metric/public/to_ast.ts
@@ -39,7 +39,7 @@ export const toExpressionAst = (vis: Vis, params: any) => {
   const esaggs = buildExpressionFunction<EsaggsExpressionFunctionDefinition>('esaggs', {
     index: vis.data.indexPattern!.id!,
     metricsAtAllLevels: vis.isHierarchical(),
-    partialRows: vis.type.requiresPartialRows || vis.params.showPartialRows || false,
+    partialRows: vis.params.showPartialRows || false,
     aggConfigs: JSON.stringify(vis.data.aggs!.aggs),
     includeFormatHints: false,
   });

diff --git a/src/plugins/vis_type_table/public/table_vis_controller.test.ts b/src/plugins/vis_type_table/public/table_vis_controller.test.ts
@@ -249,13 +249,13 @@ describe('Table Vis - Controller', () => {
     const vis = getRangeVis({ showPartialRows: true });
     initController(vis);
 
-    expect(vis.type.hierarchicalData(vis)).toEqual(true);
+    expect((vis.type.hierarchicalData as Function)(vis)).toEqual(true);
   });
 
   test('passes partialRows:false to tabify based on the vis params', () => {
     const vis = getRangeVis({ showPartialRows: false });
     initController(vis);
 
-    expect(vis.type.hierarchicalData(vis)).toEqual(false);
+    expect((vis.type.hierarchicalData as Function)(vis)).toEqual(false);
   });
 });
diff --git a/src/plugins/vis_type_table/public/table_vis_type.ts b/src/plugins/vis_type_table/public/table_vis_type.ts
@@ -20,7 +20,7 @@ import { CoreSetup, PluginInitializerContext } from 'kibana/public';
 import { i18n } from '@kbn/i18n';
 import { AggGroupNames } from '../../data/public';
 import { Schemas } from '../../vis_default_editor/public';
-import { BaseVisTypeOptions, Vis } from '../../visualizations/public';
+import { BaseVisTypeOptions } from '../../visualizations/public';
 import { tableVisResponseHandler } from './table_vis_response_handler';
 // @ts-ignore
 import tableVisTemplate from './table_vis.html';
@@ -99,7 +99,7 @@ export function getTableVisTypeDefinition(
       ]),
     },
     responseHandler: tableVisResponseHandler,
-    hierarchicalData: (vis: Vis) => {
+    hierarchicalData: (vis) => {
       return Boolean(vis.params.showPartialRows || vis.params.showMetricsAtAllLevels);
     },
   };

diff --git a/src/plugins/vis_type_table/public/vis_controller.ts b/src/plugins/vis_type_table/public/vis_controller.ts
@@ -103,7 +103,9 @@ export function getTableVisualizationControllerClass(
           this.$scope = this.$rootScope.$new();
           this.$scope.uiState = this.vis.getUiState();
           updateScope();
-          this.el.find('div').append(this.$compile(this.vis.type!.visConfig.template)(this.$scope));
+          this.el
+            .find('div')
+            .append(this.$compile(this.vis.type.visConfig?.template ?? '')(this.$scope));
           this.$scope.$apply();
         } else {
           updateScope();

diff --git a/src/plugins/vis_type_tagcloud/public/to_ast.ts b/src/plugins/vis_type_tagcloud/public/to_ast.ts
@@ -38,7 +38,7 @@ export const toExpressionAst = (vis: Vis<TagCloudVisParams>, params: BuildPipeli
   const esaggs = buildExpressionFunction<EsaggsExpressionFunctionDefinition>('esaggs', {
     index: vis.data.indexPattern!.id!,
     metricsAtAllLevels: vis.isHierarchical(),
-    partialRows: vis.type.requiresPartialRows || false,
+    partialRows: false,
     aggConfigs: JSON.stringify(vis.data.aggs!.aggs),
     includeFormatHints: false,
   });

diff --git a/src/plugins/vis_type_vega/public/vega_type.ts b/src/plugins/vis_type_vega/public/vega_type.ts
@@ -18,6 +18,7 @@
  */
 
 import { i18n } from '@kbn/i18n';
+import { BaseVisTypeOptions } from 'src/plugins/visualizations/public';
 import { DefaultEditorSize } from '../../vis_default_editor/public';
 import { VegaVisualizationDependencies } from './plugin';
 import { VegaVisEditor } from './components';
@@ -31,7 +32,9 @@ import { VIS_EVENT_TO_TRIGGER } from '../../visualizations/public';
 
 import { getInfoMessage } from './components/experimental_map_vis_info';
 
-export const createVegaTypeDefinition = (dependencies: VegaVisualizationDependencies) => {
+export const createVegaTypeDefinition = (
+  dependencies: VegaVisualizationDependencies
+): BaseVisTypeOptions => {
   const requestHandler = createVegaRequestHandler(dependencies);
   const visualization = createVegaVisualization(dependencies);
 

diff --git a/src/plugins/visualizations/public/components/visualization_chart.tsx b/src/plugins/visualizations/public/components/visualization_chart.tsx
@@ -90,6 +90,12 @@ class VisualizationChart extends React.Component<VisualizationChartProps> {
     const { vis } = this.props;
     const Visualization = vis.type.visualization;
 
+    if (!Visualization) {
+      throw new Error(
+        'Tried to use VisualizationChart component with a vis without visualization property.'
+      );
+    }
+
     this.visualization = new Visualization(this.chartDiv.current, vis);
 
     // We know that containerDiv.current will never be null, since we will always

diff --git a/src/plugins/visualizations/public/expressions/vis.ts b/src/plugins/visualizations/public/expressions/vis.ts
@@ -36,7 +36,7 @@ import { VisType } from '../vis_types';
 
 export interface ExprVisState {
   title?: string;
-  type: VisType | string;
+  type: VisType<unknown> | string;
   params?: VisParams;
 }
 
@@ -52,7 +52,7 @@ export interface ExprVisAPI {
 
 export class ExprVis extends EventEmitter {
   public title: string = '';
-  public type: VisType;
+  public type: VisType<unknown>;
   public params: VisParams = {};
   public sessionState: Record<string, any> = {};
   public API: ExprVisAPI;
@@ -92,7 +92,7 @@ export class ExprVis extends EventEmitter {
     };
   }
 
-  private getType(type: string | VisType) {
+  private getType(type: string | VisType<unknown>) {
     if (_.isString(type)) {
       const newType = getTypes().get(type);
       if (!newType) {

diff --git a/src/plugins/visualizations/public/legacy/build_pipeline.ts b/src/plugins/visualizations/public/legacy/build_pipeline.ts
@@ -86,7 +86,10 @@ const vislibCharts: string[] = [
   'line',
 ];
 
-export const getSchemas = (vis: Vis, { timeRange, timefilter }: BuildPipelineParams): Schemas => {
+export const getSchemas = <TVisParams>(
+  vis: Vis<TVisParams>,
+  { timeRange, timefilter }: BuildPipelineParams
+): Schemas => {
   const createSchemaConfig = (accessor: number, agg: IAggConfig): SchemaConfig => {
     if (isDateHistogramBucketAggConfig(agg)) {
       agg.params.timeRange = timeRange;
@@ -155,7 +158,8 @@ export const getSchemas = (vis: Vis, { timeRange, timefilter }: BuildPipelinePar
       }
     }
     if (schemaName === 'split') {
-      schemaName = `split_${vis.params.row ? 'row' : 'column'}`;
+      // TODO: We should check if there's a better way then casting to `any` here
+      schemaName = `split_${(vis.params as any).row ? 'row' : 'column'}`;
       skipMetrics = responseAggs.length - metrics.length > 1;
     }
     if (!schemas[schemaName]) {
@@ -410,7 +414,7 @@ export const buildPipeline = async (vis: Vis, params: BuildPipelineParams) => {
       pipeline += `esaggs
     ${prepareString('index', indexPattern!.id)}
     metricsAtAllLevels=${vis.isHierarchical()}
-    partialRows=${vis.type.requiresPartialRows || vis.params.showPartialRows || false}
+    partialRows=${vis.params.showPartialRows || false}
     ${prepareJson('aggConfigs', vis.data.aggs!.aggs)} | `;
     }
 
@@ -433,7 +437,7 @@ export const buildPipeline = async (vis: Vis, params: BuildPipelineParams) => {
       pipeline += `visualization type='${vis.type.name}'
     ${prepareJson('visConfig', visConfig)}
     metricsAtAllLevels=${vis.isHierarchical()}
-    partialRows=${vis.type.requiresPartialRows || vis.params.showPartialRows || false} `;
+    partialRows=${vis.params.showPartialRows || false} `;
       if (indexPattern) {
         pipeline += `${prepareString('index', indexPattern.id)} `;
         if (vis.data.aggs) {

diff --git a/src/plugins/visualizations/public/vis.test.ts b/src/plugins/visualizations/public/vis.test.ts
@@ -121,7 +121,7 @@ describe('Vis Class', function () {
     });
 
     it('should return true for hierarchical vis (like pie)', function () {
-      vis.type.hierarchicalData = true;
+      (vis.type as any).hierarchicalData = true;
       expect(vis.isHierarchical()).toBe(true);
     });
   });

diff --git a/src/plugins/visualizations/public/vis.ts b/src/plugins/visualizations/public/vis.ts
@@ -84,7 +84,7 @@ const getSearchSource = async (inputSearchSource: ISearchSource, savedSearchId?:
 type PartialVisState = Assign<SerializedVis, { data: Partial<SerializedVisData> }>;
 
 export class Vis<TVisParams = VisParams> {
-  public readonly type: VisType;
+  public readonly type: VisType<TVisParams>;
   public readonly id?: string;
   public title: string = '';
   public description: string = '';
@@ -97,14 +97,14 @@ export class Vis<TVisParams = VisParams> {
   public readonly uiState: PersistedState;
 
   constructor(visType: string, visState: SerializedVis = {} as any) {
-    this.type = this.getType(visType);
+    this.type = this.getType<TVisParams>(visType);
     this.params = this.getParams(visState.params);
     this.uiState = new PersistedState(visState.uiState);
     this.id = visState.id;
   }
 
-  private getType(visType: string) {
-    const type = getTypes().get(visType);
+  private getType<TVisParams>(visType: string) {
+    const type = getTypes().get<TVisParams>(visType);
     if (!type) {
       const errorMessage = i18n.translate('visualizations.visualizationTypeInvalidMessage', {
         defaultMessage: 'Invalid visualization type "{visType}"',
@@ -118,7 +118,7 @@ export class Vis<TVisParams = VisParams> {
   }
 
   private getParams(params: VisParams) {
-    return defaults({}, cloneDeep(params || {}), cloneDeep(this.type.visConfig.defaults || {}));
+    return defaults({}, cloneDeep(params ?? {}), cloneDeep(this.type.visConfig?.defaults ?? {}));
   }
 
   async setState(state: PartialVisState) {
@@ -202,10 +202,6 @@ export class Vis<TVisParams = VisParams> {
     };
   }
 
-  toExpressionAst() {
-    return this.type.toExpressionAst(this.params);
-  }
-
   // deprecated
   isHierarchical() {
     if (isFunction(this.type.hierarchicalData)) {