Scene: Small refactorings and name changes (#51866)

* Rename onMount and onUnmount and some other small refactorings

* More refactorings fixing typescript issues

.betterer.results

@@ -5726,11 +5726,9 @@ exports[`better eslint`] = {
     "public/app/features/scenes/components/SceneFlexLayout.tsx:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"]
     ],
-    "public/app/features/scenes/core/SceneComponentEditWrapper.tsx:5381": [
+    "public/app/features/scenes/core/SceneComponentWrapper.tsx:5381": [
-      [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
+      [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.", "1"]
-      [0, 0, 0, "Unexpected any. Specify a different type.", "2"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "3"]
     ],
     "public/app/features/scenes/core/SceneObjectBase.tsx:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

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

@@ -17,13 +17,13 @@ export class Scene extends SceneObjectBase<SceneState> {
   static Component = SceneRenderer;
   urlSyncManager?: UrlSyncManager;
 
-  onMount() {
+  activate() {
-    super.onMount();
+    super.activate();
     this.urlSyncManager = new UrlSyncManager(this);
   }
 
-  onUnmount() {
+  deactivate() {
-    super.onUnmount();
+    super.deactivate();
     this.urlSyncManager!.cleanUp();
   }
 }
@@ -31,8 +31,6 @@ export class Scene extends SceneObjectBase<SceneState> {
 function SceneRenderer({ model }: SceneComponentProps<Scene>) {
   const { title, layout, actions = [], isEditing, $editor } = model.useState();
 
-  console.log('render scene');
-
   return (
     <div style={{ height: '100%', display: 'flex', flexDirection: 'column', flex: '1 1 0', minHeight: 0 }}>
       <PageToolbar title={title}>

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

@@ -11,8 +11,8 @@ interface RepeatOptions extends SceneObjectState {
 }
 
 export class ScenePanelRepeater extends SceneObjectBase<RepeatOptions> {
-  onMount() {
+  activate(): void {
-    super.onMount();
+    super.activate();
 
     this.subs.add(
       this.getData().subscribe({

public/app/features/scenes/core/SceneComponentWrapper.tsx

@@ -0,0 +1,32 @@
+import React, { useEffect } from 'react';
+
+import { SceneComponentEditingWrapper } from '../editor/SceneComponentEditWrapper';
+
+import { SceneComponentProps, SceneObject } from './types';
+
+export function SceneComponentWrapper<T extends SceneObject>({ model, isEditing }: SceneComponentProps<T>) {
+  const Component = (model as any).constructor['Component'] ?? EmptyRenderer;
+  const inner = <Component model={model} isEditing={isEditing} />;
+
+  // Handle component activation state state
+  useEffect(() => {
+    if (!model.isActive) {
+      model.activate();
+    }
+    return () => {
+      if (model.isActive) {
+        model.deactivate();
+      }
+    };
+  }, [model]);
+
+  if (!isEditing) {
+    return inner;
+  }
+
+  return <SceneComponentEditingWrapper model={model}>{inner}</SceneComponentEditingWrapper>;
+}
+
+function EmptyRenderer<T>(_: SceneComponentProps<T>): React.ReactElement | null {
+  return null;
+}

public/app/features/scenes/core/SceneObjectBase.test.ts

@@ -23,12 +23,12 @@ describe('SceneObject', () => {
       ],
     });
 
-    scene.state.nested?.onMount();
+    scene.state.nested?.activate();
 
     const clone = scene.clone();
     expect(clone).not.toBe(scene);
     expect(clone.state.nested).not.toBe(scene.state.nested);
-    expect(clone.state.nested?.isMounted).toBe(undefined);
+    expect(clone.state.nested?.isActive).toBe(undefined);
     expect(clone.state.children![0]).not.toBe(scene.state.children![0]);
   });
 

public/app/features/scenes/core/SceneObjectBase.tsx

@@ -1,11 +1,10 @@
-import { useEffect } from 'react';
 import { useObservable } from 'react-use';
 import { Observer, Subject, Subscription } from 'rxjs';
 import { v4 as uuidv4 } from 'uuid';
 
 import { EventBusSrv } from '@grafana/data';
 
-import { SceneComponentEditWrapper } from './SceneComponentEditWrapper';
+import { SceneComponentWrapper } from './SceneComponentWrapper';
 import { SceneObjectStateChangedEvent } from './events';
 import {
   SceneDataState,
@@ -23,7 +22,7 @@ export abstract class SceneObjectBase<TState extends SceneObjectState = {}> impl
   state: TState;
   parent?: SceneObjectBase<any>;
   subs = new Subscription();
-  isMounted?: boolean;
+  isActive?: boolean;
   events = new EventBusSrv();
 
   constructor(state: TState) {
@@ -41,7 +40,7 @@ export abstract class SceneObjectBase<TState extends SceneObjectState = {}> impl
    * Wraps the component in an EditWrapper that handles edit mode
    */
   get Component(): SceneComponent<this> {
-    return SceneComponentEditWrapper;
+    return SceneComponentWrapper;
   }
 
   /**
@@ -96,46 +95,27 @@ export abstract class SceneObjectBase<TState extends SceneObjectState = {}> impl
     return !this.parent ? this : this.parent.getRoot();
   }
 
-  onMount() {
+  activate() {
-    this.isMounted = true;
+    this.isActive = true;
 
     const { $data } = this.state;
-    if ($data && !$data.isMounted) {
+    if ($data && !$data.isActive) {
-      $data.onMount();
+      $data.activate();
     }
   }
 
-  onUnmount() {
+  deactivate(): void {
-    this.isMounted = false;
+    this.isActive = false;
 
     const { $data } = this.state;
-    if ($data && $data.isMounted) {
+    if ($data && $data.isActive) {
-      $data.onUnmount();
+      $data.deactivate();
     }
 
     this.subs.unsubscribe();
     this.subs = new Subscription();
   }
 
-  /**
-   * The scene object needs to know when the react component is mounted to trigger query and other lazy actions
-   */
-  useMount() {
-    // eslint-disable-next-line react-hooks/rules-of-hooks
-    useEffect(() => {
-      if (!this.isMounted) {
-        this.onMount();
-      }
-      return () => {
-        if (this.isMounted) {
-          this.onUnmount();
-        }
-      };
-    }, []);
-
-    return this;
-  }
-
   useState() {
     // eslint-disable-next-line react-hooks/rules-of-hooks
     return useObservable(this.subject, this.state);

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

@@ -41,7 +41,7 @@ export interface SceneObject<TState extends SceneObjectState = SceneObjectState>
   state: TState;
 
   /** True when there is a React component mounted for this Object */
-  isMounted?: boolean;
+  isActive?: boolean;
 
   /** SceneObject parent */
   parent?: SceneObject;
@@ -55,14 +55,11 @@ export interface SceneObject<TState extends SceneObjectState = SceneObjectState>
   /** How to modify state */
   setState(state: Partial<TState>): void;
 
-  /** Utility hook for main component so that object knows when it's mounted */
+  /** Called when the Component is mounted. A place to register event listeners add subscribe to state changes */
-  useMount(): this;
+  activate(): void;
-
-  /** Called when component mounts. A place to register event listeners add subscribe to state changes */
-  onMount(): void;
 
   /** Called when component unmounts. Unsubscribe to events */
-  onUnmount(): void;
+  deactivate(): void;
 
   /** Get the scene editor */
   getSceneEditor(): SceneEditor;

public/app/features/scenes/editor/SceneComponentEditWrapper.tsx

@@ -4,26 +4,9 @@ import React, { CSSProperties } from 'react';
 import { GrafanaTheme2 } from '@grafana/data';
 import { useStyles2 } from '@grafana/ui';
 
-import { SceneObjectBase } from './SceneObjectBase';
+import { SceneObject } from '../core/types';
-import { SceneComponentProps } from './types';
 
-export function SceneComponentEditWrapper<T extends SceneObjectBase<any>>({
+export function SceneComponentEditingWrapper<T extends SceneObject>({
-  model,
-  isEditing,
-}: SceneComponentProps<T>) {
-  const Component = (model as any).constructor['Component'] ?? EmptyRenderer;
-  const inner = <Component model={model} isEditing={isEditing} />;
-
-  model.useMount();
-
-  if (!isEditing) {
-    return inner;
-  }
-
-  return <SceneComponentEditingWrapper model={model}>{inner}</SceneComponentEditingWrapper>;
-}
-
-export function SceneComponentEditingWrapper<T extends SceneObjectBase<any>>({
   model,
   children,
 }: {
@@ -76,7 +59,3 @@ const getStyles = (theme: GrafanaTheme2) => {
     }),
   };
 };
-
-function EmptyRenderer<T>(_: SceneComponentProps<T>): React.ReactElement | null {
-  return null;
-}

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

@@ -31,8 +31,8 @@ export interface DataQueryExtended extends DataQuery {
 export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> {
   private querySub?: Unsubscribable;
 
-  onMount() {
+  activate() {
-    super.onMount();
+    super.activate();
 
     const timeRange = this.getTimeRange();
 
@@ -49,12 +49,9 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> {
     }
   }
 
-  onUnmount() {
+  deactivate(): void {
-    super.onUnmount();
+    super.deactivate();
-    this.cleanUp();
-  }
 
-  cleanUp() {
     if (this.querySub) {
       this.querySub.unsubscribe();
       this.querySub = undefined;
@@ -108,8 +105,6 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> {
       request.interval = norm.interval;
       request.intervalMs = norm.intervalMs;
 
-      console.log('Query runner run');
-
       this.querySub = runRequest(ds, request).subscribe({
         next: (data) => {
           console.log('set data', data, data.state);