Typed variables pt1: Use discriminated union for variable model (#52981)

* wip

* make diff easier to read

* Update template_srv getVariables to return new TypedVariableModel

* update VariableType to use the type from TypedVariableModel

* tidy things up

.betterer.results

@@ -751,6 +751,10 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "2"]
     ],
+    "packages/grafana-data/src/types/templateVars.ts:5381": [
+      [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "1"]
+    ],
     "packages/grafana-data/src/types/trace.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"]
     ],
@@ -5726,6 +5730,9 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "3"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "4"]
     ],
+    "public/app/features/templating/template_srv.mock.ts:5381": [
+      [0, 0, 0, "Do not use any type assertions.", "0"]
+    ],
     "public/app/features/templating/template_srv.test.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"],
@@ -6275,13 +6282,9 @@ exports[`better eslint`] = {
     "public/app/features/variables/types.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"],
-      [0, 0, 0, "Do not use any type assertions.", "2"],
-      [0, 0, 0, "Do not use any type assertions.", "3"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "4"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "5"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "6"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "7"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "8"]
+      [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"]
     ],
     "public/app/features/variables/utils.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

packages/grafana-data/src/types/templateVars.ts

@@ -1,7 +1,159 @@
-export type VariableType = 'query' | 'adhoc' | 'constant' | 'datasource' | 'interval' | 'textbox' | 'custom' | 'system';
+import { LoadingState } from './data';
+import { DataSourceRef } from './query';
 
+export type VariableType = TypedVariableModel['type'];
+
+/** @deprecated Use TypedVariableModel instead */
 export interface VariableModel {
   type: VariableType;
   name: string;
   label?: string;
 }
+
+export type TypedVariableModel =
+  | QueryVariableModel
+  | AdHocVariableModel
+  | ConstantVariableModel
+  | DataSourceVariableModel
+  | IntervalVariableModel
+  | TextBoxVariableModel
+  | CustomVariableModel
+  | UserVariableModel
+  | OrgVariableModel
+  | DashboardVariableModel;
+
+export enum VariableRefresh {
+  never, // removed from the UI
+  onDashboardLoad,
+  onTimeRangeChanged,
+}
+
+export enum VariableSort {
+  disabled,
+  alphabeticalAsc,
+  alphabeticalDesc,
+  numericalAsc,
+  numericalDesc,
+  alphabeticalCaseInsensitiveAsc,
+  alphabeticalCaseInsensitiveDesc,
+}
+
+export enum VariableHide {
+  dontHide,
+  hideLabel,
+  hideVariable,
+}
+
+export interface AdHocVariableFilter {
+  key: string;
+  operator: string;
+  value: string;
+  condition: string;
+}
+
+export interface AdHocVariableModel extends BaseVariableModel {
+  type: 'adhoc';
+  datasource: DataSourceRef | null;
+  filters: AdHocVariableFilter[];
+}
+
+export interface VariableOption {
+  selected: boolean;
+  text: string | string[];
+  value: string | string[];
+  isNone?: boolean;
+}
+
+export interface IntervalVariableModel extends VariableWithOptions {
+  type: 'interval';
+  auto: boolean;
+  auto_min: string;
+  auto_count: number;
+  refresh: VariableRefresh;
+}
+
+export interface CustomVariableModel extends VariableWithMultiSupport {
+  type: 'custom';
+}
+
+export interface DataSourceVariableModel extends VariableWithMultiSupport {
+  type: 'datasource';
+  regex: string;
+  refresh: VariableRefresh;
+}
+
+export interface QueryVariableModel extends VariableWithMultiSupport {
+  type: 'query';
+  datasource: DataSourceRef | null;
+  definition: string;
+  sort: VariableSort;
+  queryValue?: string;
+  query: any;
+  regex: string;
+  refresh: VariableRefresh;
+}
+
+export interface TextBoxVariableModel extends VariableWithOptions {
+  type: 'textbox';
+  originalQuery: string | null;
+}
+
+export interface ConstantVariableModel extends VariableWithOptions {
+  type: 'constant';
+}
+
+export interface VariableWithMultiSupport extends VariableWithOptions {
+  multi: boolean;
+  includeAll: boolean;
+  allValue?: string | null;
+}
+
+export interface VariableWithOptions extends BaseVariableModel {
+  current: VariableOption;
+  options: VariableOption[];
+  query: string;
+}
+
+export interface DashboardProps {
+  name: string;
+  uid: string;
+  toString: () => string;
+}
+
+export interface DashboardVariableModel extends SystemVariable<DashboardProps> {}
+
+export interface OrgProps {
+  name: string;
+  id: number;
+  toString: () => string;
+}
+
+export interface OrgVariableModel extends SystemVariable<OrgProps> {}
+
+export interface UserProps {
+  login: string;
+  id: number;
+  email?: string;
+  toString: () => string;
+}
+
+export interface UserVariableModel extends SystemVariable<UserProps> {}
+
+export interface SystemVariable<TProps extends { toString: () => string }> extends BaseVariableModel {
+  type: 'system';
+  current: { value: TProps };
+}
+
+export interface BaseVariableModel extends VariableModel {
+  name: string;
+  label?: string;
+  id: string;
+  rootStateKey: string | null;
+  global: boolean;
+  hide: VariableHide;
+  skipUrlSync: boolean;
+  index: number;
+  state: LoadingState;
+  error: any | null;
+  description: string | null;
+}

