Typed variables pt4: Remove generics from getVariable (#53017)

* 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

* Chore: Use type-accurate mock variables in tests

* Chore: Type VariableState to use TypedVariableModel

* fix typo

* remove type assertion from template_srv.getVariables

* use typescript/no-redeclare for compatibility with ts overloads

* remove generics from getVariable() and overload it to only return undefined based on arguments

* update usages of getVariable()

* Fix Interval variable options picker not working

.betterer.results

@@ -6195,10 +6195,7 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"]
     ],
     "public/app/features/variables/state/selectors.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, "Do not use any type assertions.", "2"],
-      [0, 0, 0, "Do not use any type assertions.", "3"]
+      [0, 0, 0, "Do not use any type assertions.", "0"]
     ],
     "public/app/features/variables/state/sharedReducer.test.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

.eslintrc

@@ -18,7 +18,11 @@
         "newlines-between": "always",
         "alphabetize": { "order": "asc" }
       }
-    ]
+    ],
+
+    // Use typescript's no-redeclare for compatibility with overrides
+    "no-redeclare": "off",
+    "@typescript-eslint/no-redeclare": ["error"]
   },
   "overrides": [
     {
@@ -35,6 +39,13 @@
         "react/jsx-uses-react": "off",
         "react/react-in-jsx-scope": "off"
       }
+    },
+    {
+      "files": ["public/dashboards/scripted*.js"],
+      "rules": {
+        "no-redeclare": "error",
+        "@typescript-eslint/no-redeclare": "off"
+      }
     }
   ]
 }

public/app/features/variables/datasource/actions.ts

@@ -10,7 +10,6 @@ import { validateVariableSelectionState } from '../state/actions';
 import { toKeyedAction } from '../state/keyedVariablesReducer';
 import { getVariable } from '../state/selectors';
 import { KeyedVariableIdentifier } from '../state/types';
-import { DataSourceVariableModel } from '../types';
 import { toVariablePayload } from '../utils';
 
 import { createDataSourceOptions } from './reducer';
