From 8d92417a16f3d00ae0848406d14acce3ad51f7bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 19 Jul 2022 17:46:49 +0200 Subject: [PATCH] Scenes: Improve typing of scene state to avoid type guards and casting (#52422) * Trying to get rid of type guard but failing * Improve typing of scene object state * Fixed wrongly renamed event * Tweaks --- .betterer.results | 20 +++--------- .../scenes/components/NestedScene.tsx | 14 +++------ .../app/features/scenes/components/Scene.tsx | 4 +-- .../scenes/components/SceneCanvasText.tsx | 4 +-- .../scenes/components/SceneFlexLayout.tsx | 6 ++-- .../scenes/components/ScenePanelRepeater.tsx | 12 +++++-- .../scenes/components/SceneTimePicker.tsx | 4 +-- .../scenes/components/SceneToolbarButton.tsx | 6 ++-- .../features/scenes/components/VizPanel.tsx | 4 +-- .../app/features/scenes/core/SceneDataNode.ts | 4 +-- .../scenes/core/SceneObjectBase.test.ts | 8 ++--- .../features/scenes/core/SceneObjectBase.tsx | 12 +++---- public/app/features/scenes/core/events.ts | 10 +++--- public/app/features/scenes/core/types.ts | 31 ++++++++----------- .../scenes/editor/SceneObjectTree.tsx | 9 +++--- .../scenes/querying/SceneQueryRunner.ts | 4 +-- .../scenes/services/UrlSyncManager.ts | 11 +++---- .../scenes/variables/SceneVariableSet.test.ts | 4 +-- public/app/features/scenes/variables/types.ts | 6 ++-- 19 files changed, 77 insertions(+), 96 deletions(-) diff --git a/.betterer.results b/.betterer.results index 4d50cf774cc..643c4919068 100644 --- a/.betterer.results +++ b/.betterer.results @@ -5711,30 +5711,20 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "1"], [0, 0, 0, "Unexpected any. Specify a different type.", "2"], [0, 0, 0, "Do not use any type assertions.", "3"], - [0, 0, 0, "Do not use any type assertions.", "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.", "9"] + [0, 0, 0, "Unexpected any. Specify a different type.", "4"], + [0, 0, 0, "Do not use any type assertions.", "5"], + [0, 0, 0, "Unexpected any. Specify a different type.", "6"] ], "public/app/features/scenes/core/SceneTimeRange.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], - "public/app/features/scenes/core/events.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, "Unexpected any. Specify a different type.", "2"] - ], "public/app/features/scenes/core/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, "Unexpected any. Specify a different type.", "0"] ], "public/app/features/scenes/editor/SceneObjectTree.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Do not use any type assertions.", "2"] + [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], "public/app/features/scenes/querying/SceneQueryRunner.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/public/app/features/scenes/components/NestedScene.tsx b/public/app/features/scenes/components/NestedScene.tsx index b36d40d7b40..d6cdd716755 100644 --- a/public/app/features/scenes/components/NestedScene.tsx +++ b/public/app/features/scenes/components/NestedScene.tsx @@ -6,20 +6,14 @@ import { Stack } from '@grafana/experimental'; import { Button, ToolbarButton, useStyles2 } from '@grafana/ui'; import { SceneObjectBase } from '../core/SceneObjectBase'; -import { - SceneObject, - SceneObjectState, - SceneLayoutState, - SceneComponentProps, - isSceneLayoutObject, -} from '../core/types'; +import { SceneObject, SceneLayoutChildState, SceneComponentProps, SceneLayout } from '../core/types'; -interface NestedSceneState extends SceneObjectState { +interface NestedSceneState extends SceneLayoutChildState { title: string; isCollapsed?: boolean; canCollapse?: boolean; canRemove?: boolean; - layout: SceneObject; + layout: SceneLayout; actions?: SceneObject[]; } @@ -39,7 +33,7 @@ export class NestedScene extends SceneObjectBase { /** Removes itself from it's parent's children array */ onRemove = () => { const parent = this.parent!; - if (isSceneLayoutObject(parent)) { + if ('children' in parent.state) { parent.setState({ children: parent.state.children.filter((x) => x !== this), }); diff --git a/public/app/features/scenes/components/Scene.tsx b/public/app/features/scenes/components/Scene.tsx index 11a7f59a7f4..d81880e64fd 100644 --- a/public/app/features/scenes/components/Scene.tsx +++ b/public/app/features/scenes/components/Scene.tsx @@ -7,10 +7,10 @@ import { Page } from 'app/core/components/Page/Page'; import { PageLayoutType } from 'app/core/components/Page/types'; import { SceneObjectBase } from '../core/SceneObjectBase'; -import { SceneComponentProps, SceneObjectState, SceneObject } from '../core/types'; +import { SceneComponentProps, SceneObjectStatePlain, SceneObject } from '../core/types'; import { UrlSyncManager } from '../services/UrlSyncManager'; -interface SceneState extends SceneObjectState { +interface SceneState extends SceneObjectStatePlain { title: string; layout: SceneObject; actions?: SceneObject[]; diff --git a/public/app/features/scenes/components/SceneCanvasText.tsx b/public/app/features/scenes/components/SceneCanvasText.tsx index 042c86312e7..6f4e354a101 100644 --- a/public/app/features/scenes/components/SceneCanvasText.tsx +++ b/public/app/features/scenes/components/SceneCanvasText.tsx @@ -3,9 +3,9 @@ import React, { CSSProperties } from 'react'; import { Field, Input } from '@grafana/ui'; import { SceneObjectBase } from '../core/SceneObjectBase'; -import { SceneComponentProps, SceneObjectState } from '../core/types'; +import { SceneComponentProps, SceneLayoutChildState } from '../core/types'; -export interface SceneCanvasTextState extends SceneObjectState { +export interface SceneCanvasTextState extends SceneLayoutChildState { text: string; fontSize?: number; align?: 'left' | 'center' | 'right'; diff --git a/public/app/features/scenes/components/SceneFlexLayout.tsx b/public/app/features/scenes/components/SceneFlexLayout.tsx index 24691000c33..0b9cfa149e7 100644 --- a/public/app/features/scenes/components/SceneFlexLayout.tsx +++ b/public/app/features/scenes/components/SceneFlexLayout.tsx @@ -3,11 +3,11 @@ import React, { CSSProperties } from 'react'; import { Field, RadioButtonGroup } from '@grafana/ui'; import { SceneObjectBase } from '../core/SceneObjectBase'; -import { SceneObject, SceneObjectSize, SceneObjectState, SceneLayoutState, SceneComponentProps } from '../core/types'; +import { SceneObjectSize, SceneLayoutState, SceneComponentProps, SceneLayoutChild } from '../core/types'; export type FlexLayoutDirection = 'column' | 'row'; -interface SceneFlexLayoutState extends SceneObjectState, SceneLayoutState { +interface SceneFlexLayoutState extends SceneLayoutState { direction?: FlexLayoutDirection; } @@ -39,7 +39,7 @@ function FlexLayoutChildComponent({ direction, isEditing, }: { - item: SceneObject; + item: SceneLayoutChild; direction: FlexLayoutDirection; isEditing?: boolean; }) { diff --git a/public/app/features/scenes/components/ScenePanelRepeater.tsx b/public/app/features/scenes/components/ScenePanelRepeater.tsx index 81c3d3441e4..cce54e0d229 100644 --- a/public/app/features/scenes/components/ScenePanelRepeater.tsx +++ b/public/app/features/scenes/components/ScenePanelRepeater.tsx @@ -4,9 +4,15 @@ import { LoadingState, PanelData } from '@grafana/data'; import { SceneDataNode } from '../core/SceneDataNode'; import { SceneObjectBase } from '../core/SceneObjectBase'; -import { SceneComponentProps, SceneObject, SceneObjectList, SceneObjectState, SceneLayoutState } from '../core/types'; +import { + SceneComponentProps, + SceneObject, + SceneObjectStatePlain, + SceneLayoutState, + SceneLayoutChild, +} from '../core/types'; -interface RepeatOptions extends SceneObjectState { +interface RepeatOptions extends SceneObjectStatePlain { layout: SceneObject; } @@ -28,7 +34,7 @@ export class ScenePanelRepeater extends SceneObjectBase { performRepeat(data: PanelData) { // assume parent is a layout const firstChild = this.state.layout.state.children[0]!; - const newChildren: SceneObjectList = []; + const newChildren: SceneLayoutChild[] = []; for (const series of data.series) { const clone = firstChild.clone({ diff --git a/public/app/features/scenes/components/SceneTimePicker.tsx b/public/app/features/scenes/components/SceneTimePicker.tsx index 5ef53b264c5..7d2c15e9630 100644 --- a/public/app/features/scenes/components/SceneTimePicker.tsx +++ b/public/app/features/scenes/components/SceneTimePicker.tsx @@ -4,9 +4,9 @@ import { RefreshPicker, ToolbarButtonRow } from '@grafana/ui'; import { TimePickerWithHistory } from 'app/core/components/TimePicker/TimePickerWithHistory'; import { SceneObjectBase } from '../core/SceneObjectBase'; -import { SceneComponentProps, SceneObjectState } from '../core/types'; +import { SceneComponentProps, SceneObjectStatePlain } from '../core/types'; -export interface SceneTimePickerState extends SceneObjectState { +export interface SceneTimePickerState extends SceneObjectStatePlain { hidePicker?: boolean; } diff --git a/public/app/features/scenes/components/SceneToolbarButton.tsx b/public/app/features/scenes/components/SceneToolbarButton.tsx index 849fbfa1ef6..24f16ca44ef 100644 --- a/public/app/features/scenes/components/SceneToolbarButton.tsx +++ b/public/app/features/scenes/components/SceneToolbarButton.tsx @@ -3,9 +3,9 @@ import React from 'react'; import { IconName, Input, ToolbarButton } from '@grafana/ui'; import { SceneObjectBase } from '../core/SceneObjectBase'; -import { SceneComponentProps, SceneObjectState } from '../core/types'; +import { SceneComponentProps, SceneObjectStatePlain } from '../core/types'; -export interface ToolbarButtonState extends SceneObjectState { +export interface ToolbarButtonState extends SceneObjectStatePlain { icon: IconName; onClick: () => void; } @@ -18,7 +18,7 @@ export class SceneToolbarButton extends SceneObjectBase { }; } -export interface SceneToolbarInputState extends SceneObjectState { +export interface SceneToolbarInputState extends SceneObjectStatePlain { value?: string; onChange: (value: number) => void; } diff --git a/public/app/features/scenes/components/VizPanel.tsx b/public/app/features/scenes/components/VizPanel.tsx index dee9ef6865d..83fe0978327 100644 --- a/public/app/features/scenes/components/VizPanel.tsx +++ b/public/app/features/scenes/components/VizPanel.tsx @@ -6,9 +6,9 @@ import { PanelRenderer } from '@grafana/runtime'; import { Field, PanelChrome, Input } from '@grafana/ui'; import { SceneObjectBase } from '../core/SceneObjectBase'; -import { SceneComponentProps, SceneObjectState } from '../core/types'; +import { SceneComponentProps, SceneLayoutChildState } from '../core/types'; -export interface VizPanelState extends SceneObjectState { +export interface VizPanelState extends SceneLayoutChildState { title?: string; pluginId: string; options?: object; diff --git a/public/app/features/scenes/core/SceneDataNode.ts b/public/app/features/scenes/core/SceneDataNode.ts index adaf6b256e4..0506317b604 100644 --- a/public/app/features/scenes/core/SceneDataNode.ts +++ b/public/app/features/scenes/core/SceneDataNode.ts @@ -1,9 +1,9 @@ import { PanelData } from '@grafana/data'; import { SceneObjectBase } from './SceneObjectBase'; -import { SceneObjectState } from './types'; +import { SceneObjectStatePlain } from './types'; -export interface SceneDataNodeState extends SceneObjectState { +export interface SceneDataNodeState extends SceneObjectStatePlain { data?: PanelData; } diff --git a/public/app/features/scenes/core/SceneObjectBase.test.ts b/public/app/features/scenes/core/SceneObjectBase.test.ts index 2cfc82c73f1..39f83c25dd1 100644 --- a/public/app/features/scenes/core/SceneObjectBase.test.ts +++ b/public/app/features/scenes/core/SceneObjectBase.test.ts @@ -1,11 +1,11 @@ import { SceneObjectBase } from './SceneObjectBase'; -import { SceneObject, SceneObjectList, SceneObjectState } from './types'; +import { SceneLayoutChild, SceneObject, SceneObjectStatePlain } from './types'; -interface TestSceneState extends SceneObjectState { +interface TestSceneState extends SceneObjectStatePlain { name?: string; nested?: SceneObject; - children?: SceneObjectList; - actions?: SceneObjectList; + children?: SceneLayoutChild[]; + actions?: SceneObject[]; } class TestScene extends SceneObjectBase {} diff --git a/public/app/features/scenes/core/SceneObjectBase.tsx b/public/app/features/scenes/core/SceneObjectBase.tsx index 28bdd477b78..159338d9f1e 100644 --- a/public/app/features/scenes/core/SceneObjectBase.tsx +++ b/public/app/features/scenes/core/SceneObjectBase.tsx @@ -9,13 +9,12 @@ import { SceneObjectStateChangedEvent } from './events'; import { SceneDataState, SceneObject, - SceneLayoutState, - SceneObjectState, SceneComponent, SceneEditor, - SceneObjectList, SceneTimeRange, isSceneObject, + SceneObjectState, + SceneLayoutChild, } from './types'; export abstract class SceneObjectBase implements SceneObject { @@ -185,10 +184,9 @@ export abstract class SceneObjectBase impl } // Clone layout children - const layout = this.state as any as SceneLayoutState; - if (layout.children) { - const newChildren: SceneObjectList = []; - for (const child of layout.children) { + if ('children' in this.state) { + const newChildren: SceneLayoutChild[] = []; + for (const child of this.state.children) { newChildren.push(child.clone()); } (clonedState as any).children = newChildren; diff --git a/public/app/features/scenes/core/events.ts b/public/app/features/scenes/core/events.ts index a0113c03311..b447aa5a45d 100644 --- a/public/app/features/scenes/core/events.ts +++ b/public/app/features/scenes/core/events.ts @@ -1,12 +1,12 @@ import { BusEventWithPayload } from '@grafana/data'; -import { SceneObject } from './types'; +import { SceneObject, SceneObjectState, SceneObjectWithUrlSync } from './types'; export interface SceneObjectStateChangedPayload { - prevState: any; - newState: any; - partialUpdate: any; - changedObject: SceneObject; + prevState: SceneObjectState; + newState: SceneObjectState; + partialUpdate: Partial; + changedObject: SceneObject | SceneObjectWithUrlSync; } export class SceneObjectStateChangedEvent extends BusEventWithPayload { diff --git a/public/app/features/scenes/core/types.ts b/public/app/features/scenes/core/types.ts index 673bdb44490..8021435d317 100644 --- a/public/app/features/scenes/core/types.ts +++ b/public/app/features/scenes/core/types.ts @@ -5,15 +5,20 @@ import { EventBus, PanelData, TimeRange, UrlQueryMap } from '@grafana/data'; import { SceneVariableSet } from '../variables/types'; -export interface SceneObjectState { +export interface SceneObjectStatePlain { key?: string; - size?: SceneObjectSize; $timeRange?: SceneTimeRange; $data?: SceneObject; $editor?: SceneEditor; $variables?: SceneVariableSet; } +export interface SceneLayoutChildState extends SceneObjectStatePlain { + size?: SceneObjectSize; +} + +export type SceneObjectState = SceneObjectStatePlain | SceneLayoutState | SceneLayoutChildState; + export interface SceneObjectSize { width?: number | string; height?: number | string; @@ -32,7 +37,7 @@ export interface SceneComponentProps { export type SceneComponent = React.FunctionComponent>; -export interface SceneDataState extends SceneObjectState { +export interface SceneDataState extends SceneObjectStatePlain { data?: PanelData; } @@ -74,15 +79,15 @@ export interface SceneObject Editor(props: SceneComponentProps>): React.ReactElement | null; } -export type SceneObjectList = Array>; +export type SceneLayoutChild = SceneObject; -export interface SceneLayoutState extends SceneObjectState { - children: SceneObjectList; +export interface SceneLayoutState extends SceneLayoutChildState { + children: SceneLayoutChild[]; } export type SceneLayout = SceneObject; -export interface SceneEditorState extends SceneObjectState { +export interface SceneEditorState extends SceneObjectStatePlain { hoverObject?: SceneObjectRef; selectedObject?: SceneObjectRef; } @@ -93,7 +98,7 @@ export interface SceneEditor extends SceneObject { onSelectObject(model: SceneObject): void; } -export interface SceneTimeRangeState extends SceneObjectState, TimeRange {} +export interface SceneTimeRangeState extends SceneObjectStatePlain, TimeRange {} export interface SceneTimeRange extends SceneObject { onTimeRangeChange(timeRange: TimeRange): void; onIntervalChanged(interval: string): void; @@ -113,13 +118,3 @@ export interface SceneObjectWithUrlSync extends SceneObject { getUrlState(): UrlQueryMap; updateFromUrl(values: UrlQueryMap): void; } - -export function isSceneObjectWithUrlSync(obj: any): obj is SceneObjectWithUrlSync { - return obj.getUrlState !== undefined; -} - -export function isSceneLayoutObject( - obj: SceneObject -): obj is SceneObject { - return 'children' in obj.state && obj.state.children !== undefined; -} diff --git a/public/app/features/scenes/editor/SceneObjectTree.tsx b/public/app/features/scenes/editor/SceneObjectTree.tsx index 13c33b42e80..c167300afae 100644 --- a/public/app/features/scenes/editor/SceneObjectTree.tsx +++ b/public/app/features/scenes/editor/SceneObjectTree.tsx @@ -4,7 +4,7 @@ import React from 'react'; import { GrafanaTheme2 } from '@grafana/data'; import { Icon, useStyles2 } from '@grafana/ui'; -import { SceneObject, SceneLayoutState, SceneObjectList, isSceneObject } from '../core/types'; +import { SceneObject, isSceneObject, SceneLayoutChild } from '../core/types'; export interface Props { node: SceneObject; @@ -14,7 +14,7 @@ export interface Props { export function SceneObjectTree({ node, selectedObject }: Props) { const styles = useStyles2(getStyles); const state = node.useState(); - let children: SceneObjectList = []; + let children: SceneLayoutChild[] = []; for (const propKey of Object.keys(state)) { const propValue = (state as any)[propKey]; @@ -23,9 +23,8 @@ export function SceneObjectTree({ node, selectedObject }: Props) { } } - let layoutChildren = (state as SceneLayoutState).children; - if (layoutChildren) { - for (const child of layoutChildren) { + if ('children' in state) { + for (const child of state.children) { children.push(child); } } diff --git a/public/app/features/scenes/querying/SceneQueryRunner.ts b/public/app/features/scenes/querying/SceneQueryRunner.ts index 3d006885d3b..dadbf7524f0 100644 --- a/public/app/features/scenes/querying/SceneQueryRunner.ts +++ b/public/app/features/scenes/querying/SceneQueryRunner.ts @@ -17,9 +17,9 @@ import { getNextRequestId } from 'app/features/query/state/PanelQueryRunner'; import { runRequest } from 'app/features/query/state/runRequest'; import { SceneObjectBase } from '../core/SceneObjectBase'; -import { SceneObjectState } from '../core/types'; +import { SceneObjectStatePlain } from '../core/types'; -export interface QueryRunnerState extends SceneObjectState { +export interface QueryRunnerState extends SceneObjectStatePlain { data?: PanelData; queries: DataQueryExtended[]; } diff --git a/public/app/features/scenes/services/UrlSyncManager.ts b/public/app/features/scenes/services/UrlSyncManager.ts index b065d90ef49..83c55e635c3 100644 --- a/public/app/features/scenes/services/UrlSyncManager.ts +++ b/public/app/features/scenes/services/UrlSyncManager.ts @@ -4,7 +4,7 @@ import { Unsubscribable } from 'rxjs'; import { locationService } from '@grafana/runtime'; import { SceneObjectStateChangedEvent } from '../core/events'; -import { isSceneObjectWithUrlSync, SceneObject } from '../core/types'; +import { SceneObject } from '../core/types'; export class UrlSyncManager { private locationListenerUnsub: () => void; @@ -21,12 +21,11 @@ export class UrlSyncManager { onStateChanged = ({ payload }: SceneObjectStateChangedEvent) => { const changedObject = payload.changedObject; - if (!isSceneObjectWithUrlSync(changedObject)) { - return; - } - const urlUpdate = changedObject.getUrlState(); - locationService.partial(urlUpdate, true); + if ('getUrlState' in changedObject) { + const urlUpdate = changedObject.getUrlState(); + locationService.partial(urlUpdate, true); + } }; cleanUp() { diff --git a/public/app/features/scenes/variables/SceneVariableSet.test.ts b/public/app/features/scenes/variables/SceneVariableSet.test.ts index 230b49545d4..d1f9f927125 100644 --- a/public/app/features/scenes/variables/SceneVariableSet.test.ts +++ b/public/app/features/scenes/variables/SceneVariableSet.test.ts @@ -1,9 +1,9 @@ import { SceneObjectBase } from '../core/SceneObjectBase'; -import { SceneObjectState } from '../core/types'; +import { SceneObjectStatePlain } from '../core/types'; import { sceneTemplateInterpolator, SceneVariableManager, TextBoxSceneVariable } from './SceneVariableSet'; -interface TestSceneState extends SceneObjectState { +interface TestSceneState extends SceneObjectStatePlain { nested?: TestScene; } diff --git a/public/app/features/scenes/variables/types.ts b/public/app/features/scenes/variables/types.ts index b3b80e3fcd5..8cd1cbdc59a 100644 --- a/public/app/features/scenes/variables/types.ts +++ b/public/app/features/scenes/variables/types.ts @@ -1,9 +1,9 @@ import { LoadingState } from '@grafana/data'; import { VariableHide } from 'app/features/variables/types'; -import { SceneObject, SceneObjectState } from '../core/types'; +import { SceneObject, SceneObjectStatePlain } from '../core/types'; -export interface SceneVariableState extends SceneObjectState { +export interface SceneVariableState extends SceneObjectStatePlain { name: string; hide?: VariableHide; skipUrlSync?: boolean; @@ -15,7 +15,7 @@ export interface SceneVariableState extends SceneObjectState { export interface SceneVariable extends SceneObject {} -export interface SceneVariableSetState extends SceneObjectState { +export interface SceneVariableSetState extends SceneObjectStatePlain { variables: SceneVariable[]; }