packages/grafana-runtime/src/services/templateSrv.ts

@@ -1,4 +1,4 @@
-import { VariableModel, ScopedVars, TimeRange } from '@grafana/data';
+import { ScopedVars, TimeRange, TypedVariableModel } from '@grafana/data';
 
 /**
  * Via the TemplateSrv consumers get access to all the available template variables
@@ -11,7 +11,7 @@ export interface TemplateSrv {
   /**
    * List the dashboard variables
    */
-  getVariables(): VariableModel[];
+  getVariables(): TypedVariableModel[];
 
   /**
    * Replace the values within the target string.  See also {@link InterpolateFunction}

public/app/features/dashboard/state/DashboardMigrator.ts

@@ -51,7 +51,7 @@ import {
   migrateMultipleStatsAnnotationQuery,
   migrateMultipleStatsMetricsQuery,
 } from '../../../plugins/datasource/cloudwatch/migrations/dashboardMigrations';
-import { VariableHide } from '../../variables/types';
+import { ConstantVariableModel, TextBoxVariableModel, VariableHide } from '../../variables/types';
 
 import { DashboardModel } from './DashboardModel';
 import { PanelModel } from './PanelModel';
@@ -621,18 +621,27 @@ export class DashboardMigrator {
     }
 
     if (oldVersion < 27) {
-      for (const variable of this.dashboard.templating.list) {
+      this.dashboard.templating.list = this.dashboard.templating.list.map((variable) => {
         if (!isConstant(variable)) {
-          continue;
+          return variable;
         }
 
-        if (variable.hide === VariableHide.dontHide || variable.hide === VariableHide.hideLabel) {
-          variable.type = 'textbox';
+        const newVariable: ConstantVariableModel | TextBoxVariableModel = {
+          ...variable,
+        };
+
+        newVariable.current = { selected: true, text: newVariable.query ?? '', value: newVariable.query ?? '' };
+        newVariable.options = [newVariable.current];
+
+        if (newVariable.hide === VariableHide.dontHide || newVariable.hide === VariableHide.hideLabel) {
+          return {
+            ...newVariable,
+            type: 'textbox',
+          };
         }
 
-        variable.current = { selected: true, text: variable.query ?? '', value: variable.query ?? '' };
-        variable.options = [variable.current];
-      }
+        return newVariable;
+      });
     }
 
     if (oldVersion < 28) {

public/app/features/plugins/datasource_srv.ts

@@ -24,8 +24,6 @@ import {
   instanceSettings as expressionInstanceSettings,
 } from 'app/features/expressions/ExpressionDatasource';
 
-import { isDataSource } from '../variables/guard';
-
 import { importDataSourcePlugin } from './plugin_loader';
 
 export class DatasourceSrv implements DataSourceService {
@@ -246,7 +244,7 @@ export class DatasourceSrv implements DataSourceService {
 
     if (filters.variables) {
       for (const variable of this.templateSrv.getVariables()) {
-        if (!isDataSource(variable) || variable.multi || variable.includeAll) {
+        if (variable.type !== 'datasource' || variable.multi || variable.includeAll) {
           continue;
         }
         const dsName = variable.current.value === 'default' ? this.defaultName : variable.current.value;

public/app/features/templating/template_srv.mock.ts

@@ -1,4 +1,4 @@
-import { ScopedVars, TimeRange, VariableModel } from '@grafana/data';
+import { ScopedVars, TimeRange, TypedVariableModel } from '@grafana/data';
 import { TemplateSrv } from '@grafana/runtime';
 
 import { variableRegex } from '../variables/utils';
@@ -15,14 +15,16 @@ export class TemplateSrvMock implements TemplateSrv {
   private regex = variableRegex;
   constructor(private variables: Record<string, string>) {}
 
-  getVariables(): VariableModel[] {
+  getVariables(): TypedVariableModel[] {
     return Object.keys(this.variables).map((key) => {
       return {
         type: 'custom',
         name: key,
         label: key,
       };
-    });
+      // TODO: we remove this type assertion in a later PR
+      // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
+    }) as TypedVariableModel[];
   }
 
   replace(target?: string, scopedVars?: ScopedVars, format?: string | Function): string {

public/app/features/templating/template_srv.ts

@@ -1,13 +1,19 @@
 import { escape, isString, property } from 'lodash';
 
-import { deprecationWarning, ScopedVars, TimeRange } from '@grafana/data';
+import {
+  deprecationWarning,
+  ScopedVars,
+  TimeRange,
+  AdHocVariableFilter,
+  AdHocVariableModel,
+  TypedVariableModel,
+} from '@grafana/data';
 import { getDataSourceSrv, setTemplateSrv, TemplateSrv as BaseTemplateSrv } from '@grafana/runtime';
 
 import { variableAdapters } from '../variables/adapters';
 import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE } from '../variables/constants';
 import { isAdHoc } from '../variables/guard';
 import { getFilteredVariables, getVariables, getVariableWithName } from '../variables/state/selectors';
-import { AdHocVariableFilter, AdHocVariableModel, VariableModel } from '../variables/types';
 import { variableRegex } from '../variables/utils';
 
 import { FormatOptions, formatRegistry, FormatRegistryID } from './formatRegistry';
@@ -56,8 +62,10 @@ export class TemplateSrv implements BaseTemplateSrv {
     return this.getVariables();
   }
 
-  getVariables(): VariableModel[] {
-    return this.dependencies.getVariables();
+  getVariables(): TypedVariableModel[] {
+    // TODO: we remove this type assertion in a later PR
+    // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
+    return this.dependencies.getVariables() as TypedVariableModel[];
   }
 
   updateIndex() {

public/app/features/variables/guard.ts

@@ -28,14 +28,17 @@ import {
   DataSourceVariableModel,
 } from './types';
 
+/** @deprecated use a if (model.type === "query") type narrowing check instead */
 export const isQuery = (model: VariableModel): model is QueryVariableModel => {
   return model.type === 'query';
 };
 
