improve according to review

src/legacy/core_plugins/kibana/public/dashboard/np_ready/application.ts

@@ -19,7 +19,7 @@
 
 import { EuiConfirmModal, EuiIcon } from '@elastic/eui';
 import angular, { IModule } from 'angular';
-import { createHashHistory, History } from 'history';
+import { History } from 'history';
 import { i18nDirective, i18nFilter, I18nProvider } from '@kbn/i18n/angular';
 import {
   AppMountContext,
@@ -28,11 +28,7 @@ import {
   LegacyCoreStart,
   SavedObjectsClientContract,
 } from 'kibana/public';
-import {
-  createKbnUrlStateStorage,
-  IKbnUrlStateStorage,
-  Storage,
-} from '../../../../../../plugins/kibana_utils/public';
+import { IKbnUrlStateStorage, Storage } from '../../../../../../plugins/kibana_utils/public';
 import {
   configureAppAngularModule,
   confirmModalFactory,
@@ -52,10 +48,7 @@ import {
 import { initDashboardApp } from './legacy_app';
 import { IEmbeddableStart } from '../../../../../../plugins/embeddable/public';
 import { NavigationPublicPluginStart as NavigationStart } from '../../../../../../plugins/navigation/public';
-import {
-  DataPublicPluginStart as NpDataStart,
-  syncQuery,
-} from '../../../../../../plugins/data/public';
+import { DataPublicPluginStart as NpDataStart } from '../../../../../../plugins/data/public';
 import { SharePluginStart } from '../../../../../../plugins/share/public';
 
 export interface RenderDeps {
@@ -73,57 +66,25 @@ export interface RenderDeps {
   embeddables: IEmbeddableStart;
   localStorage: Storage;
   share: SharePluginStart;
-
-  // hack:
-  // renderApp() called each time the app is mounted,
-  // but initialisation of angular module happens only once.
-  // On each app mount, new history and kbnUrlStateStorage instances are created
-  // and we have to make sure, that those new instances will be used in DashboardAppController
-  // and not the ones which where created when angular module was initialised.
-  // This is achieved by having reference to initial object, which angular module was initialised with,
-  // and then by mutating that object on subsequent mounts we can pass new instances down to controller.
-  // If not for this hack, we could end up that global state syncing uses different history instance then app state syncing
-  // Which could cause excessive browser history entries.
-  angularGlobalStateHacks?: AngularGlobalStateHacks;
+  history: History;
+  kbnUrlStateStorage: IKbnUrlStateStorage;
 }
 
-interface AngularGlobalStateHacks {
-  hasInheritedGlobalState?: boolean;
-  kbnUrlStateStorage?: IKbnUrlStateStorage;
-  history?: History;
-}
 let angularModuleInstance: IModule | null = null;
-const angularGlobalStateHacks: AngularGlobalStateHacks = {};
 
 export const renderApp = (element: HTMLElement, appBasePath: string, deps: RenderDeps) => {
-  const history = createHashHistory();
-  const kbnUrlStateStorage = createKbnUrlStateStorage({
-    history,
-    useHash: deps.uiSettings.get('state:storeInSessionStorage'),
-  });
-  const { stop: stopSyncingGlobalState, hasInheritedQueryFromUrl } = syncQuery(
-    deps.npDataStart.query,
-    kbnUrlStateStorage
-  );
-
-  angularGlobalStateHacks.hasInheritedGlobalState = hasInheritedQueryFromUrl;
-  angularGlobalStateHacks.history = history;
-  angularGlobalStateHacks.kbnUrlStateStorage = kbnUrlStateStorage;
-
   if (!angularModuleInstance) {
     angularModuleInstance = createLocalAngularModule(deps.core, deps.navigation);
     // global routing stuff
     configureAppAngularModule(angularModuleInstance, deps.core as LegacyCoreStart, true);
     // custom routing stuff
-    deps.angularGlobalStateHacks = angularGlobalStateHacks;
     initDashboardApp(angularModuleInstance, deps);
   }
 
   const $injector = mountDashboardApp(appBasePath, element);
 
   return () => {
     $injector.get('$rootScope').$destroy();
-    stopSyncingGlobalState();
   };
 };
 

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx

@@ -42,6 +42,7 @@ import {
   Query,
   SavedQuery,
   syncAppFilters,
+  syncQuery,
 } from '../../../../../../plugins/data/public';
 
 import {
@@ -107,20 +108,21 @@ export class DashboardAppController {
     embeddables,
     share,
     dashboardCapabilities,
-    npDataStart: {
-      query: {
-        filterManager,
-        timefilter: { timefilter },
-      },
-    },
+    npDataStart: { query: queryService },
     core: { notifications, overlays, chrome, injectedMetadata, uiSettings, savedObjects, http },
-    angularGlobalStateHacks,
+    history,
+    kbnUrlStateStorage,
   }: DashboardAppControllerDependencies) {
-    const history = angularGlobalStateHacks!.history!;
-    const kbnUrlStateStorage = angularGlobalStateHacks!.kbnUrlStateStorage!;
-    const hasInheritedGlobalState = angularGlobalStateHacks!.hasInheritedGlobalState!;
-
+    const filterManager = queryService.filterManager;
     const queryFilter = filterManager;
+    const timefilter = queryService.timefilter.timefilter;
+
+    // starts syncing `_g` portion of url with query services
+    // note: dashboard_state_manager.ts syncs `_a` portion of url
+    const {
+      stop: stopSyncingGlobalStateWithUrl,
+      hasInheritedQueryFromUrl: hasInheritedGlobalStateFromUrl,
+    } = syncQuery(queryService, kbnUrlStateStorage);
 
     let lastReloadRequestTime = 0;
 
@@ -145,7 +147,7 @@ export class DashboardAppController {
 
     // The hash check is so we only update the time filter on dashboard open, not during
     // normal cross app navigation.
-    if (dashboardStateManager.getIsTimeSavedWithDashboard() && !hasInheritedGlobalState) {
+    if (dashboardStateManager.getIsTimeSavedWithDashboard() && !hasInheritedGlobalStateFromUrl) {
       dashboardStateManager.syncTimefilterWithDashboard(timefilter);
     }
     $scope.showSaveQuery = dashboardCapabilities.saveQuery as boolean;
@@ -884,6 +886,7 @@ export class DashboardAppController {
 
     $scope.$on('$destroy', () => {
       updateSubscription.unsubscribe();
+      stopSyncingGlobalStateWithUrl();
       stopSyncingAppFilters();
       visibleSubscription.unsubscribe();
       $scope.timefilterSubscriptions$.unsubscribe();

src/legacy/core_plugins/kibana/public/dashboard/np_ready/legacy_app.js

@@ -31,6 +31,7 @@ import {
 } from '../../../../../../plugins/kibana_utils/public';
 import { DashboardListing, EMPTY_FILTER } from './listing/dashboard_listing';
 import { addHelpMenuToAppChrome } from './help_menu/help_menu_util';
+import { syncQuery } from '../../../../../../plugins/data/public/query/state_sync';
 
 export function initDashboardApp(app, deps) {
   initDashboardAppDirective(app, deps);
@@ -88,10 +89,15 @@ export function initDashboardApp(app, deps) {
         template: dashboardListingTemplate,
         controller($injector, $location, $scope) {
           const service = deps.savedDashboards;
-
           const kbnUrl = $injector.get('kbnUrl');
           const dashboardConfig = deps.dashboardConfig;
 
+          // syncs `_g` portion of url with query services
+          const { stop: stopSyncingGlobalStateWithUrl } = syncQuery(
+            deps.npDataStart.query,
+            deps.kbnUrlStateStorage
+          );
+
           $scope.listingLimit = deps.uiSettings.get('savedObjects:listingLimit');
           $scope.create = () => {
             kbnUrl.redirect(DashboardConstants.CREATE_NEW_DASHBOARD_URL);
@@ -119,6 +125,10 @@ export function initDashboardApp(app, deps) {
           ]);
           addHelpMenuToAppChrome(deps.chrome, deps.core.docLinks);
           $scope.core = deps.core;
+
+          $scope.$on('$destroy', () => {
+            stopSyncingGlobalStateWithUrl();
+          });
         },
         resolve: {
           dash: function($rootScope, $route, redirectWhenMissing, kbnUrl) {
@@ -206,7 +216,7 @@ export function initDashboardApp(app, deps) {
                 // See https://github.com/elastic/kibana/issues/10951 for more context.
                 if (error instanceof SavedObjectNotFound && id === 'create') {
                   // Note preserve querystring part is necessary so the state is preserved through the redirect.
-                  const history = deps.angularGlobalStateHacks.history;
+                  const history = deps.history;
                   history.replace({
                     ...history.location, // preserve query,
                     pathname: DashboardConstants.CREATE_NEW_DASHBOARD_URL,

src/legacy/core_plugins/kibana/public/dashboard/plugin.ts

@@ -25,12 +25,13 @@ import {
   Plugin,
   SavedObjectsClientContract,
 } from 'kibana/public';
+import { createHashHistory } from 'history';
 import { i18n } from '@kbn/i18n';
 import { RenderDeps } from './np_ready/application';
 import { DataStart } from '../../../data/public';
 import { DataPublicPluginStart as NpDataStart } from '../../../../../plugins/data/public';
 import { IEmbeddableStart } from '../../../../../plugins/embeddable/public';
-import { Storage } from '../../../../../plugins/kibana_utils/public';
+import { createKbnUrlStateStorage, Storage } from '../../../../../plugins/kibana_utils/public';
 import { NavigationPublicPluginStart as NavigationStart } from '../../../../../plugins/navigation/public';
 import { DashboardConstants } from './np_ready/dashboard_constants';
 import {
@@ -96,6 +97,12 @@ export class DashboardPlugin implements Plugin {
           overlays: contextCore.overlays,
         });
 
+        const history = createHashHistory();
+        const kbnUrlStateStorage = createKbnUrlStateStorage({
+          history,
+          useHash: core.uiSettings.get('state:storeInSessionStorage'),
+        });
+
         const deps: RenderDeps = {
           core: contextCore as LegacyCoreStart,
           ...angularDependencies,
@@ -111,6 +118,8 @@ export class DashboardPlugin implements Plugin {
           embeddables,
           dashboardCapabilities: contextCore.application.capabilities.dashboard,
           localStorage: new Storage(localStorage),
+          history,
+          kbnUrlStateStorage,
         };
         const { renderApp } = await import('./np_ready/application');
         return renderApp(params.element, params.appBasePath, deps);