Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: updates to fix ts errors in v4.9.5 upgrade #176114

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ x-pack/plugins/lens/public/datatable_visualization/expression.tsx kibana-app
x-pack/plugins/lens/public/datatable_visualization/index.ts kibana-app
x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx kibana-app
x-pack/plugins/lens/public/datatable_visualization/visualization.tsx kibana-app
x-pack/plugins/lens/public/debounced_component/debounced_component.test.tsx kibana-app
x-pack/plugins/lens/public/debounced_component/debounced_component.tsx kibana-app
x-pack/plugins/lens/public/debounced_component/index.ts kibana-app
x-pack/plugins/lens/public/drag_drop/drag_drop.test.tsx kibana-app
x-pack/plugins/lens/public/drag_drop/drag_drop.tsx kibana-app
x-pack/plugins/lens/public/drag_drop/index.ts kibana-app
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import React from 'react';
import { storiesOf } from '@storybook/react';
import { from } from 'rxjs';
import { ExpressionValueVisDimension } from '@kbn/visualizations-plugin/common';
import { Datatable, DatatableColumn } from '@kbn/expressions-plugin/common';
import { Datatable, DatatableColumn, TextAlignment } from '@kbn/expressions-plugin/common';
import { Render } from '@kbn/presentation-util-plugin/public/__stories__';
import { ColorMode, CustomPaletteState } from '@kbn/charts-plugin/common';
import { getFormatService } from '../__mocks__/format_service';
Expand Down Expand Up @@ -209,8 +209,11 @@ storiesOf('renderers/visMetric', module)
},
labels: {
show: false,
// @ts-expect-error upgrade typescript v4.9.5
style: { spec: { fontSize: '60px', align: 'left' }, type: 'style', css: '' },
style: {
spec: { fontSize: '60px', textAlign: TextAlignment.LEFT },
type: 'style',
css: '',
},
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
position: LabelPosition.TOP,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,13 @@ function hasAutoscaleProps<T>(props: T): props is T & AutoScaleProps {
return false;
}

function getWrappedComponentProps<T>(props: T) {
function getWrappedComponentProps<T>(props: T): T {
if (hasAutoscaleProps(props)) {
const { autoScaleParams, renderComplete, ...rest } = props;
return rest;
return {
...props,
autoScaleParams: undefined,
renderComplete: undefined,
};
}
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved

return props;
Expand Down Expand Up @@ -132,8 +135,7 @@ export function withAutoScale<T>(WrappedComponent: ComponentType<T>) {
: {}),
}}
>
{/* @ts-expect-error upgrade typescript v4.9.5*/}
<WrappedComponent {...(restProps as T)} />
<WrappedComponent {...restProps} />
</div>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,19 @@ import { ExpressionAstExpression } from '../ast';
* `ExecutionContract` is a wrapper around `Execution` class. It provides the
* same functionality but does not expose Expressions plugin internals.
*/
export class ExecutionContract<Input = unknown, Output = unknown, InspectorAdapters = unknown> {
export class ExecutionContract<
Input = unknown,
Output = unknown,
InspectorAdapters extends Adapters = object
> {
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
public get isPending(): boolean {
const { state, result } = this.execution.state.get();
const finished = state === 'error' || (state === 'result' && !result?.partial);
return !finished;
}

// @ts-expect-error upgrade typescript v4.9.5
protected readonly execution: Execution<Input, Output, InspectorAdapters>;

// @ts-expect-error upgrade typescript v4.9.5
constructor(execution: Execution<Input, Output, InspectorAdapters>) {
this.execution = execution;
}
Expand Down Expand Up @@ -82,6 +84,5 @@ export class ExecutionContract<Input = unknown, Output = unknown, InspectorAdapt
* Get Inspector adapters provided to all functions of expression through
* execution context.
*/
// @ts-expect-error upgrade typescript v4.9.5
inspect = (): Adapters => this.execution.inspectorAdapters;
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { FormattedColumn, TableVisConfig, TableVisUiState } from '../types';
import { DatatableColumn } from '@kbn/expressions-plugin/common';
import { createTableVisCell } from './table_vis_cell';
import { createGridColumns } from './table_vis_columns';
import { EuiDataGridProps } from '@elastic/eui';

jest.mock('./table_vis_columns', () => ({
createGridColumns: jest.fn(() => []),
Expand Down Expand Up @@ -107,8 +108,8 @@ describe('TableVisBasic', () => {
undefined
);

// @ts-expect-error upgrade typescript v4.9.5
const { onSort } = comp.find('EuiDataGrid').prop('sorting');
const { onSort } = comp.find('EuiDataGrid').prop<EuiDataGridProps['sorting']>('sorting')!;

// sort the first column
onSort([{ id: 'first', direction: 'asc' }]);
expect(uiStateProps.setSort).toHaveBeenCalledWith({ columnIndex: 0, direction: 'asc' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,20 @@ export const convertToPercentileColumn = (
return null;
}
const commonColumnParams = createColumn(series, metric, field, { reducedTimeRange, timeShift });
const meta: PercentileColumn['meta'] =
index !== undefined
? {
reference: `${metric.id}.${index}`,
...commonColumnParams.meta,
}
: commonColumnParams.meta;

return {
operationType: 'percentile',
sourceField: field.name,
...commonColumnParams,
params: { ...params, ...getFormat(series) },
// @ts-expect-error upgrade typescript v4.9.5
meta:
index !== undefined
? {
reference: `${metric.id}.${index}`,
...commonColumnParams.meta,
}
: commonColumnParams.meta,
meta,
};
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
Column,
PercentileRanksColumnWithExtendedMeta,
CommonColumnConverterArgs,
PercentileColumn,
} from './types';

export const isPercentileRanksColumnWithMeta = (
Expand Down Expand Up @@ -56,19 +57,19 @@ export const convertToPercentileRankColumn = (
}

const commonColumnParams = createColumn(series, metric, field, { reducedTimeRange, timeShift });
const meta: PercentileColumn['meta'] =
index !== undefined
? {
reference: `${metric.id}.${index}`,
...commonColumnParams.meta,
}
: commonColumnParams.meta;
return {
operationType: 'percentile_rank',
sourceField: field.name,
...commonColumnParams,
params: { ...params, ...getFormat(series) },
// @ts-expect-error upgrade typescript v4.9.5
meta:
index !== undefined
? {
reference: `${metric.id}.${index}`,
...commonColumnParams.meta,
}
: commonColumnParams.meta,
meta,
};
};

Expand Down
4 changes: 1 addition & 3 deletions src/plugins/visualizations/public/vis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const getSearchSource = async (inputSearchSource: ISearchSource, savedSearchId?:

type PartialVisState = Assign<SerializedVis, { data: Partial<SerializedVisData> }>;

export class Vis<TVisParams = VisParams> {
export class Vis<TVisParams extends VisParams = VisParams> {
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
public readonly type: BaseVisType<TVisParams>;
public readonly id?: string;
public title: string = '';
Expand All @@ -69,7 +69,6 @@ export class Vis<TVisParams = VisParams> {

constructor(visType: string, visState: SerializedVis<TVisParams> = {} as any) {
this.type = this.getType(visType);
// @ts-expect-error upgrade typescript v4.9.5
this.params = this.getParams(visState.params);
this.uiState = new PersistedState(visState.uiState);
this.id = visState.id;
Expand Down Expand Up @@ -192,7 +191,6 @@ export class Vis<TVisParams = VisParams> {
title: this.title,
description: this.description,
type: this.type.name,
// @ts-expect-error upgrade typescript v4.9.5
params: cloneDeep(this.params),
uiState: this.uiState.toJSON(),
data: {
Expand Down
3 changes: 1 addition & 2 deletions src/plugins/visualizations/public/vis_async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import type { SerializedVis } from './vis';
import type { VisParams } from '../common';

export const createVisAsync = async <TVisParams = VisParams>(
export const createVisAsync = async <TVisParams extends VisParams = VisParams>(
visType: string,
visState: SerializedVis<TVisParams> = {} as any
) => {
Expand All @@ -20,7 +20,6 @@ export const createVisAsync = async <TVisParams = VisParams>(
const { Vis } = await import('./vis');
const vis = new Vis(visType, visState);

// @ts-expect-error upgrade typescript v4.9.5
await vis.setState(visState);
return vis;
};
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export interface ValueFormatConfig {
// Formatting can optionally be added to any column
export interface FormattedIndexPatternColumn extends BaseIndexPatternColumn {
params?: {
window?: number;
format?: ValueFormatConfig;
};
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When defining the moving average definition like here...

moving_average: createOperationDefinitionMock('moving_average', {
input: 'fullReference',
operationParams: [{ name: 'window', type: 'number', required: true }],
filterable: true,
getErrorMessage: jest.fn(() => ['mock error']),
buildColumn: ({ referenceIds }, columnsParams) => ({
label: 'moving_average',
dataType: 'number',
operationType: 'moving_average',
isBucketed: false,
scale: 'ratio',
timeScale: undefined,
params: { window: 5 },
references: referenceIds,
filter: getFilter(undefined, columnsParams),
}),
}),

The type of params: { window: 5 } is pointing to this type here thus why adding the window prop fixes the error.

But I think window type should be pulling from MovingAverageIndexPatternColumn instead which is here...

export type MovingAverageIndexPatternColumn = FormattedIndexPatternColumn &
ReferenceBasedIndexPatternColumn & {
operationType: typeof MOVING_AVERAGE_ID;
params: {
window: number;
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,17 @@ const operationDefinitionMap: Record<string, GenericOperationDefinition> = {
dataType: type === 'string' ? type : 'number',
})),
}),
max: createOperationDefinitionMock('max'),
max: createOperationDefinitionMock('max', {}),
count: createOperationDefinitionMock('count', {
filterable: true,
canReduceTimeRange: true,
}),
derivative: createOperationDefinitionMock('derivative', { input: 'fullReference' }),
moving_average: createOperationDefinitionMock('moving_average', {
// @ts-expect-error upgrade typescript v4.9.5
input: 'fullReference',
operationParams: [{ name: 'window', type: 'number', required: true }],
filterable: true,
getErrorMessage: jest.fn(() => ['mock error']),
// @ts-expect-error upgrade typescript v4.9.5
buildColumn: ({ referenceIds }, columnsParams) => ({
label: 'moving_average',
dataType: 'number',
Expand Down
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These types are a little crazy but they are mocks trying to conform to multiple type definitions in a single function.

I reworked the types to pass a generic type based on the input type, with it defaulting to the field type.

This works for almost all cases, haven't checked other files 🤞🏼, except for used with the label: 'moving_average', see next comment 👇🏼 .

Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
* 2.0.
*/

import type { GenericOperationDefinition } from '../..';
import type { GenericIndexPatternColumn } from '../../column_types';
import type {
GenericOperationDefinition,
OperationDefinition,
OperationDefinitionMap,
} from '../..';
import type { BaseIndexPatternColumn, GenericIndexPatternColumn } from '../../column_types';
import { getFilter } from '../../helpers';

interface PartialColumnParams {
Expand All @@ -16,27 +20,22 @@ interface PartialColumnParams {
reducedTimeRange?: string;
}

type OperationByInputType<Input extends GenericOperationDefinition['input']> = Extract<
GenericOperationDefinition,
{ input: Input }
>;

export function createOperationDefinitionMock(
export function createOperationDefinitionMock<
T extends keyof OperationDefinitionMap<BaseIndexPatternColumn>
>(
operation: string,
{
input = 'field',
getErrorMessage,
buildColumn,
...params
}: Partial<GenericOperationDefinition> = {},
column: T extends keyof OperationDefinitionMap<BaseIndexPatternColumn>
? Partial<OperationDefinition<BaseIndexPatternColumn, T>>
: Partial<OperationDefinition<BaseIndexPatternColumn, 'field'>>,
{
label = operation,
dataType = 'number',
isBucketed = false,
scale = 'ratio',
timeScale,
}: Partial<GenericIndexPatternColumn> = {}
): OperationByInputType<typeof input> {
): GenericOperationDefinition {
const { input = 'field', getErrorMessage, buildColumn, ...params } = column;
const sharedColumnParams = {
label,
dataType,
Expand Down Expand Up @@ -67,15 +66,16 @@ export function createOperationDefinitionMock(
onFieldChange: jest.fn(),
toEsAggsFn: jest.fn(),
getPossibleOperationForField:
(params as OperationByInputType<typeof input>).getPossibleOperationForField ??
(params as unknown as OperationDefinition<BaseIndexPatternColumn, 'field'>)
?.getPossibleOperationForField ??
jest.fn((arg) => ({
scale,
dataType,
isBucketed,
})),
...sharedDefinitionParams,
...params,
} as OperationByInputType<typeof input>;
} as OperationDefinition<BaseIndexPatternColumn, 'field'>;
}
if (input === 'fullReference') {
return {
Expand All @@ -102,7 +102,7 @@ export function createOperationDefinitionMock(
selectionStyle: 'field',
...sharedDefinitionParams,
...params,
} as OperationByInputType<typeof input>;
} as OperationDefinition<BaseIndexPatternColumn, 'fullReference'>;
}
return {
buildColumn:
Expand All @@ -116,5 +116,5 @@ export function createOperationDefinitionMock(
createCopy: jest.fn(),
...sharedDefinitionParams,
...params,
} as OperationByInputType<typeof input>;
} as OperationDefinition<BaseIndexPatternColumn, 'none' | 'managedReference'>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ interface ManagedReferenceOperationDefinition<C extends BaseIndexPatternColumn>
selectionStyle?: 'hidden';
}

interface OperationDefinitionMap<C extends BaseIndexPatternColumn, P = {}> {
export interface OperationDefinitionMap<C extends BaseIndexPatternColumn, P = {}> {
field: FieldBasedOperationDefinition<C, P>;
none: FieldlessOperationDefinition<C, P>;
fullReference: FullReferenceOperationDefinition<C>;
Expand Down Expand Up @@ -731,11 +731,11 @@ export type OperationType = string;
* This is an operation definition of an unspecified column out of all possible
* column types.
*/
export type GenericOperationDefinition =
| OperationDefinition<BaseIndexPatternColumn, 'field'>
| OperationDefinition<BaseIndexPatternColumn, 'none'>
| OperationDefinition<BaseIndexPatternColumn, 'fullReference'>
| OperationDefinition<BaseIndexPatternColumn, 'managedReference'>;
export type GenericOperationDefinition<
C extends BaseIndexPatternColumn = BaseIndexPatternColumn,
P = {},
AR extends boolean = false
> = OperationDefinition<BaseIndexPatternColumn, keyof OperationDefinitionMap<C>, P, AR>;

/**
* List of all available operation definitions
Expand Down

This file was deleted.

Loading
Loading