ng: reload now only reloads current dashboard (#3426)

Previously, we were reloading all stamped dashboards which is not the
behavior of the tf-tensorboard.

Side-effect: because we were reloading scalars dashboard when it was in
the background, the charts could more frequently appear tiny when the
reload kicked in. This change removes that bug.

Author: stephanwlee

tensorboard/webapp/plugins/plugins_component.ts

@@ -59,7 +59,7 @@ export class PluginsComponent implements OnChanges {
   private readonly ngPluginContainer!: ViewContainerRef;
 
   @Input()
-  activePlugin?: UiPluginMetadata;
+  activePlugin!: UiPluginMetadata | null;
 
   @Input()
   lastUpdated?: number;
@@ -140,11 +140,15 @@ export class PluginsComponent implements OnChanges {
   }
 
   private reload() {
-    for (const instance of this.pluginInstances.values()) {
-      const maybePolymerDashboard = instance as any;
-      if (maybePolymerDashboard.reload) {
-        maybePolymerDashboard.reload();
-      }
+    if (!this.activePlugin) {
+      return;
+    }
+
+    const maybeDashboard = this.pluginInstances.get(
+      this.activePlugin.id
+    ) as any;
+    if (maybeDashboard.reload) {
+      maybeDashboard.reload();
     }
   }
 }

tensorboard/webapp/plugins/plugins_container_test.ts

@@ -19,19 +19,26 @@ import {provideMockStore, MockStore} from '@ngrx/store/testing';
 
 import {PluginsContainer} from './plugins_container';
 import {PluginsComponent} from './plugins_component';
-
-import {PluginId, LoadingMechanismType} from '../types/api';
+import {PluginRegistryModule} from './plugin_registry_module';
+import {ExtraDashboardModule} from './testing';
+
+import {
+  PluginId,
+  LoadingMechanismType,
+  CustomElementLoadingMechanism,
+  IframeLoadingMechanism,
+  NgElementLoadingMechanism,
+} from '../types/api';
 import {DataLoadState} from '../types/data';
-import {createState, createCoreState} from '../core/testing';
 import {State} from '../core/store';
-// store/index.ts doesn't export this, but it's OK to use for testing
-import {CoreState} from '../core/store/core_types';
-
+import {
+  getPlugins,
+  getActivePlugin,
+  getPluginsListLoaded,
+} from '../core/store/core_selectors';
 import {TestingDebuggerModule} from '../../plugins/debugger_v2/tf_debugger_v2_plugin/testing';
 
 /** @typehack */ import * as _typeHackStore from '@ngrx/store';
-import {PluginRegistryModule} from './plugin_registry_module';
-import {ExtraDashboardComponent, ExtraDashboardModule} from './testing';
 
 function expectPluginIframe(element: HTMLElement, name: string) {
   expect(element.tagName).toBe('IFRAME');
@@ -42,72 +49,61 @@ function expectPluginIframe(element: HTMLElement, name: string) {
 
 describe('plugins_component', () => {
   let store: MockStore<State>;
-  const INITIAL_CORE_STATE: Partial<CoreState> = {
-    plugins: {
-      bar: {
-        disable_reload: false,
-        enabled: true,
-        loading_mechanism: {
-          type: LoadingMechanismType.CUSTOM_ELEMENT,
-          element_name: 'tb-bar',
-        },
-        tab_name: 'Bar',
-        remove_dom: false,
-      },
-      'extra-plugin': {
-        disable_reload: false,
-        enabled: true,
-        loading_mechanism: {
-          type: LoadingMechanismType.NG_COMPONENT,
-        },
-        tab_name: 'Extra',
-        remove_dom: false,
-      },
-      foo: {
-        disable_reload: false,
-        enabled: true,
-        loading_mechanism: {
-          type: LoadingMechanismType.IFRAME,
-          // This will cause 404 as test bundles do not serve
-          // data file in the karma server.
-          module_path: 'random_esmodule.js',
-        },
-        tab_name: 'Bar',
-        remove_dom: false,
-      },
+  const PLUGINS = {
+    bar: {
+      disable_reload: false,
+      enabled: true,
+      loading_mechanism: {
+        type: LoadingMechanismType.CUSTOM_ELEMENT,
+        element_name: 'tb-bar',
+      } as CustomElementLoadingMechanism,
+      tab_name: 'Bar',
+      remove_dom: false,
+    },
+    'extra-plugin': {
+      disable_reload: false,
+      enabled: true,
+      loading_mechanism: {
+        type: LoadingMechanismType.NG_COMPONENT,
+      } as NgElementLoadingMechanism,
+      tab_name: 'Extra',
+      remove_dom: false,
+    },
+    foo: {
+      disable_reload: false,
+      enabled: true,
+      loading_mechanism: {
+        type: LoadingMechanismType.IFRAME,
+        // This will cause 404 as test bundles do not serve
+        // data file in the karma server.
+        module_path: 'random_esmodule.js',
+      } as IframeLoadingMechanism,
+      tab_name: 'Bar',
+      remove_dom: false,
     },
   };
 
+  function setActivePlugin(plugin: PluginId) {
+    store.overrideSelector(getActivePlugin, plugin);
+    store.refreshState();
+  }
+
   beforeEach(async () => {
-    const initialState = createState(
-      createCoreState({
-        ...INITIAL_CORE_STATE,
-      })
-    );
     await TestBed.configureTestingModule({
-      providers: [
-        provideMockStore({initialState}),
-        PluginsContainer,
-        PluginRegistryModule,
-      ],
+      providers: [provideMockStore(), PluginsContainer, PluginRegistryModule],
       declarations: [PluginsContainer, PluginsComponent],
       imports: [TestingDebuggerModule, ExtraDashboardModule],
     }).compileComponents();
     store = TestBed.get(Store);
+    store.overrideSelector(getPlugins, PLUGINS);
+    store.overrideSelector(getActivePlugin, null);
+    store.overrideSelector(getPluginsListLoaded, {
+      state: DataLoadState.NOT_LOADED,
+      lastLoadedTimeInMs: null,
+    });
   });
 
   describe('plugin DOM creation', () => {
-    function setActivePlugin(plugin: PluginId) {
-      store.setState(
-        createState(
-          createCoreState({
-            ...INITIAL_CORE_STATE,
-            activePlugin: plugin,
-          })
-        )
-      );
-    }
-
     it('creates no plugin when there is no activePlugin', () => {
       const fixture = TestBed.createComponent(PluginsContainer);
       const el = fixture.debugElement.query(By.css('.plugins'));
@@ -219,42 +215,56 @@ describe('plugins_component', () => {
       timeInMs: number | null,
       state = DataLoadState.LOADED
     ) {
-      store.setState(
-        createState(
-          createCoreState({
-            ...INITIAL_CORE_STATE,
-            activePlugin: 'bar',
-            pluginsListLoaded: {
-              state,
-              lastLoadedTimeInMs: timeInMs,
-            },
-          })
-        )
-      );
+      store.overrideSelector(getPluginsListLoaded, {
+        state:
+          timeInMs !== null ? DataLoadState.LOADED : DataLoadState.NOT_LOADED,
+        lastLoadedTimeInMs: timeInMs,
+      });
+      store.refreshState();
     }
 
     it('invokes reload method on the dashboard DOM', () => {
       const fixture = TestBed.createComponent(PluginsContainer);
 
       setLastLoadedTime(null, DataLoadState.NOT_LOADED);
+      setActivePlugin('bar');
+      fixture.detectChanges();
+      setActivePlugin('foo');
+      fixture.detectChanges();
+      setActivePlugin('bar');
       fixture.detectChanges();
 
       const {nativeElement} = fixture.debugElement.query(By.css('.plugins'));
-      const [barElement] = nativeElement.children;
-      const reloadSpy = jasmine.createSpy();
-      barElement.reload = reloadSpy;
+      // Stamped 'bar' and 'foo'
+      expect(nativeElement.children.length).toBe(2);
+      const [barElement, fooElement] = nativeElement.children;
+      const barReloadSpy = jasmine.createSpy();
+      barElement.reload = barReloadSpy;
+      const fooReloadSpy = jasmine.createSpy();
+      fooElement.reload = fooReloadSpy;
 
       setLastLoadedTime(1);
       fixture.detectChanges();
-      expect(reloadSpy).toHaveBeenCalledTimes(1);
+      expect(barReloadSpy).toHaveBeenCalledTimes(1);
+      expect(fooReloadSpy).not.toHaveBeenCalled();
 
       setLastLoadedTime(1);
       fixture.detectChanges();
-      expect(reloadSpy).toHaveBeenCalledTimes(1);
+      expect(barReloadSpy).toHaveBeenCalledTimes(1);
+      expect(fooReloadSpy).not.toHaveBeenCalled();
 
       setLastLoadedTime(2);
       fixture.detectChanges();
-      expect(reloadSpy).toHaveBeenCalledTimes(2);
+      expect(barReloadSpy).toHaveBeenCalledTimes(2);
+      expect(fooReloadSpy).not.toHaveBeenCalled();
+
+      setActivePlugin('foo');
+      fixture.detectChanges();
+
+      setLastLoadedTime(3);
+      fixture.detectChanges();
+      expect(barReloadSpy).toHaveBeenCalledTimes(2);
+      expect(fooReloadSpy).toHaveBeenCalledTimes(1);
     });
   });
 });