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 1 commit
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 @@ -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
Loading