From 1fb37b54b358233ffe5fcafe2bc5e9d7408dfd19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 7 Nov 2022 15:32:02 +0100 Subject: [PATCH] Scenes: Enforce explicit accessibility modifiers (ESLint) (#58331) * Public test * Update * Update * revert * Added some public accessability modifiers * Force public acessability only for scenes/* folder * Fixes --- .eslintrc | 6 +++ .../scenes/components/NestedScene.tsx | 6 +-- .../app/features/scenes/components/Scene.tsx | 8 +-- .../scenes/components/SceneCanvasText.tsx | 4 +- .../scenes/components/SceneFlexLayout.tsx | 6 +-- .../scenes/components/ScenePanelRepeater.tsx | 8 +-- .../scenes/components/SceneTimePicker.tsx | 2 +- .../scenes/components/SceneToolbarButton.tsx | 4 +- .../features/scenes/components/VizPanel.tsx | 6 +-- .../features/scenes/core/SceneObjectBase.tsx | 51 +++++++++++-------- .../features/scenes/core/SceneTimeRange.tsx | 10 ++-- public/app/features/scenes/core/events.ts | 9 +--- .../scenes/editor/SceneEditManager.tsx | 10 ++-- .../scenes/querying/SceneQueryRunner.ts | 8 +-- .../scenes/services/UrlSyncManager.ts | 8 +-- .../scenes/variables/SceneVariableSet.ts | 2 +- 16 files changed, 78 insertions(+), 70 deletions(-) diff --git a/.eslintrc b/.eslintrc index 0ffbc3f1dbf..9535a971dd8 100644 --- a/.eslintrc +++ b/.eslintrc @@ -44,6 +44,12 @@ "@typescript-eslint/no-redeclare": ["error"] }, "overrides": [ + { + "files": ["public/app/features/scenes/**/*.{ts,tsx}"], + "rules": { + "@typescript-eslint/explicit-member-accessibility": ["error", { "accessibility": "explicit" }] + } + }, { "files": ["packages/grafana-ui/src/components/uPlot/**/*.{ts,tsx}"], "rules": { diff --git a/public/app/features/scenes/components/NestedScene.tsx b/public/app/features/scenes/components/NestedScene.tsx index a6b027bce03..6ee7053f3a9 100644 --- a/public/app/features/scenes/components/NestedScene.tsx +++ b/public/app/features/scenes/components/NestedScene.tsx @@ -18,9 +18,9 @@ interface NestedSceneState extends SceneLayoutChildState { } export class NestedScene extends SceneObjectBase { - static Component = NestedSceneRenderer; + public static Component = NestedSceneRenderer; - onToggle = () => { + public onToggle = () => { this.setState({ isCollapsed: !this.state.isCollapsed, size: { @@ -31,7 +31,7 @@ export class NestedScene extends SceneObjectBase { }; /** Removes itself from its parent's children array */ - onRemove = () => { + public onRemove = () => { const parent = this.parent!; if ('children' in parent.state) { parent.setState({ diff --git a/public/app/features/scenes/components/Scene.tsx b/public/app/features/scenes/components/Scene.tsx index 9e012430b53..2848d2d2a8e 100644 --- a/public/app/features/scenes/components/Scene.tsx +++ b/public/app/features/scenes/components/Scene.tsx @@ -18,15 +18,15 @@ interface SceneState extends SceneObjectStatePlain { } export class Scene extends SceneObjectBase { - static Component = SceneRenderer; - urlSyncManager?: UrlSyncManager; + public static Component = SceneRenderer; + private urlSyncManager?: UrlSyncManager; - activate() { + public activate() { super.activate(); this.urlSyncManager = new UrlSyncManager(this); } - deactivate() { + public deactivate() { super.deactivate(); this.urlSyncManager!.cleanUp(); } diff --git a/public/app/features/scenes/components/SceneCanvasText.tsx b/public/app/features/scenes/components/SceneCanvasText.tsx index 6f4e354a101..aee61d0dc85 100644 --- a/public/app/features/scenes/components/SceneCanvasText.tsx +++ b/public/app/features/scenes/components/SceneCanvasText.tsx @@ -12,8 +12,8 @@ export interface SceneCanvasTextState extends SceneLayoutChildState { } export class SceneCanvasText extends SceneObjectBase { - static Editor = Editor; - static Component = ({ model }: SceneComponentProps) => { + public static Editor = Editor; + public static Component = ({ model }: SceneComponentProps) => { const { text, fontSize = 20, align = 'left' } = model.useState(); const style: CSSProperties = { diff --git a/public/app/features/scenes/components/SceneFlexLayout.tsx b/public/app/features/scenes/components/SceneFlexLayout.tsx index 0b9cfa149e7..cf6ec2e3dca 100644 --- a/public/app/features/scenes/components/SceneFlexLayout.tsx +++ b/public/app/features/scenes/components/SceneFlexLayout.tsx @@ -12,10 +12,10 @@ interface SceneFlexLayoutState extends SceneLayoutState { } export class SceneFlexLayout extends SceneObjectBase { - static Component = FlexLayoutRenderer; - static Editor = FlexLayoutEditor; + public static Component = FlexLayoutRenderer; + public static Editor = FlexLayoutEditor; - toggleDirection() { + public toggleDirection() { this.setState({ direction: this.state.direction === 'row' ? 'column' : 'row', }); diff --git a/public/app/features/scenes/components/ScenePanelRepeater.tsx b/public/app/features/scenes/components/ScenePanelRepeater.tsx index 89d6f62da73..26063a64211 100644 --- a/public/app/features/scenes/components/ScenePanelRepeater.tsx +++ b/public/app/features/scenes/components/ScenePanelRepeater.tsx @@ -17,10 +17,10 @@ interface RepeatOptions extends SceneObjectStatePlain { } export class ScenePanelRepeater extends SceneObjectBase { - activate(): void { + public activate(): void { super.activate(); - this.subs.add( + this._subs.add( this.getData().subscribeToState({ next: (data) => { if (data.data?.state === LoadingState.Done) { @@ -31,7 +31,7 @@ export class ScenePanelRepeater extends SceneObjectBase { ); } - performRepeat(data: PanelData) { + private performRepeat(data: PanelData) { // assume parent is a layout const firstChild = this.state.layout.state.children[0]!; const newChildren: SceneLayoutChild[] = []; @@ -53,7 +53,7 @@ export class ScenePanelRepeater extends SceneObjectBase { this.state.layout.setState({ children: newChildren }); } - static Component = ({ model, isEditing }: SceneComponentProps) => { + public static Component = ({ model, isEditing }: SceneComponentProps) => { const { layout } = model.useState(); return ; }; diff --git a/public/app/features/scenes/components/SceneTimePicker.tsx b/public/app/features/scenes/components/SceneTimePicker.tsx index 09640d778c7..ad6b8281d9d 100644 --- a/public/app/features/scenes/components/SceneTimePicker.tsx +++ b/public/app/features/scenes/components/SceneTimePicker.tsx @@ -11,7 +11,7 @@ export interface SceneTimePickerState extends SceneObjectStatePlain { } export class SceneTimePicker extends SceneObjectBase { - static Component = SceneTimePickerRenderer; + public static Component = SceneTimePickerRenderer; } function SceneTimePickerRenderer({ model }: SceneComponentProps) { diff --git a/public/app/features/scenes/components/SceneToolbarButton.tsx b/public/app/features/scenes/components/SceneToolbarButton.tsx index 24f16ca44ef..abe4ed53298 100644 --- a/public/app/features/scenes/components/SceneToolbarButton.tsx +++ b/public/app/features/scenes/components/SceneToolbarButton.tsx @@ -11,7 +11,7 @@ export interface ToolbarButtonState extends SceneObjectStatePlain { } export class SceneToolbarButton extends SceneObjectBase { - static Component = ({ model }: SceneComponentProps) => { + public static Component = ({ model }: SceneComponentProps) => { const state = model.useState(); return ; @@ -24,7 +24,7 @@ export interface SceneToolbarInputState extends SceneObjectStatePlain { } export class SceneToolbarInput extends SceneObjectBase { - static Component = ({ model }: SceneComponentProps) => { + public static Component = ({ model }: SceneComponentProps) => { const state = model.useState(); return ( diff --git a/public/app/features/scenes/components/VizPanel.tsx b/public/app/features/scenes/components/VizPanel.tsx index 83fe0978327..764a5741c1f 100644 --- a/public/app/features/scenes/components/VizPanel.tsx +++ b/public/app/features/scenes/components/VizPanel.tsx @@ -16,10 +16,10 @@ export interface VizPanelState extends SceneLayoutChildState { } export class VizPanel extends SceneObjectBase { - static Component = ScenePanelRenderer; - static Editor = VizPanelEditor; + public static Component = ScenePanelRenderer; + public static Editor = VizPanelEditor; - onSetTimeRange = (timeRange: AbsoluteTimeRange) => { + public onSetTimeRange = (timeRange: AbsoluteTimeRange) => { const sceneTimeRange = this.getTimeRange(); sceneTimeRange.setState({ raw: { diff --git a/public/app/features/scenes/core/SceneObjectBase.tsx b/public/app/features/scenes/core/SceneObjectBase.tsx index 8cfe7065720..d5871def18b 100644 --- a/public/app/features/scenes/core/SceneObjectBase.tsx +++ b/public/app/features/scenes/core/SceneObjectBase.tsx @@ -16,9 +16,9 @@ export abstract class SceneObjectBase impl private _events = new EventBusSrv(); protected _parent?: SceneObject; - protected subs = new Subscription(); + protected _subs = new Subscription(); - constructor(state: TState) { + public constructor(state: TState) { if (!state.key) { state.key = uuidv4(); } @@ -29,17 +29,17 @@ export abstract class SceneObjectBase impl } /** Current state */ - get state(): TState { + public get state(): TState { return this._state; } /** True if currently being active (ie displayed for visual objects) */ - get isActive(): boolean { + public get isActive(): boolean { return this._isActive; } /** Returns the parent, undefined for root object */ - get parent(): SceneObject | undefined { + public get parent(): SceneObject | undefined { return this._parent; } @@ -47,14 +47,14 @@ export abstract class SceneObjectBase impl * Used in render functions when rendering a SceneObject. * Wraps the component in an EditWrapper that handles edit mode */ - get Component(): SceneComponent { + public get Component(): SceneComponent { return SceneComponentWrapper; } /** * Temporary solution, should be replaced by declarative options */ - get Editor(): SceneComponent { + public get Editor(): SceneComponent { return ((this as any).constructor['Editor'] ?? (() => null)) as SceneComponent; } @@ -77,18 +77,18 @@ export abstract class SceneObjectBase impl /** * Subscribe to the scene state subject **/ - subscribeToState(observerOrNext?: Partial>): Subscription { + public subscribeToState(observerOrNext?: Partial>): Subscription { return this._subject.subscribe(observerOrNext); } /** * Subscribe to the scene event **/ - subscribeToEvent(eventType: BusEventType, handler: BusEventHandler): Unsubscribable { + public subscribeToEvent(eventType: BusEventType, handler: BusEventHandler): Unsubscribable { return this._events.subscribe(eventType, handler); } - setState(update: Partial) { + public setState(update: Partial) { const prevState = this._state; this._state = { ...this._state, @@ -112,7 +112,7 @@ export abstract class SceneObjectBase impl /* * Publish an event and optionally bubble it up the scene **/ - publishEvent(event: BusEvent, bubble?: boolean) { + public publishEvent(event: BusEvent, bubble?: boolean) { this._events.publish(event); if (bubble && this.parent) { @@ -120,11 +120,14 @@ export abstract class SceneObjectBase impl } } - getRoot(): SceneObject { + public getRoot(): SceneObject { return !this._parent ? this : this._parent.getRoot(); } - activate() { + /** + * Called by the SceneComponentWrapper when the react component is mounted + */ + public activate() { this._isActive = true; const { $data, $variables } = this.state; @@ -138,7 +141,10 @@ export abstract class SceneObjectBase impl } } - deactivate(): void { + /** + * Called by the SceneComponentWrapper when the react component is unmounted + */ + public deactivate(): void { this._isActive = false; const { $data, $variables } = this.state; @@ -153,14 +159,17 @@ export abstract class SceneObjectBase impl // Clear subscriptions and listeners this._events.removeAllListeners(); - this.subs.unsubscribe(); - this.subs = new Subscription(); + this._subs.unsubscribe(); + this._subs = new Subscription(); this._subject.complete(); this._subject = new Subject(); } - useState() { + /** + * Utility hook to get and subscribe to state + */ + public useState() { // eslint-disable-next-line react-hooks/rules-of-hooks return useSceneObjectState(this); } @@ -168,7 +177,7 @@ export abstract class SceneObjectBase impl /** * Will walk up the scene object graph to the closest $timeRange scene object */ - getTimeRange(): SceneTimeRange { + public getTimeRange(): SceneTimeRange { const { $timeRange } = this.state; if ($timeRange) { return $timeRange; @@ -184,7 +193,7 @@ export abstract class SceneObjectBase impl /** * Will walk up the scene object graph to the closest $data scene object */ - getData(): SceneObject { + public getData(): SceneObject { const { $data } = this.state; if ($data) { return $data; @@ -200,7 +209,7 @@ export abstract class SceneObjectBase impl /** * Will walk up the scene object graph to the closest $editor scene object */ - getSceneEditor(): SceneEditor { + public getSceneEditor(): SceneEditor { const { $editor } = this.state; if ($editor) { return $editor; @@ -216,7 +225,7 @@ export abstract class SceneObjectBase impl /** * Will create new SceneItem with shalled cloned state, but all states items of type SceneObject are deep cloned */ - clone(withState?: Partial): this { + public clone(withState?: Partial): this { const clonedState = { ...this.state }; // Clone any SceneItems in state diff --git a/public/app/features/scenes/core/SceneTimeRange.tsx b/public/app/features/scenes/core/SceneTimeRange.tsx index 595db0d864b..596b2337637 100644 --- a/public/app/features/scenes/core/SceneTimeRange.tsx +++ b/public/app/features/scenes/core/SceneTimeRange.tsx @@ -4,26 +4,26 @@ import { SceneObjectBase } from './SceneObjectBase'; import { SceneObjectWithUrlSync, SceneTimeRangeState } from './types'; export class SceneTimeRange extends SceneObjectBase implements SceneObjectWithUrlSync { - onTimeRangeChange = (timeRange: TimeRange) => { + public onTimeRangeChange = (timeRange: TimeRange) => { this.setState(timeRange); }; - onRefresh = () => { + public onRefresh = () => { // TODO re-eval time range this.setState({ ...this.state }); }; - onIntervalChanged = (_: string) => {}; + public onIntervalChanged = (_: string) => {}; /** These url sync functions are only placeholders for something more sophisticated */ - getUrlState() { + public getUrlState() { return { from: this.state.raw.from, to: this.state.raw.to, } as any; } - updateFromUrl(values: UrlQueryMap) { + public updateFromUrl(values: UrlQueryMap) { // TODO } } diff --git a/public/app/features/scenes/core/events.ts b/public/app/features/scenes/core/events.ts index ab221616554..650bac14759 100644 --- a/public/app/features/scenes/core/events.ts +++ b/public/app/features/scenes/core/events.ts @@ -10,12 +10,5 @@ export interface SceneObjectStateChangedPayload { } export class SceneObjectStateChangedEvent extends BusEventWithPayload { - static type = 'scene-object-state-change'; -} - -export class SceneObjectActivedEvent extends BusEventWithPayload { - static type = 'scene-object-activated'; -} -export class SceneObjectDeactivatedEvent extends BusEventWithPayload { - static type = 'scene-object-deactivated'; + public static readonly type = 'scene-object-state-change'; } diff --git a/public/app/features/scenes/editor/SceneEditManager.tsx b/public/app/features/scenes/editor/SceneEditManager.tsx index bfa22349e71..f7fea9e0bc9 100644 --- a/public/app/features/scenes/editor/SceneEditManager.tsx +++ b/public/app/features/scenes/editor/SceneEditManager.tsx @@ -11,17 +11,17 @@ import { SceneObjectEditor } from './SceneObjectEditor'; import { SceneObjectTree } from './SceneObjectTree'; export class SceneEditManager extends SceneObjectBase implements SceneEditor { - static Component = SceneEditorRenderer; + public static Component = SceneEditorRenderer; - get Component(): SceneComponent { + public get Component(): SceneComponent { return SceneEditorRenderer; } - onMouseEnterObject(model: SceneObject) { + public onMouseEnterObject(model: SceneObject) { this.setState({ hoverObject: { ref: model } }); } - onMouseLeaveObject(model: SceneObject) { + public onMouseLeaveObject(model: SceneObject) { if (model.parent) { this.setState({ hoverObject: { ref: model.parent } }); } else { @@ -29,7 +29,7 @@ export class SceneEditManager extends SceneObjectBase implemen } } - onSelectObject(model: SceneObject) { + public onSelectObject(model: SceneObject) { this.setState({ selectedObject: { ref: model } }); } } diff --git a/public/app/features/scenes/querying/SceneQueryRunner.ts b/public/app/features/scenes/querying/SceneQueryRunner.ts index 81f644cfadb..fed9e351f59 100644 --- a/public/app/features/scenes/querying/SceneQueryRunner.ts +++ b/public/app/features/scenes/querying/SceneQueryRunner.ts @@ -31,12 +31,12 @@ export interface DataQueryExtended extends DataQuery { export class SceneQueryRunner extends SceneObjectBase { private querySub?: Unsubscribable; - activate() { + public activate() { super.activate(); const timeRange = this.getTimeRange(); - this.subs.add( + this._subs.add( timeRange.subscribeToState({ next: (timeRange) => { this.runWithTimeRange(timeRange); @@ -49,7 +49,7 @@ export class SceneQueryRunner extends SceneObjectBase { } } - deactivate(): void { + public deactivate(): void { super.deactivate(); if (this.querySub) { @@ -58,7 +58,7 @@ export class SceneQueryRunner extends SceneObjectBase { } } - runQueries() { + public runQueries() { const timeRange = this.getTimeRange(); this.runWithTimeRange(timeRange.state); } diff --git a/public/app/features/scenes/services/UrlSyncManager.ts b/public/app/features/scenes/services/UrlSyncManager.ts index bb20cacc082..23c5ad7e184 100644 --- a/public/app/features/scenes/services/UrlSyncManager.ts +++ b/public/app/features/scenes/services/UrlSyncManager.ts @@ -10,16 +10,16 @@ export class UrlSyncManager { private locationListenerUnsub: () => void; private stateChangeSub: Unsubscribable; - constructor(sceneRoot: SceneObject) { + public constructor(sceneRoot: SceneObject) { this.stateChangeSub = sceneRoot.subscribeToEvent(SceneObjectStateChangedEvent, this.onStateChanged); this.locationListenerUnsub = locationService.getHistory().listen(this.onLocationUpdate); } - onLocationUpdate = (location: Location) => { + private onLocationUpdate = (location: Location) => { // TODO: find any scene object whose state we need to update }; - onStateChanged = ({ payload }: SceneObjectStateChangedEvent) => { + private onStateChanged = ({ payload }: SceneObjectStateChangedEvent) => { const changedObject = payload.changedObject; if ('getUrlState' in changedObject) { @@ -28,7 +28,7 @@ export class UrlSyncManager { } }; - cleanUp() { + public cleanUp() { this.stateChangeSub.unsubscribe(); this.locationListenerUnsub(); } diff --git a/public/app/features/scenes/variables/SceneVariableSet.ts b/public/app/features/scenes/variables/SceneVariableSet.ts index d7f90d8c777..c56c1f15e1b 100644 --- a/public/app/features/scenes/variables/SceneVariableSet.ts +++ b/public/app/features/scenes/variables/SceneVariableSet.ts @@ -8,7 +8,7 @@ import { SceneVariable, SceneVariables, SceneVariableSetState, SceneVariableStat export class TextBoxSceneVariable extends SceneObjectBase implements SceneVariable {} export class SceneVariableSet extends SceneObjectBase implements SceneVariables { - getVariableByName(name: string): SceneVariable | undefined { + public getVariableByName(name: string): SceneVariable | undefined { // TODO: Replace with index return this.state.variables.find((x) => x.state.name === name); }