+/** @deprecated use a if (model.type === "adhoc") type narrowing check instead */
 export const isAdHoc = (model: VariableModel): model is AdHocVariableModel => {
   return model.type === 'adhoc';
 };
 
+/** @deprecated use a if (model.type === "constant") type narrowing check instead */
 export const isConstant = (model: VariableModel): model is ConstantVariableModel => {
   return model.type === 'constant';
 };

public/app/features/variables/query/QueryVariableEditor.test.tsx

@@ -9,17 +9,18 @@ import { DataSourceType } from 'app/features/alerting/unified/utils/datasource';
 import { NEW_VARIABLE_ID } from '../constants';
 import { LegacyVariableQueryEditor } from '../editor/LegacyVariableQueryEditor';
 import { KeyedVariableIdentifier } from '../state/types';
-import { VariableModel } from '../types';
+import { QueryVariableModel } from '../types';
 
 import { Props, QueryVariableEditorUnConnected } from './QueryVariableEditor';
 import { initialQueryVariableModelState } from './reducer';
 
 const setupTestContext = (options: Partial<Props>) => {
-  const variableDefaults: Partial<VariableModel> = { rootStateKey: 'key' };
+  const variableDefaults: Partial<QueryVariableModel> = { rootStateKey: 'key' };
   const extended = {
     VariableQueryEditor: LegacyVariableQueryEditor,
     dataSource: {} as unknown as DataSourceApi,
   };
+
   const defaults: Props = {
     variable: { ...initialQueryVariableModelState, ...variableDefaults },
     initQueryVariableEditor: jest.fn(),

public/app/features/variables/shared/testing/datasourceVariableBuilder.ts

@@ -1,8 +1,10 @@
-import { DataSourceVariableModel, VariableRefresh } from 'app/features/variables/types';
+import { DataSourceVariableModel, QueryVariableModel, VariableRefresh } from 'app/features/variables/types';
 
 import { MultiVariableBuilder } from './multiVariableBuilder';
 
-export class DatasourceVariableBuilder<T extends DataSourceVariableModel> extends MultiVariableBuilder<T> {
+export class DatasourceVariableBuilder<
+  T extends DataSourceVariableModel | QueryVariableModel
+> extends MultiVariableBuilder<T> {
   withRefresh(refresh: VariableRefresh) {
     this.variable.refresh = refresh;
     return this;

public/app/features/variables/state/sharedReducer.test.ts

@@ -37,7 +37,7 @@ describe('sharedReducer', () => {
     it('then state should be correct', () => {
       const model: any = {
         name: 'name from model',
-        type: 'type from model',
+        type: 'query',
         current: undefined,
       };
 
@@ -47,7 +47,7 @@ describe('sharedReducer', () => {
         global: true,
         index: 0,
         name: 'name from model',
-        type: 'type from model' as unknown as VariableType,
+        type: 'query',
         current: {} as unknown as VariableOption,
       };
 

public/app/features/variables/types.ts

@@ -4,11 +4,56 @@ import {
   BusEventWithPayload,
   DataQuery,
   DataSourceJsonData,
-  DataSourceRef,
   LoadingState,
   QueryEditorProps,
-  VariableModel as BaseVariableModel,
-  VariableType,
+  BaseVariableModel,
+  VariableHide,
+} from '@grafana/data';
+export {
+  /** @deprecated Import from @grafana/data instead */
+  VariableRefresh,
+  /** @deprecated Import from @grafana/data instead */
+  VariableSort,
+  /** @deprecated Import from @grafana/data instead */
+  VariableHide,
+  /** @deprecated Import from @grafana/data instead */
+  AdHocVariableFilter,
+  /** @deprecated Import from @grafana/data instead */
+  AdHocVariableModel,
+  /** @deprecated Import from @grafana/data instead */
+  VariableOption,
+  /** @deprecated Import from @grafana/data instead */
+  IntervalVariableModel,
+  /** @deprecated Import from @grafana/data instead */
+  CustomVariableModel,
+  /** @deprecated Import from @grafana/data instead */
+  DataSourceVariableModel,
+  /** @deprecated Import from @grafana/data instead */
+  QueryVariableModel,
+  /** @deprecated Import from @grafana/data instead */
+  TextBoxVariableModel,
+  /** @deprecated Import from @grafana/data instead */
+  ConstantVariableModel,
+  /** @deprecated Import from @grafana/data instead */
+  VariableWithMultiSupport,
+  /** @deprecated Import from @grafana/data instead */
+  VariableWithOptions,
+  /** @deprecated Import from @grafana/data instead */
+  DashboardProps,
+  /** @deprecated Import from @grafana/data instead */
+  DashboardVariableModel,
+  /** @deprecated Import from @grafana/data instead */
+  OrgProps,
+  /** @deprecated Import from @grafana/data instead */
+  OrgVariableModel,
+  /** @deprecated Import from @grafana/data instead */
+  UserProps,
+  /** @deprecated Import from @grafana/data instead */
+  UserVariableModel,
+  /** @deprecated Import from @grafana/data instead */
+  SystemVariable,
+  /** @deprecated Import from @grafana/data instead */
+  BaseVariableModel as VariableModel,
 } from '@grafana/data';
 import { TemplateSrv } from '@grafana/runtime';
 
@@ -20,133 +65,12 @@ export enum TransactionStatus {
   Completed = 'Completed',
 }
 
-export enum VariableRefresh {
-  never, // removed from the UI
-  onDashboardLoad,
-  onTimeRangeChanged,
-}
-
-export enum VariableHide {
-  dontHide,
-  hideLabel,
-  hideVariable,
-}
-
-export enum VariableSort {
-  disabled,
-  alphabeticalAsc,
-  alphabeticalDesc,
-  numericalAsc,
-  numericalDesc,
-  alphabeticalCaseInsensitiveAsc,
-  alphabeticalCaseInsensitiveDesc,
-}
-
-export interface VariableOption {
-  selected: boolean;
-  text: string | string[];
-  value: string | string[];
-  isNone?: boolean;
-}
-
-export interface AdHocVariableFilter {
-  key: string;
-  operator: string;
-  value: string;
-  condition: string;
-}
-
-export interface AdHocVariableModel extends VariableModel {
-  datasource: DataSourceRef | null;
-  filters: AdHocVariableFilter[];
-}
-
-export interface IntervalVariableModel extends VariableWithOptions {
-  auto: boolean;
-  auto_min: string;
-  auto_count: number;
-  refresh: VariableRefresh;
-}
-
-export interface CustomVariableModel extends VariableWithMultiSupport {}
-
-export interface DataSourceVariableModel extends VariableWithMultiSupport {
-  regex: string;
-  refresh: VariableRefresh;
-}
-
-export interface QueryVariableModel extends DataSourceVariableModel {
-  datasource: DataSourceRef | null;
-  definition: string;
-  sort: VariableSort;
-  queryValue?: string;
-  query: any;
-}
-
-export interface TextBoxVariableModel extends VariableWithOptions {
-  originalQuery: string | null;
-}
-
-export interface ConstantVariableModel extends VariableWithOptions {}
-
-export interface VariableWithMultiSupport extends VariableWithOptions {
-  multi: boolean;
-  includeAll: boolean;
-  allValue?: string | null;
-}
-
-export interface VariableWithOptions extends VariableModel {
-  current: VariableOption;
-  options: VariableOption[];
-  query: string;
-}
-
-export interface DashboardProps {
-  name: string;
-  uid: string;
-  toString: () => string;
-}
-
-export interface DashboardVariableModel extends SystemVariable<DashboardProps> {}
-
-export interface OrgProps {
-  name: string;
-  id: number;
-  toString: () => string;
-}
-
-export interface OrgVariableModel extends SystemVariable<OrgProps> {}
-
-export interface UserProps {
-  login: string;
-  id: number;
-  email?: string;
-  toString: () => string;
-}
-
-export interface UserVariableModel extends SystemVariable<UserProps> {}
-
-export interface SystemVariable<TProps extends { toString: () => string }> extends VariableModel {
-  current: { value: TProps };
-}
-
-export interface VariableModel extends BaseVariableModel {
-  id: string;
-  rootStateKey: string | null;
-  global: boolean;
-  hide: VariableHide;
-  skipUrlSync: boolean;
-  index: number;
-  state: LoadingState;
-  error: any | null;
-  description: string | null;
-}
-
-export const initialVariableModelState: VariableModel = {
+export const initialVariableModelState: BaseVariableModel = {
   id: NEW_VARIABLE_ID,
   rootStateKey: null,
   name: '',
-  type: '' as unknown as VariableType,
+  // TODO: in a later PR, remove type and type this object to Partial<BaseVariableModel>
+  type: 'query',
   global: false,
   index: -1,
   hide: VariableHide.dontHide,