@@ -27,7 +26,11 @@ export const updateDataSourceVariableOptions =
   async (dispatch, getState) => {
     const { rootStateKey } = identifier;
     const sources = dependencies.getDatasourceSrv().getList({ metrics: true, variables: false });
-    const variableInState = getVariable<DataSourceVariableModel>(identifier, getState());
+    const variableInState = getVariable(identifier, getState());
+    if (variableInState.type !== 'datasource') {
+      return;
+    }
+
     let regex;
 
     if (variableInState.regex) {

public/app/features/variables/editor/VariableEditorEditor.tsx

@@ -29,7 +29,7 @@ import { OnPropChangeArguments } from './types';
 
 const mapStateToProps = (state: StoreState, ownProps: OwnProps) => ({
   editor: getVariablesState(ownProps.identifier.rootStateKey, state).editor,
-  variable: getVariable(ownProps.identifier, state, false), // we could be renaming a variable and we don't want this to throw
+  variable: getVariable(ownProps.identifier, state),
 });
 
 const mapDispatchToProps = (dispatch: ThunkDispatch) => {

public/app/features/variables/interval/actions.ts

@@ -7,7 +7,6 @@ import { validateVariableSelectionState } from '../state/actions';
 import { toKeyedAction } from '../state/keyedVariablesReducer';
 import { getVariable } from '../state/selectors';
 import { KeyedVariableIdentifier } from '../state/types';
-import { IntervalVariableModel } from '../types';
 import { toVariablePayload } from '../utils';
 
 import { createIntervalOptions } from './reducer';
@@ -37,7 +36,11 @@ export const updateAutoValue =
     }
   ): ThunkResult<void> =>
   (dispatch, getState) => {
-    const variableInState = getVariable<IntervalVariableModel>(identifier, getState());
+    const variableInState = getVariable(identifier, getState());
+    if (variableInState.type !== 'interval') {
+      return;
+    }
+
     if (variableInState.auto) {
       const res = dependencies.calculateInterval(
         dependencies.getTimeSrv().timeRange(),

public/app/features/variables/pickers/OptionsPicker/actions.ts

@@ -3,11 +3,12 @@ import { debounce, trim } from 'lodash';
 import { StoreState, ThunkDispatch, ThunkResult } from 'app/types';
 
 import { variableAdapters } from '../../adapters';
+import { hasOptions } from '../../guard';
 import { toKeyedAction } from '../../state/keyedVariablesReducer';
 import { getVariable, getVariablesState } from '../../state/selectors';
 import { changeVariableProp, setCurrentVariableValue } from '../../state/sharedReducer';
 import { KeyedVariableIdentifier } from '../../state/types';
-import { VariableOption, VariableWithMultiSupport, VariableWithOptions } from '../../types';
+import { VariableOption, VariableWithOptions } from '../../types';
 import { containsSearchFilter, getCurrentValue, toVariablePayload } from '../../utils';
 import { NavigationKey } from '../types';
 
@@ -57,7 +58,12 @@ export const filterOrSearchOptions = (
     const { rootStateKey } = passedIdentifier;
     const { id, queryValue } = getVariablesState(rootStateKey, getState()).optionsPicker;
     const identifier: KeyedVariableIdentifier = { id, rootStateKey: rootStateKey, type: 'query' };
-    const { query, options } = getVariable<VariableWithOptions>(identifier, getState());
+    const variable = getVariable(identifier, getState());
+    if (!hasOptions(variable)) {
+      return;
+    }
+
+    const { query, options } = variable;
     dispatch(toKeyedAction(rootStateKey, updateSearchQuery(searchQuery)));
 
     if (trim(queryValue) === trim(searchQuery)) {
@@ -71,7 +77,7 @@ export const filterOrSearchOptions = (
   };
 };
 
-const setVariable = async (updated: VariableWithMultiSupport) => {
+const setVariable = async (updated: VariableWithOptions) => {
   const adapter = variableAdapters.get(updated.type);
   await adapter.setValue(updated, updated.current, true);
   return;
@@ -81,13 +87,22 @@ export const commitChangesToVariable = (key: string, callback?: (updated: any) =
   return async (dispatch, getState) => {
     const picker = getVariablesState(key, getState()).optionsPicker;
     const identifier: KeyedVariableIdentifier = { id: picker.id, rootStateKey: key, type: 'query' };
-    const existing = getVariable<VariableWithMultiSupport>(identifier, getState());
+    const existing = getVariable(identifier, getState());
+    if (!hasOptions(existing)) {
+      return;
+    }
+
     const currentPayload = { option: mapToCurrent(picker) };
     const searchQueryPayload = { propName: 'queryValue', propValue: picker.queryValue };
 
     dispatch(toKeyedAction(key, setCurrentVariableValue(toVariablePayload(existing, currentPayload))));
     dispatch(toKeyedAction(key, changeVariableProp(toVariablePayload(existing, searchQueryPayload))));
-    const updated = getVariable<VariableWithMultiSupport>(identifier, getState());
+
+    const updated = getVariable(identifier, getState());
+    if (!hasOptions(updated)) {
+      return;
+    }
+
     dispatch(toKeyedAction(key, hideOptions()));
 
     if (getCurrentValue(existing) === getCurrentValue(updated)) {
@@ -112,7 +127,11 @@ export const openOptions =
       await dispatch(commitChangesToVariable(uid, callback));
     }
 
-    const variable = getVariable<VariableWithMultiSupport>(identifier, getState());
+    const variable = getVariable(identifier, getState());
+    if (!hasOptions(variable)) {
+      return;
+    }
+
     dispatch(toKeyedAction(uid, showOptions(variable)));
   };
 
@@ -133,12 +152,19 @@ const searchForOptions = async (
   try {
     const { id } = getVariablesState(key, getState()).optionsPicker;
     const identifier: KeyedVariableIdentifier = { id, rootStateKey: key, type: 'query' };
-    const existing = getVariable<VariableWithOptions>(identifier, getState());
+    const existing = getVariable(identifier, getState());
+    if (!hasOptions(existing)) {
+      return;
+    }
 
     const adapter = variableAdapters.get(existing.type);
     await adapter.updateOptions(existing, searchQuery);
 
-    const updated = getVariable<VariableWithOptions>(identifier, getState());
+    const updated = getVariable(identifier, getState());
+    if (!hasOptions(updated)) {
+      return;
+    }
+
     dispatch(toKeyedAction(key, updateOptionsFromSearch(updated.options)));
   } catch (error) {
     console.error(error);

public/app/features/variables/pickers/OptionsPicker/reducer.ts

@@ -147,8 +147,10 @@ const optionsPickerSlice = createSlice({
           } else {
             state.selectedValues = [];
           }
+
           return applyStateChanges(state, updateDefaultSelection, updateAllSelection, updateOptions);
         }
+
         if (forceSelect || selected) {
           state.selectedValues.push({ ...option, selected: true });
           return applyStateChanges(state, updateDefaultSelection, updateAllSelection, updateOptions);

public/app/features/variables/query/VariableQueryRunner.ts

@@ -105,7 +105,11 @@ export class VariableQueryRunner {
 
       this.updateOptionsResults.next({ identifier, state: LoadingState.Loading });
 
-      const variable = getVariable<QueryVariableModel>(identifier, getState());
+      const variable = getVariable(identifier, getState());
+      if (variable.type !== 'query') {
+        return;
+      }
+
       const timeSrv = getTimeSrv();
       const runnerArgs = { variable, datasource, searchFilter, timeSrv, runRequest };
       const runner = queryRunners.getRunnerForDatasource(datasource);

public/app/features/variables/query/actions.ts

@@ -12,7 +12,6 @@ import { toKeyedAction } from '../state/keyedVariablesReducer';
 import { getVariable, getVariablesState } from '../state/selectors';
 import { changeVariableProp } from '../state/sharedReducer';
 import { KeyedVariableIdentifier } from '../state/types';
-import { QueryVariableModel } from '../types';
 import { hasOngoingTransaction, toKeyedVariableIdentifier, toVariablePayload } from '../utils';
 
 import { getVariableQueryRunner } from './VariableQueryRunner';
@@ -30,7 +29,11 @@ export const updateQueryVariableOptions = (
         return;
       }
 
-      const variableInState = getVariable<QueryVariableModel>(identifier, getState());
+      const variableInState = getVariable(identifier, getState());
+      if (variableInState.type !== 'query') {
+        return;
+      }
+
       if (getVariablesState(rootStateKey, getState()).editor.id === variableInState.id) {
         dispatch(toKeyedAction(rootStateKey, removeVariableEditorError({ errorProp: 'update' })));
       }
@@ -65,7 +68,11 @@ export const updateQueryVariableOptions = (
 export const initQueryVariableEditor =
   (identifier: KeyedVariableIdentifier): ThunkResult<void> =>
   async (dispatch, getState) => {
-    const variable = getVariable<QueryVariableModel>(identifier, getState());
+    const variable = getVariable(identifier, getState());
+    if (variable.type !== 'query') {
+      return;
+    }
+
     await dispatch(changeQueryVariableDataSource(toKeyedVariableIdentifier(variable), variable.datasource));
   };
 
@@ -111,7 +118,11 @@ export const changeQueryVariableQuery =
   (identifier: KeyedVariableIdentifier, query: any, definition?: string): ThunkResult<void> =>
   async (dispatch, getState) => {
     const { rootStateKey } = identifier;
-    const variableInState = getVariable<QueryVariableModel>(identifier, getState());
+    const variableInState = getVariable(identifier, getState());
+    if (variableInState.type !== 'query') {
+      return;
+    }
+
     if (hasSelfReferencingQuery(variableInState.name, query)) {
       const errorText = 'Query cannot contain a reference to itself. Variable: $' + variableInState.name;
       dispatch(toKeyedAction(rootStateKey, addVariableEditorError({ errorProp: 'query', errorText })));

public/app/features/variables/state/actions.ts

@@ -54,7 +54,6 @@ import {
   VariablesChangedEvent,
   VariablesChangedInUrl,
   VariablesTimeRangeProcessDone,
-  VariableWithMultiSupport,
   VariableWithOptions,
 } from '../types';
 import {
@@ -256,7 +255,11 @@ export const addSystemTemplateVariables = (key: string, dashboard: DashboardMode
 export const changeVariableMultiValue = (identifier: KeyedVariableIdentifier, multi: boolean): ThunkResult<void> => {
   return (dispatch, getState) => {
     const { rootStateKey: key } = identifier;
-    const variable = getVariable<VariableWithMultiSupport>(identifier, getState());
+    const variable = getVariable(identifier, getState());
+    if (!isMulti(variable)) {
+      return;
+    }
+
     const current = alignCurrentWithMulti(variable.current, multi);
 
     dispatch(
@@ -404,7 +407,11 @@ export const setOptionFromUrl = (
     }
 
     // get variable from state
-    const variableFromState = getVariable<VariableWithOptions>(toKeyedVariableIdentifier(variable), getState());
+    const variableFromState = getVariable(toKeyedVariableIdentifier(variable), getState());
+    if (!hasOptions(variableFromState)) {
+      return;
+    }
+
     if (!variableFromState) {
       throw new Error(`Couldn't find variable with name: ${variable.name}`);
     }
@@ -486,7 +493,11 @@ export const validateVariableSelectionState = (
   defaultValue?: string
 ): ThunkResult<Promise<void>> => {
   return (dispatch, getState) => {
-    const variableInState = getVariable<VariableWithOptions>(identifier, getState());
+    const variableInState = getVariable(identifier, getState());
+    if (!hasOptions(variableInState)) {
+      return Promise.resolve();
+    }
+
     const current = variableInState.current || ({} as unknown as VariableOption);
     const setValue = variableAdapters.get(variableInState.type).setValue;
 
@@ -664,12 +675,20 @@ export const onTimeRangeUpdated =
 const timeRangeUpdated =
   (identifier: KeyedVariableIdentifier): ThunkResult<Promise<void>> =>
   async (dispatch, getState) => {
-    const variableInState = getVariable<VariableWithOptions>(identifier, getState());
+    const variableInState = getVariable(identifier, getState());
+    if (!hasOptions(variableInState)) {
+      return;
+    }
+
     const previousOptions = variableInState.options.slice();
 
     await dispatch(updateOptions(toKeyedVariableIdentifier(variableInState), true));
 
-    const updatedVariable = getVariable<VariableWithOptions>(identifier, getState());
+    const updatedVariable = getVariable(identifier, getState());
+    if (!hasOptions(updatedVariable)) {
+      return;
+    }
+
     const updatedOptions = updatedVariable.options;
 
     if (JSON.stringify(previousOptions) !== JSON.stringify(updatedOptions)) {
@@ -918,9 +937,8 @@ export function upgradeLegacyQueries(
       return;
     }
 
-    const variable = getVariable<QueryVariableModel>(identifier, getState());
-
-    if (!isQuery(variable)) {
+    const variable = getVariable(identifier, getState());
+    if (variable.type !== 'query') {
       return;
     }
 

public/app/features/variables/state/selectors.ts

@@ -9,26 +9,35 @@ import { toStateKey } from '../utils';
 import { getInitialTemplatingState, TemplatingState } from './reducers';
 import { KeyedVariableIdentifier, VariablesState } from './types';
 
-// TODO: this is just a temporary type until we remove generics from getVariable and getInstanceState in a later PR
+// TODO: this is just a temporary type until we remove generics from getInstanceState in a later PR
 // we need to it satisfy the constraint of callers who specify VariableWithOptions or VariableWithMultiSupport
 type GenericVariableModel = TypedVariableModel | VariableWithOptions | VariableWithMultiSupport;
 
-export const getVariable = <T extends GenericVariableModel = GenericVariableModel>(
+export function getVariable(
+  identifier: KeyedVariableIdentifier,
+  state: StoreState,
+  throwWhenMissing: false
+): TypedVariableModel | undefined;
+export function getVariable(identifier: KeyedVariableIdentifier, state?: StoreState): TypedVariableModel;
+export function getVariable(
   identifier: KeyedVariableIdentifier,
   state: StoreState = getState(),
   throwWhenMissing = true
-): T => {
+): TypedVariableModel | undefined {
   const { id, rootStateKey } = identifier;
   const variablesState = getVariablesState(rootStateKey, state);
-  if (!variablesState.variables[id]) {
+  var variable = variablesState.variables[id];
+
+  if (!variable) {
     if (throwWhenMissing) {
       throw new Error(`Couldn't find variable with id:${id}`);
     }
-    return undefined as unknown as T;
+
+    return undefined;
   }
 
-  return variablesState.variables[id] as T;
-};
+  return variable;
+}
 
 function getFilteredVariablesByKey(
   filter: (model: TypedVariableModel) => boolean,

public/app/features/variables/textbox/actions.ts

@@ -7,7 +7,6 @@ import { toKeyedAction } from '../state/keyedVariablesReducer';
 import { getVariable } from '../state/selectors';
 import { changeVariableProp } from '../state/sharedReducer';
 import { KeyedVariableIdentifier } from '../state/types';
-import { TextBoxVariableModel } from '../types';
 import { ensureStringValues, toKeyedVariableIdentifier, toVariablePayload } from '../utils';
 
 import { createTextBoxOptions } from './reducer';
@@ -17,7 +16,10 @@ export const updateTextBoxVariableOptions = (identifier: KeyedVariableIdentifier
     const { rootStateKey, type } = identifier;
     dispatch(toKeyedAction(rootStateKey, createTextBoxOptions(toVariablePayload(identifier))));
 
-    const variableInState = getVariable<TextBoxVariableModel>(identifier, getState());
+    const variableInState = getVariable(identifier, getState());
+    if (variableInState.type !== 'textbox') {
+      return;
+    }
     await variableAdapters.get(type).setValue(variableInState, variableInState.options[0], true);
   };
 };
@@ -26,7 +28,10 @@ export const setTextBoxVariableOptionsFromUrl =
   (identifier: KeyedVariableIdentifier, urlValue: UrlQueryValue): ThunkResult<void> =>
   async (dispatch, getState) => {
     const { rootStateKey } = identifier;
-    const variableInState = getVariable<TextBoxVariableModel>(identifier, getState());
+    const variableInState = getVariable(identifier, getState());
+    if (variableInState.type !== 'textbox') {
+      return;
+    }
 
     const stringUrlValue = ensureStringValues(urlValue);
     dispatch(