diff --git a/.betterer.results b/.betterer.results index 446992a0562..6c3ec87ca59 100644 --- a/.betterer.results +++ b/.betterer.results @@ -4661,14 +4661,16 @@ exports[`better eslint`] = { [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/SceneObjectBase.test.ts:5381": [ + [0, 0, 0, "Unexpected any. Specify a different type.", "0"] + ], "public/app/features/scenes/core/SceneObjectBase.tsx: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, "Do not use any type assertions.", "3"], - [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"] + [0, 0, 0, "Unexpected any. Specify a different type.", "3"], + [0, 0, 0, "Do not use any type assertions.", "4"], + [0, 0, 0, "Unexpected any. Specify a different type.", "5"] ], "public/app/features/scenes/core/SceneTimeRange.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], diff --git a/packages/grafana-data/src/events/EventBus.test.ts b/packages/grafana-data/src/events/EventBus.test.ts index be189e409c5..10826bdda9f 100644 --- a/packages/grafana-data/src/events/EventBus.test.ts +++ b/packages/grafana-data/src/events/EventBus.test.ts @@ -181,15 +181,18 @@ describe('EventBus', () => { it('removeAllListeners should unsubscribe to all', () => { const bus = new EventBusSrv(); const events: LoginEvent[] = []; + let completed = false; - bus.subscribe(LoginEvent, (event) => { - events.push(event); + bus.getStream(LoginEvent).subscribe({ + next: (evt) => events.push(evt), + complete: () => (completed = true), }); bus.removeAllListeners(); bus.publish(new LoginEvent({ logins: 10 })); expect(events.length).toBe(0); + expect(completed).toBe(true); }); }); }); diff --git a/packages/grafana-data/src/events/EventBus.ts b/packages/grafana-data/src/events/EventBus.ts index ca498274da7..4296d98c668 100644 --- a/packages/grafana-data/src/events/EventBus.ts +++ b/packages/grafana-data/src/events/EventBus.ts @@ -1,5 +1,5 @@ import EventEmitter from 'eventemitter3'; -import { Unsubscribable, Observable } from 'rxjs'; +import { Unsubscribable, Observable, Subscriber } from 'rxjs'; import { filter } from 'rxjs/operators'; import { @@ -18,6 +18,7 @@ import { */ export class EventBusSrv implements EventBus, LegacyEmitter { private emitter: EventEmitter; + private subscribers = new Map>(); constructor() { this.emitter = new EventEmitter(); @@ -31,16 +32,18 @@ export class EventBusSrv implements EventBus, LegacyEmitter { return this.getStream(typeFilter).subscribe({ next: handler }); } - getStream(eventType: BusEventType): Observable { + getStream(eventType: BusEventType): Observable { return new Observable((observer) => { const handler = (event: T) => { observer.next(event); }; this.emitter.on(eventType.type, handler); + this.subscribers.set(handler, observer); return () => { this.emitter.off(eventType.type, handler); + this.subscribers.delete(handler); }; }); } @@ -95,6 +98,10 @@ export class EventBusSrv implements EventBus, LegacyEmitter { removeAllListeners() { this.emitter.removeAllListeners(); + for (const [key, sub] of this.subscribers) { + sub.complete(); + this.subscribers.delete(key); + } } } diff --git a/packages/grafana-data/src/events/types.ts b/packages/grafana-data/src/events/types.ts index 6f54cd65218..ce7d89f17c1 100644 --- a/packages/grafana-data/src/events/types.ts +++ b/packages/grafana-data/src/events/types.ts @@ -68,7 +68,7 @@ export interface EventFilterOptions { */ export interface EventBus { /** - * Publish single vent + * Publish single event */ publish(event: T): void; diff --git a/public/app/features/scenes/components/ScenePanelRepeater.tsx b/public/app/features/scenes/components/ScenePanelRepeater.tsx index cce54e0d229..89d6f62da73 100644 --- a/public/app/features/scenes/components/ScenePanelRepeater.tsx +++ b/public/app/features/scenes/components/ScenePanelRepeater.tsx @@ -21,7 +21,7 @@ export class ScenePanelRepeater extends SceneObjectBase { super.activate(); this.subs.add( - this.getData().subscribe({ + this.getData().subscribeToState({ next: (data) => { if (data.data?.state === LoadingState.Done) { this.performRepeat(data.data); diff --git a/public/app/features/scenes/core/SceneObjectBase.test.ts b/public/app/features/scenes/core/SceneObjectBase.test.ts index 39f83c25dd1..17c86014163 100644 --- a/public/app/features/scenes/core/SceneObjectBase.test.ts +++ b/public/app/features/scenes/core/SceneObjectBase.test.ts @@ -1,4 +1,8 @@ +import { SceneVariableSet } from '../variables/SceneVariableSet'; + +import { SceneDataNode } from './SceneDataNode'; import { SceneObjectBase } from './SceneObjectBase'; +import { SceneObjectStateChangedEvent } from './events'; import { SceneLayoutChild, SceneObject, SceneObjectStatePlain } from './types'; interface TestSceneState extends SceneObjectStatePlain { @@ -16,6 +20,11 @@ describe('SceneObject', () => { nested: new TestScene({ name: 'nested', }), + actions: [ + new TestScene({ + name: 'action child', + }), + ], children: [ new TestScene({ name: 'layout child', @@ -28,8 +37,9 @@ describe('SceneObject', () => { const clone = scene.clone(); expect(clone).not.toBe(scene); expect(clone.state.nested).not.toBe(scene.state.nested); - expect(clone.state.nested?.isActive).toBe(undefined); + expect(clone.state.nested?.isActive).toBe(false); expect(clone.state.children![0]).not.toBe(scene.state.children![0]); + expect(clone.state.actions![0]).not.toBe(scene.state.actions![0]); }); it('SceneObject should have parent when added to container', () => { @@ -65,4 +75,57 @@ describe('SceneObject', () => { const clone = scene.clone({ name: 'new name' }); expect(clone.state.name).toBe('new name'); }); + + describe('When activated', () => { + const scene = new TestScene({ + $data: new SceneDataNode({}), + $variables: new SceneVariableSet({ variables: [] }), + }); + + scene.activate(); + + it('Should set isActive true', () => { + expect(scene.isActive).toBe(true); + }); + + it('Should activate $data', () => { + expect(scene.state.$data!.isActive).toBe(true); + }); + + it('Should activate $variables', () => { + expect(scene.state.$variables!.isActive).toBe(true); + }); + }); + + describe('When deactivated', () => { + const scene = new TestScene({ + $data: new SceneDataNode({}), + $variables: new SceneVariableSet({ variables: [] }), + }); + + scene.activate(); + + // Subscribe to state change and to event + const stateSub = scene.subscribeToState({ next: () => {} }); + const eventSub = scene.subscribeToEvent(SceneObjectStateChangedEvent, () => {}); + + scene.deactivate(); + + it('Should close subscriptions', () => { + expect(stateSub.closed).toBe(true); + expect((eventSub as any).closed).toBe(true); + }); + + it('Should set isActive false', () => { + expect(scene.isActive).toBe(false); + }); + + it('Should deactivate $data', () => { + expect(scene.state.$data!.isActive).toBe(false); + }); + + it('Should deactivate $variables', () => { + expect(scene.state.$variables!.isActive).toBe(false); + }); + }); }); diff --git a/public/app/features/scenes/core/SceneObjectBase.tsx b/public/app/features/scenes/core/SceneObjectBase.tsx index 9596f196e44..8cfe7065720 100644 --- a/public/app/features/scenes/core/SceneObjectBase.tsx +++ b/public/app/features/scenes/core/SceneObjectBase.tsx @@ -1,41 +1,48 @@ import { useEffect } from 'react'; -import { Observer, Subject, Subscription } from 'rxjs'; +import { Observer, Subject, Subscription, Unsubscribable } from 'rxjs'; import { v4 as uuidv4 } from 'uuid'; -import { EventBusSrv } from '@grafana/data'; +import { BusEvent, BusEventHandler, BusEventType, EventBusSrv } from '@grafana/data'; import { useForceUpdate } from '@grafana/ui'; import { SceneComponentWrapper } from './SceneComponentWrapper'; import { SceneObjectStateChangedEvent } from './events'; -import { - SceneDataState, - SceneObject, - SceneComponent, - SceneEditor, - SceneTimeRange, - isSceneObject, - SceneObjectState, - SceneLayoutChild, -} from './types'; +import { SceneDataState, SceneObject, SceneComponent, SceneEditor, SceneTimeRange, SceneObjectState } from './types'; export abstract class SceneObjectBase implements SceneObject { - subject = new Subject(); - state: TState; - parent?: SceneObjectBase; - subs = new Subscription(); - isActive?: boolean; - events = new EventBusSrv(); + private _isActive = false; + private _subject = new Subject(); + private _state: TState; + private _events = new EventBusSrv(); + + protected _parent?: SceneObject; + protected subs = new Subscription(); constructor(state: TState) { if (!state.key) { state.key = uuidv4(); } - this.state = state; - this.subject.next(state); + this._state = state; + this._subject.next(state); this.setParent(); } + /** Current state */ + get state(): TState { + return this._state; + } + + /** True if currently being active (ie displayed for visual objects) */ + get isActive(): boolean { + return this._isActive; + } + + /** Returns the parent, undefined for root object */ + get parent(): SceneObject | undefined { + return this._parent; + } + /** * Used in render functions when rendering a SceneObject. * Wraps the component in an EditWrapper that handles edit mode @@ -52,69 +59,105 @@ export abstract class SceneObjectBase impl } private setParent() { - for (const propValue of Object.values(this.state)) { - if (isSceneObject(propValue)) { - propValue.parent = this; + for (const propValue of Object.values(this._state)) { + if (propValue instanceof SceneObjectBase) { + propValue._parent = this; } if (Array.isArray(propValue)) { for (const child of propValue) { - if (isSceneObject(child)) { - child.parent = this; + if (child instanceof SceneObjectBase) { + child._parent = this; } } } } } - /** This function implements the Subscribable interface */ - subscribe(observer: Partial>) { - return this.subject.subscribe(observer); + /** + * Subscribe to the scene state subject + **/ + subscribeToState(observerOrNext?: Partial>): Subscription { + return this._subject.subscribe(observerOrNext); + } + + /** + * Subscribe to the scene event + **/ + subscribeToEvent(eventType: BusEventType, handler: BusEventHandler): Unsubscribable { + return this._events.subscribe(eventType, handler); } setState(update: Partial) { - const prevState = this.state; - this.state = { - ...this.state, + const prevState = this._state; + this._state = { + ...this._state, ...update, }; this.setParent(); - this.subject.next(this.state); + this._subject.next(this._state); - // broadcast state change. This is event is subscribed to by UrlSyncManager and UndoManager - this.getRoot().events.publish( + // Bubble state change event. This is event is subscribed to by UrlSyncManager and UndoManager + this.publishEvent( new SceneObjectStateChangedEvent({ prevState, - newState: this.state, + newState: this._state, partialUpdate: update, changedObject: this, - }) + }), + true ); } - private getRoot(): SceneObject { - return !this.parent ? this : this.parent.getRoot(); + /* + * Publish an event and optionally bubble it up the scene + **/ + publishEvent(event: BusEvent, bubble?: boolean) { + this._events.publish(event); + + if (bubble && this.parent) { + this.parent.publishEvent(event, bubble); + } + } + + getRoot(): SceneObject { + return !this._parent ? this : this._parent.getRoot(); } activate() { - this.isActive = true; + this._isActive = true; + + const { $data, $variables } = this.state; - const { $data } = this.state; if ($data && !$data.isActive) { $data.activate(); } + + if ($variables && !$variables.isActive) { + $variables.activate(); + } } deactivate(): void { - this.isActive = false; + this._isActive = false; + + const { $data, $variables } = this.state; - const { $data } = this.state; if ($data && $data.isActive) { $data.deactivate(); } + if ($variables && $variables.isActive) { + $variables.deactivate(); + } + + // Clear subscriptions and listeners + this._events.removeAllListeners(); this.subs.unsubscribe(); this.subs = new Subscription(); + + this._subject.complete(); + this._subject = new Subject(); } useState() { @@ -171,7 +214,7 @@ export abstract class SceneObjectBase impl } /** - * Will create new SceneItem with shalled cloned state, but all states items of type SceneItem are deep cloned + * Will create new SceneItem with shalled cloned state, but all states items of type SceneObject are deep cloned */ clone(withState?: Partial): this { const clonedState = { ...this.state }; @@ -182,15 +225,19 @@ export abstract class SceneObjectBase impl if (propValue instanceof SceneObjectBase) { clonedState[key] = propValue.clone(); } - } - // Clone layout children - if ('children' in this.state) { - const newChildren: SceneLayoutChild[] = []; - for (const child of this.state.children) { - newChildren.push(child.clone()); + // Clone scene objects in arrays + if (Array.isArray(propValue)) { + const newArray: any = []; + for (const child of propValue) { + if (child instanceof SceneObjectBase) { + newArray.push(child.clone()); + } else { + newArray.push(child); + } + } + clonedState[key] = newArray; } - (clonedState as any).children = newChildren; } Object.assign(clonedState, withState); @@ -207,7 +254,7 @@ function useSceneObjectState(model: SceneObject const forceUpdate = useForceUpdate(); useEffect(() => { - const s = model.subject.subscribe(forceUpdate); + const s = model.subscribeToState({ next: forceUpdate }); return () => s.unsubscribe(); }, [model, forceUpdate]); diff --git a/public/app/features/scenes/core/events.ts b/public/app/features/scenes/core/events.ts index b447aa5a45d..ab221616554 100644 --- a/public/app/features/scenes/core/events.ts +++ b/public/app/features/scenes/core/events.ts @@ -12,3 +12,10 @@ 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'; +} diff --git a/public/app/features/scenes/core/types.ts b/public/app/features/scenes/core/types.ts index 170b4791534..9791ba034e5 100644 --- a/public/app/features/scenes/core/types.ts +++ b/public/app/features/scenes/core/types.ts @@ -1,16 +1,16 @@ import React from 'react'; -import { Subscribable } from 'rxjs'; +import { Observer, Subscription, Unsubscribable } from 'rxjs'; -import { EventBus, PanelData, TimeRange, UrlQueryMap } from '@grafana/data'; +import { BusEvent, BusEventHandler, BusEventType, PanelData, TimeRange, UrlQueryMap } from '@grafana/data'; -import { SceneVariableSet } from '../variables/types'; +import { SceneVariables } from '../variables/types'; export interface SceneObjectStatePlain { key?: string; $timeRange?: SceneTimeRange; $data?: SceneObject; $editor?: SceneEditor; - $variables?: SceneVariableSet; + $variables?: SceneVariables; } export interface SceneLayoutChildState extends SceneObjectStatePlain { @@ -41,18 +41,24 @@ export interface SceneDataState extends SceneObjectStatePlain { data?: PanelData; } -export interface SceneObject extends Subscribable { +export interface SceneObject { /** The current state */ - state: TState; + readonly state: TState; /** True when there is a React component mounted for this Object */ - isActive?: boolean; + readonly isActive: boolean; /** SceneObject parent */ - parent?: SceneObject; + readonly parent?: SceneObject; - /** Currently only used from root to broadcast events */ - events: EventBus; + /** Subscribe to state changes */ + subscribeToState(observer?: Partial>): Subscription; + + /** Subscribe to a scene event */ + subscribeToEvent(typeFilter: BusEventType, handler: BusEventHandler): Unsubscribable; + + /** Publish an event and optionally bubble it up the scene */ + publishEvent(event: BusEvent, bubble?: boolean): void; /** Utility hook that wraps useObservable. Used by React components to subscribes to state changes */ useState(): TState; @@ -63,12 +69,21 @@ export interface SceneObject /** Called when the Component is mounted. A place to register event listeners add subscribe to state changes */ activate(): void; - /** Called when component unmounts. Unsubscribe to events */ + /** Called when component unmounts. Unsubscribe and closes all subscriptions */ deactivate(): void; /** Get the scene editor */ getSceneEditor(): SceneEditor; + /** Get the scene root */ + getRoot(): SceneObject; + + /** Get the closest node with data */ + getData(): SceneObject; + + /** Get the closest node with time range */ + getTimeRange(): SceneTimeRange; + /** Returns a deep clone this object and all its children */ clone(state?: Partial): this; diff --git a/public/app/features/scenes/querying/SceneQueryRunner.ts b/public/app/features/scenes/querying/SceneQueryRunner.ts index dadbf7524f0..81f644cfadb 100644 --- a/public/app/features/scenes/querying/SceneQueryRunner.ts +++ b/public/app/features/scenes/querying/SceneQueryRunner.ts @@ -37,7 +37,7 @@ export class SceneQueryRunner extends SceneObjectBase { const timeRange = this.getTimeRange(); this.subs.add( - timeRange.subscribe({ + timeRange.subscribeToState({ next: (timeRange) => { this.runWithTimeRange(timeRange); }, diff --git a/public/app/features/scenes/services/UrlSyncManager.ts b/public/app/features/scenes/services/UrlSyncManager.ts index 83c55e635c3..bb20cacc082 100644 --- a/public/app/features/scenes/services/UrlSyncManager.ts +++ b/public/app/features/scenes/services/UrlSyncManager.ts @@ -11,7 +11,7 @@ export class UrlSyncManager { private stateChangeSub: Unsubscribable; constructor(sceneRoot: SceneObject) { - this.stateChangeSub = sceneRoot.events.subscribe(SceneObjectStateChangedEvent, this.onStateChanged); + this.stateChangeSub = sceneRoot.subscribeToEvent(SceneObjectStateChangedEvent, this.onStateChanged); this.locationListenerUnsub = locationService.getHistory().listen(this.onLocationUpdate); } diff --git a/public/app/features/scenes/variables/SceneVariableSet.test.ts b/public/app/features/scenes/variables/SceneVariableSet.test.ts index d1f9f927125..655b6d8cd68 100644 --- a/public/app/features/scenes/variables/SceneVariableSet.test.ts +++ b/public/app/features/scenes/variables/SceneVariableSet.test.ts @@ -1,7 +1,7 @@ import { SceneObjectBase } from '../core/SceneObjectBase'; import { SceneObjectStatePlain } from '../core/types'; -import { sceneTemplateInterpolator, SceneVariableManager, TextBoxSceneVariable } from './SceneVariableSet'; +import { sceneTemplateInterpolator, SceneVariableSet, TextBoxSceneVariable } from './SceneVariableSet'; interface TestSceneState extends SceneObjectStatePlain { nested?: TestScene; @@ -12,7 +12,7 @@ class TestScene extends SceneObjectBase {} describe('SceneObject with variables', () => { it('Should be interpolate and use closest variable', () => { const scene = new TestScene({ - $variables: new SceneVariableManager({ + $variables: new SceneVariableSet({ variables: [ new TextBoxSceneVariable({ name: 'test', @@ -25,7 +25,7 @@ describe('SceneObject with variables', () => { ], }), nested: new TestScene({ - $variables: new SceneVariableManager({ + $variables: new SceneVariableSet({ variables: [ new TextBoxSceneVariable({ name: 'test', diff --git a/public/app/features/scenes/variables/SceneVariableSet.ts b/public/app/features/scenes/variables/SceneVariableSet.ts index 13e0c21862a..d7f90d8c777 100644 --- a/public/app/features/scenes/variables/SceneVariableSet.ts +++ b/public/app/features/scenes/variables/SceneVariableSet.ts @@ -3,11 +3,11 @@ import { variableRegex } from 'app/features/variables/utils'; import { SceneObjectBase } from '../core/SceneObjectBase'; import { SceneObject } from '../core/types'; -import { SceneVariable, SceneVariableSet, SceneVariableSetState, SceneVariableState } from './types'; +import { SceneVariable, SceneVariables, SceneVariableSetState, SceneVariableState } from './types'; export class TextBoxSceneVariable extends SceneObjectBase implements SceneVariable {} -export class SceneVariableManager extends SceneObjectBase implements SceneVariableSet { +export class SceneVariableSet extends SceneObjectBase implements SceneVariables { getVariableByName(name: string): SceneVariable | undefined { // TODO: Replace with index return this.state.variables.find((x) => x.state.name === name); diff --git a/public/app/features/scenes/variables/types.ts b/public/app/features/scenes/variables/types.ts index 8cd1cbdc59a..e99f93b64de 100644 --- a/public/app/features/scenes/variables/types.ts +++ b/public/app/features/scenes/variables/types.ts @@ -19,6 +19,6 @@ export interface SceneVariableSetState extends SceneObjectStatePlain { variables: SceneVariable[]; } -export interface SceneVariableSet extends SceneObject { +export interface SceneVariables extends SceneObject { getVariableByName(name: string): SceneVariable | undefined; }