Scene: SceneObject and SceneObjectBase refinements and stricter typing (#57851)

.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"],

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);
     });
   });
 });

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<Function, Subscriber<BusEvent>>();
 
   constructor() {
     this.emitter = new EventEmitter();
@@ -31,16 +32,18 @@ export class EventBusSrv implements EventBus, LegacyEmitter {
     return this.getStream(typeFilter).subscribe({ next: handler });
   }
 
-  getStream<T extends BusEvent>(eventType: BusEventType<T>): Observable<T> {
+  getStream<T extends BusEvent = BusEvent>(eventType: BusEventType<T>): Observable<T> {
     return new Observable<T>((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);
+    }
   }
 }
 

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

@@ -68,7 +68,7 @@ export interface EventFilterOptions {
  */
 export interface EventBus {
   /**
-   * Publish single vent
+   * Publish single event
    */
   publish<T extends BusEvent>(event: T): void;
 

public/app/features/scenes/components/ScenePanelRepeater.tsx

@@ -21,7 +21,7 @@ export class ScenePanelRepeater extends SceneObjectBase<RepeatOptions> {
     super.activate();
 
     this.subs.add(
-      this.getData().subscribe({
+      this.getData().subscribeToState({
         next: (data) => {
           if (data.data?.state === LoadingState.Done) {
             this.performRepeat(data.data);

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);
+    });
+  });
 });

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<TState extends SceneObjectState = {}> implements SceneObject<TState> {
-  subject = new Subject<TState>();
-  state: TState;
-  parent?: SceneObjectBase<SceneObjectState>;
-  subs = new Subscription();
-  isActive?: boolean;
-  events = new EventBusSrv();
+  private _isActive = false;
+  private _subject = new Subject<TState>();
+  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<TState extends SceneObjectState = {}> 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<TState> interface */
-  subscribe(observer: Partial<Observer<TState>>) {
-    return this.subject.subscribe(observer);
+  /**
+   * Subscribe to the scene state subject
+   **/
+  subscribeToState(observerOrNext?: Partial<Observer<TState>>): Subscription {
+    return this._subject.subscribe(observerOrNext);
+  }
+
+  /**
+   * Subscribe to the scene event
+   **/
+  subscribeToEvent<T extends BusEvent>(eventType: BusEventType<T>, handler: BusEventHandler<T>): Unsubscribable {
+    return this._events.subscribe(eventType, handler);
   }
 
   setState(update: Partial<TState>) {
-    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<TState>();
   }
 
   useState() {
@@ -171,7 +214,7 @@ export abstract class SceneObjectBase<TState extends SceneObjectState = {}> 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<TState>): this {
     const clonedState = { ...this.state };
@@ -182,15 +225,19 @@ export abstract class SceneObjectBase<TState extends SceneObjectState = {}> 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<TState extends SceneObjectState>(model: SceneObject
   const forceUpdate = useForceUpdate();
 
   useEffect(() => {
-    const s = model.subject.subscribe(forceUpdate);
+    const s = model.subscribeToState({ next: forceUpdate });
     return () => s.unsubscribe();
   }, [model, forceUpdate]);
 

public/app/features/scenes/core/events.ts

@@ -12,3 +12,10 @@ export interface SceneObjectStateChangedPayload {
 export class SceneObjectStateChangedEvent extends BusEventWithPayload<SceneObjectStateChangedPayload> {
   static type = 'scene-object-state-change';
 }
+
+export class SceneObjectActivedEvent extends BusEventWithPayload<SceneObject> {
+  static type = 'scene-object-activated';
+}
+export class SceneObjectDeactivatedEvent extends BusEventWithPayload<SceneObject> {
+  static type = 'scene-object-deactivated';
+}

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<SceneDataState>;
   $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<TState extends SceneObjectState = SceneObjectState> extends Subscribable<TState> {
+export interface SceneObject<TState extends SceneObjectState = SceneObjectState> {
   /** 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<Observer<TState>>): Subscription;
+
+  /** Subscribe to a scene event */
+  subscribeToEvent<T extends BusEvent>(typeFilter: BusEventType<T>, handler: BusEventHandler<T>): 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<TState extends SceneObjectState = SceneObjectState>
   /** 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<SceneDataState>;
+
+  /** Get the closest node with time range */
+  getTimeRange(): SceneTimeRange;
+
   /** Returns a deep clone this object and all its children */
   clone(state?: Partial<TState>): this;
 

public/app/features/scenes/querying/SceneQueryRunner.ts

@@ -37,7 +37,7 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> {
     const timeRange = this.getTimeRange();
 
     this.subs.add(
-      timeRange.subscribe({
+      timeRange.subscribeToState({
         next: (timeRange) => {
           this.runWithTimeRange(timeRange);
         },

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);
   }
 

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<TestSceneState> {}
 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',

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<SceneVariableState> implements SceneVariable {}
 
-export class SceneVariableManager extends SceneObjectBase<SceneVariableSetState> implements SceneVariableSet {
+export class SceneVariableSet extends SceneObjectBase<SceneVariableSetState> implements SceneVariables {
   getVariableByName(name: string): SceneVariable | undefined {
     // TODO: Replace with index
     return this.state.variables.find((x) => x.state.name === name);

public/app/features/scenes/variables/types.ts

@@ -19,6 +19,6 @@ export interface SceneVariableSetState extends SceneObjectStatePlain {
   variables: SceneVariable[];
 }
 
-export interface SceneVariableSet extends SceneObject<SceneVariableSetState> {
+export interface SceneVariables extends SceneObject<SceneVariableSetState> {
   getVariableByName(name: string): SceneVariable | undefined;
 }