diff --git a/.betterer.results b/.betterer.results index 3fff26564be..16d6dde1d23 100644 --- a/.betterer.results +++ b/.betterer.results @@ -5872,15 +5872,8 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "32"] ], "public/app/features/variables/guard.ts:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"], - [0, 0, 0, "Do not use any type assertions.", "1"], - [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Unexpected any. Specify a different type.", "4"], - [0, 0, 0, "Unexpected any. Specify a different type.", "5"], - [0, 0, 0, "Do not use any type assertions.", "6"], - [0, 0, 0, "Unexpected any. Specify a different type.", "7"], - [0, 0, 0, "Do not use any type assertions.", "8"] + [0, 0, 0, "Unexpected any. Specify a different type.", "0"], + [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], "public/app/features/variables/inspect/NetworkGraph.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/packages/grafana-data/src/types/datasource.ts b/packages/grafana-data/src/types/datasource.ts index 157902aff58..94399d04730 100644 --- a/packages/grafana-data/src/types/datasource.ts +++ b/packages/grafana-data/src/types/datasource.ts @@ -173,6 +173,13 @@ export interface DataSourceConstructor< new (instanceSettings: DataSourceInstanceSettings, ...args: any[]): DSType; } +// VariableSupport is hoisted up to its own type to fix the wonky intermittent +// 'variables is references directly or indirectly' error +type VariableSupport = + | StandardVariableSupport> + | CustomVariableSupport> + | DataSourceVariableSupport>; + /** * The main data source abstraction interface, represents an instance of a data source * @@ -341,10 +348,7 @@ abstract class DataSourceApi< * Defines new variable support * @alpha -- experimental */ - variables?: - | StandardVariableSupport> - | CustomVariableSupport> - | DataSourceVariableSupport>; + variables?: VariableSupport; /* * Optionally, use this method to set default values for a query diff --git a/public/app/features/variables/guard.test.ts b/public/app/features/variables/guard.test.ts index 476fbf4289b..d01b060bce0 100644 --- a/public/app/features/variables/guard.test.ts +++ b/public/app/features/variables/guard.test.ts @@ -1,4 +1,4 @@ -import { VariableSupportType } from '@grafana/data'; +import { TypedVariableModel, VariableSupportType, VariableType } from '@grafana/data'; import { LegacyVariableQueryEditor } from './editor/LegacyVariableQueryEditor'; import { StandardVariableQueryEditor } from './editor/getVariableQueryEditor'; @@ -9,7 +9,22 @@ import { hasStandardVariableSupport, isLegacyQueryEditor, isQueryEditor, + isMulti, + hasOptions, + hasCurrent, } from './guard'; +import { + createAdhocVariable, + createConstantVariable, + createCustomVariable, + createDashboardVariable, + createDatasourceVariable, + createIntervalVariable, + createOrgVariable, + createQueryVariable, + createTextBoxVariable, + createUserVariable, +} from './state/__tests__/fixtures'; describe('type guards', () => { describe('hasLegacyVariableSupport', () => { @@ -135,6 +150,53 @@ describe('type guards', () => { }); }); }); + + interface VariableFacts { + variable: TypedVariableModel; + isMulti: boolean; + hasOptions: boolean; + hasCurrent: boolean; + } + // This structure is typed (because the key is a const string union) to ensure that we always + // test every type of variable, as new variables are added + type ExtraVariableTypes = 'org' | 'dashboard' | 'user'; + // prettier-ignore + const variableFactsObj: Record = { + query: { variable: createQueryVariable(), isMulti: true, hasOptions: true, hasCurrent: true }, + adhoc: { variable: createAdhocVariable(), isMulti: false, hasOptions: false, hasCurrent: false }, + constant: { variable: createConstantVariable(), isMulti: false, hasOptions: true, hasCurrent: true }, + datasource: { variable: createDatasourceVariable(), isMulti: true, hasOptions: true, hasCurrent: true }, + interval: { variable: createIntervalVariable(), isMulti: false, hasOptions: true, hasCurrent: true }, + textbox: { variable: createTextBoxVariable(), isMulti: false, hasOptions: true, hasCurrent: true }, + system: { variable: createUserVariable(), isMulti: false, hasOptions: false, hasCurrent: true }, + user: { variable: createUserVariable(), isMulti: false, hasOptions: false, hasCurrent: true }, + org: { variable: createOrgVariable(), isMulti: false, hasOptions: false, hasCurrent: true }, + dashboard: { variable: createDashboardVariable(), isMulti: false, hasOptions: false, hasCurrent: true }, + custom: { variable: createCustomVariable(), isMulti: true, hasOptions: true, hasCurrent: true }, + }; + + const variableFacts = Object.values(variableFactsObj); + + it.each(variableFacts)( + 'isMulti correctly identifies variables with multi support: $variable.type should be $isMulti', + ({ variable, isMulti: expected }) => { + expect(isMulti(variable)).toBe(expected); + } + ); + + it.each(variableFacts)( + 'hasOptions correctly identifies variables with options support: $variable.type should be $hasOptions', + ({ variable, hasOptions: expected }) => { + expect(hasOptions(variable)).toBe(expected); + } + ); + + it.each(variableFacts)( + 'hasCurrent correctly identifies variables with options support: $variable.type should be $hasCurrent', + ({ variable, hasCurrent: expected }) => { + expect(hasCurrent(variable)).toBe(expected); + } + ); }); describe('isLegacyQueryEditor', () => { diff --git a/public/app/features/variables/guard.ts b/public/app/features/variables/guard.ts index 524e959d061..a24869c2c67 100644 --- a/public/app/features/variables/guard.ts +++ b/public/app/features/variables/guard.ts @@ -2,7 +2,6 @@ import { ComponentType } from 'react'; import { Observable } from 'rxjs'; import { - CustomVariableSupport, DataQuery, DataQueryRequest, DataQueryResponse, @@ -11,7 +10,6 @@ import { DataSourceRef, MetricFindValue, StandardVariableQuery, - StandardVariableSupport, VariableModel, VariableSupportType, } from '@grafana/data'; @@ -43,27 +41,17 @@ export const isConstant = (model: VariableModel): model is ConstantVariableModel }; export const isMulti = (model: VariableModel): model is VariableWithMultiSupport => { - const withMulti = model as VariableWithMultiSupport; - return withMulti.hasOwnProperty('multi') && typeof withMulti.multi === 'boolean'; + return 'multi' in model; }; export const hasOptions = (model: VariableModel): model is VariableWithOptions => { - return hasObjectProperty(model, 'options'); + return 'options' in model; }; export const hasCurrent = (model: VariableModel): model is VariableWithOptions => { - return hasObjectProperty(model, 'current'); + return 'current' in model; }; -function hasObjectProperty(model: VariableModel, property: string): model is VariableWithOptions { - if (!model) { - return false; - } - - const withProperty = model as Record; - return withProperty.hasOwnProperty(property) && typeof withProperty[property] === 'object'; -} - export function isLegacyAdHocDataSource(datasource: null | DataSourceRef | string): datasource is string { if (datasource === null) { return false; @@ -92,7 +80,6 @@ interface DataSourceWithStandardVariableSupport< } interface DataSourceWithCustomVariableSupport< - VariableQuery extends DataQuery = any, TQuery extends DataQuery = DataQuery, TOptions extends DataSourceJsonData = DataSourceJsonData > extends DataSourceApi { @@ -139,9 +126,8 @@ export const hasStandardVariableSupport = < return false; } - const variableSupport = datasource.variables as StandardVariableSupport>; - - return Boolean(variableSupport.toDataQuery); + const variableSupport = datasource.variables; + return 'toDataQuery' in variableSupport && Boolean(variableSupport.toDataQuery); }; export const hasCustomVariableSupport = < @@ -149,7 +135,7 @@ export const hasCustomVariableSupport = < TOptions extends DataSourceJsonData = DataSourceJsonData >( datasource: DataSourceApi -): datasource is DataSourceWithCustomVariableSupport => { +): datasource is DataSourceWithCustomVariableSupport => { if (!datasource.variables) { return false; } @@ -158,9 +144,13 @@ export const hasCustomVariableSupport = < return false; } - const variableSupport = datasource.variables as CustomVariableSupport>; - - return Boolean(variableSupport.query) && Boolean(variableSupport.editor); + const variableSupport = datasource.variables; + return ( + 'query' in variableSupport && + 'editor' in variableSupport && + Boolean(variableSupport.query) && + Boolean(variableSupport.editor) + ); }; export const hasDatasourceVariableSupport = < diff --git a/public/app/features/variables/state/__tests__/fixtures.ts b/public/app/features/variables/state/__tests__/fixtures.ts index e8e7e6178fc..8f7ae7cc9f2 100644 --- a/public/app/features/variables/state/__tests__/fixtures.ts +++ b/public/app/features/variables/state/__tests__/fixtures.ts @@ -1,9 +1,16 @@ import { + AdHocVariableModel, BaseVariableModel, ConstantVariableModel, CustomVariableModel, + DashboardVariableModel, + DataSourceVariableModel, + IntervalVariableModel, LoadingState, + OrgVariableModel, QueryVariableModel, + TextBoxVariableModel, + UserVariableModel, VariableHide, VariableOption, VariableRefresh, @@ -59,7 +66,19 @@ export function createQueryVariable(input: Partial = {}): Qu }; } -export function createConstantVariable(input: Partial): ConstantVariableModel { +export function createAdhocVariable(input?: Partial): AdHocVariableModel { + return { + ...createBaseVariableModel('adhoc'), + datasource: { + uid: 'abc-123', + type: 'prometheus', + }, + filters: [], + ...input, + }; +} + +export function createConstantVariable(input: Partial = {}): ConstantVariableModel { return { ...createBaseVariableModel('constant'), query: '', @@ -70,7 +89,89 @@ export function createConstantVariable(input: Partial): C }; } -export function createCustomVariable(input: Partial): CustomVariableModel { +export function createDatasourceVariable(input: Partial = {}): DataSourceVariableModel { + return { + ...createBaseVariableModel('datasource'), + regex: '', + refresh: VariableRefresh.onDashboardLoad, + multi: false, + includeAll: false, + query: '', + current: createVariableOption('prom-prod', { text: 'Prometheus (main)', selected: true }), + options: [ + createVariableOption('prom-prod', { text: 'Prometheus (main)', selected: true }), + createVariableOption('prom-dev'), + ], + ...input, + }; +} + +export function createIntervalVariable(input: Partial = {}): IntervalVariableModel { + return { + ...createBaseVariableModel('interval'), + auto: false, + auto_count: 30, + auto_min: '10s', + refresh: VariableRefresh.onTimeRangeChanged, + query: '1m,10m,30m,1h,6h,12h,1d,7d,14d,30d', + options: [], + current: createVariableOption('10m'), + ...input, + }; +} + +export function createTextBoxVariable(input: Partial = {}): TextBoxVariableModel { + return { + ...createBaseVariableModel('textbox'), + originalQuery: null, + query: '', + current: createVariableOption('prom-prod'), + options: [], + ...input, + }; +} + +export function createUserVariable(input: Partial = {}): UserVariableModel { + return { + ...createBaseVariableModel('system'), + current: { + value: { + login: 'biggus-chungus', + id: 0, + email: 'chungus@example.com', + }, + }, + ...input, + }; +} + +export function createOrgVariable(input: Partial = {}): OrgVariableModel { + return { + ...createBaseVariableModel('system'), + current: { + value: { + name: 'Big Chungus Corp.', + id: 3, + }, + }, + ...input, + }; +} + +export function createDashboardVariable(input: Partial = {}): DashboardVariableModel { + return { + ...createBaseVariableModel('system'), + current: { + value: { + name: 'Chungus Monitoring', + uid: 'b1g', + }, + }, + ...input, + }; +} + +export function createCustomVariable(input: Partial = {}): CustomVariableModel { return { ...createBaseVariableModel('custom'), multi: false,