ui: plumb explicitly Trace through Pages

- Rework the pages a bit. Now pages need to declare if they require a trace or not. If they don't, the router will just fall back on the homepage if trying to open a page like /viewer or /query before loading a trace. This is necessary to ensure we never end up on ViewerPage or QueryPage without a valid engine. - Inject the App interface down into pages and stop using globals for accessing the engine. Change-Id: I33c00373ae6429bfe48d0cf0235b3381a9f3f594
google · Sep 23, 2024 · 59e2eb0 · 59e2eb0
1 parent f79657e
commit 59e2eb0
Show file tree

Hide file tree

Showing 18 changed files with 211 additions and 221 deletions.
diff --git a/ui/src/frontend/flags_page.ts b/ui/src/frontend/flags_page.ts
@@ -16,7 +16,7 @@ import m from 'mithril';
 import {channelChanged, getNextChannel, setChannel} from '../common/channels';
 import {featureFlags, Flag, OverrideState} from '../core/feature_flags';
 import {raf} from '../core/raf_scheduler';
-import {createPage} from './pages';
+import {PageAttrs} from './pages';
 import {Router} from './router';
 
 const RELEASE_PROCESS_URL =
@@ -100,7 +100,7 @@ class FlagWidget implements m.ClassComponent<FlagWidgetAttrs> {
   }
 }
 
-export const FlagsPage = createPage({
+export class FlagsPage implements m.ClassComponent<PageAttrs> {
   view() {
     const needsReload = channelChanged();
     return m(
@@ -147,16 +147,15 @@ export const FlagsPage = createPage({
         featureFlags.allFlags().map((flag) => m(FlagWidget, {flag})),
       ),
     );
-  },
+  }
 
-  oncreate(vnode: m.VnodeDOM) {
-    const route = Router.parseUrl(window.location.href);
-    const flagId = /[/](\w+)/.exec(route.subpage)?.slice(1, 2)[0];
+  oncreate(vnode: m.VnodeDOM<PageAttrs>) {
+    const flagId = /[/](\w+)/.exec(vnode.attrs.subpage ?? '')?.slice(1, 2)[0];
     if (flagId) {
       const flag = vnode.dom.querySelector(`#${flagId}`);
       if (flag) {
         flag.scrollIntoView({block: 'center'});
       }
     }
-  },
-});
+  }
+}
diff --git a/ui/src/frontend/home_page.ts b/ui/src/frontend/home_page.ts
@@ -17,7 +17,7 @@ import {channelChanged, getNextChannel, setChannel} from '../common/channels';
 import {Anchor} from '../widgets/anchor';
 import {HotkeyGlyphs} from '../widgets/hotkey_glyphs';
 import {globals} from './globals';
-import {createPage} from './pages';
+import {PageAttrs} from './pages';
 
 export class Hints implements m.ClassComponent {
   view() {
@@ -66,7 +66,7 @@ export class Hints implements m.ClassComponent {
   }
 }
 
-export const HomePage = createPage({
+export class HomePage implements m.ClassComponent<PageAttrs> {
   view() {
     return m(
       '.page.home-page',
@@ -94,8 +94,8 @@ export const HomePage = createPage({
         'Privacy policy',
       ),
     );
-  },
-});
+  }
+}
 
 function mkChan(chan: string) {
   const checked = getNextChannel() === chan ? '[checked=true]' : '';

diff --git a/ui/src/frontend/index.ts b/ui/src/frontend/index.ts
@@ -60,6 +60,7 @@ import {showModal} from '../widgets/modal';
 import {initAnalytics} from './analytics';
 import {IdleDetector} from './idle_detector';
 import {IdleDetectorWindow} from './idle_detector_interface';
+import {pageWithTrace} from './pages';
 
 const EXTENSION_ID = 'lfmkphfpdbjijhpomgecfikhfohaoine';
 
@@ -289,16 +290,16 @@ function onCssLoaded() {
 
   const router = new Router({
     '/': HomePage,
-    '/viewer': ViewerPage,
-    '/record': RECORDING_V2_FLAG.get() ? RecordPageV2 : RecordPage,
-    '/query': QueryPage,
-    '/insights': InsightsPage,
     '/flags': FlagsPage,
-    '/metrics': MetricsPage,
-    '/info': TraceInfoPage,
-    '/widgets': WidgetsPage,
-    '/viz': VizPage,
+    '/info': pageWithTrace(TraceInfoPage),
+    '/insights': pageWithTrace(InsightsPage),
+    '/metrics': pageWithTrace(MetricsPage),
     '/plugins': PluginsPage,
+    '/query': pageWithTrace(QueryPage),
+    '/record': RECORDING_V2_FLAG.get() ? RecordPageV2 : RecordPage,
+    '/viewer': pageWithTrace(ViewerPage),
+    '/viz': pageWithTrace(VizPage),
+    '/widgets': WidgetsPage,
   });
   router.onRouteChanged = routeChange;
 

diff --git a/ui/src/frontend/insights_page.ts b/ui/src/frontend/insights_page.ts
@@ -13,10 +13,10 @@
 // limitations under the License.
 
 import m from 'mithril';
-import {createPage} from './pages';
+import {PageWithTraceAttrs} from './pages';
 
-export const InsightsPage = createPage({
+export class InsightsPage implements m.ClassComponent<PageWithTraceAttrs> {
   view() {
     return m('.insights-page');
-  },
-});
+  }
+}
diff --git a/ui/src/frontend/metrics_page.ts b/ui/src/frontend/metrics_page.ts
@@ -29,21 +29,12 @@ import {STR} from '../trace_processor/query_result';
 import {Select} from '../widgets/select';
 import {Spinner} from '../widgets/spinner';
 import {VegaView} from '../widgets/vega_view';
-import {globals} from './globals';
-import {createPage} from './pages';
+import {PageWithTraceAttrs} from './pages';
+import {assertExists} from '../base/logging';
 
 type Format = 'json' | 'prototext' | 'proto';
 const FORMATS: Format[] = ['json', 'prototext', 'proto'];
 
-function getEngine(): Engine | undefined {
-  const engineId = globals.getCurrentEngine()?.id;
-  if (engineId === undefined) {
-    return undefined;
-  }
-  const engine = globals.engines.get(engineId)?.getProxy('MetricsPage');
-  return engine;
-}
-
 async function getMetrics(engine: Engine): Promise<string[]> {
   const metrics: string[] = [];
   const metricsResult = await engine.query('select name from trace_metrics');
@@ -251,25 +242,19 @@ class MetricVizView implements m.ClassComponent<MetricVizViewAttrs> {
   }
 }
 
-class MetricPageContents implements m.ClassComponent {
-  controller?: MetricsController;
+export class MetricsPage implements m.ClassComponent<PageWithTraceAttrs> {
+  private controller?: MetricsController;
 
-  oncreate() {
-    const engine = getEngine();
-    if (engine !== undefined) {
-      this.controller = new MetricsController(pluginManager, engine);
-    }
+  oninit({attrs}: m.Vnode<PageWithTraceAttrs>) {
+    const engine = attrs.trace.engine.getProxy('MetricsPage');
+    this.controller = new MetricsController(pluginManager, engine);
   }
 
   view() {
-    const controller = this.controller;
-    if (controller === undefined) {
-      return m('');
-    }
-
+    const controller = assertExists(this.controller);
     const json = controller.resultAsJson;
-
-    return [
+    return m(
+      '.metrics-page',
       m(MetricPicker, {
         controller,
       }),
@@ -282,12 +267,6 @@ class MetricPageContents implements m.ClassComponent {
           return m(MetricVizView, {visualisation, data});
         }),
       m(MetricResultView, {result: controller.result}),
-    ];
+    );
   }
 }
-
-export const MetricsPage = createPage({
-  view() {
-    return m('.metrics-page', m(MetricPageContents));
-  },
-});
diff --git a/ui/src/frontend/pages.ts b/ui/src/frontend/pages.ts
@@ -13,14 +13,29 @@
 // limitations under the License.
 
 import m from 'mithril';
-
-// Wrap component with common UI elements (nav bar etc).
-export function createPage(
-  component: m.Component<PageAttrs>,
-): m.Component<PageAttrs> {
-  return component;
-}
+import {AppImpl, TraceImpl} from '../core/app_trace_impl';
+import {HomePage} from './home_page';
 
 export interface PageAttrs {
   subpage?: string;
 }
+
+export interface PageWithTraceAttrs extends PageAttrs {
+  trace: TraceImpl;
+}
+
+export function pageWithTrace(
+  component: m.ComponentTypes<PageWithTraceAttrs>,
+): m.Component<PageAttrs> {
+  return {
+    view(vnode: m.Vnode<PageAttrs>) {
+      const trace = AppImpl.instance.trace;
+      if (trace !== undefined) {
+        return m(component, {...vnode.attrs, trace});
+      }
+      // Fallback on homepage if trying to open a page that requires a trace
+      // while no trace is loaded.
+      return m(HomePage);
+    },
+  };
+}
diff --git a/ui/src/frontend/plugins_page.ts b/ui/src/frontend/plugins_page.ts
@@ -18,11 +18,11 @@ import {raf} from '../core/raf_scheduler';
 import {Button} from '../widgets/button';
 import {exists} from '../base/utils';
 import {PluginDescriptor} from '../public/plugin';
-import {createPage} from './pages';
 import {defaultPlugins} from '../core/default_plugins';
 import {Intent} from '../widgets/common';
+import {PageAttrs} from './pages';
 
-export const PluginsPage = createPage({
+export class PluginsPage implements m.ClassComponent<PageAttrs> {
   view() {
     return m(
       '.pf-plugins-page',
@@ -79,8 +79,8 @@ export const PluginsPage = createPage({
         }),
       ),
     );
-  },
-});
+  }
+}
 
 function renderPluginRow(plugin: PluginDescriptor): m.Children {
   const pluginId = plugin.pluginId;

diff --git a/ui/src/frontend/query_page.ts b/ui/src/frontend/query_page.ts
@@ -17,14 +17,14 @@ import {SimpleResizeObserver} from '../base/resize_observer';
 import {undoCommonChatAppReplacements} from '../base/string_utils';
 import {QueryResponse, runQuery} from '../common/queries';
 import {raf} from '../core/raf_scheduler';
-import {Engine} from '../trace_processor/engine';
 import {Callout} from '../widgets/callout';
 import {Editor} from '../widgets/editor';
-import {globals} from './globals';
-import {createPage} from './pages';
+import {PageWithTraceAttrs} from './pages';
 import {QueryHistoryComponent, queryHistoryStorage} from './query_history';
 import {addQueryResultsTab} from './query_result_tab';
 import {QueryTable} from './query_table';
+import {Engine, EngineAttrs} from '../trace_processor/engine';
+import {assertExists} from '../base/logging';
 
 interface QueryPageState {
   enteredText: string;
@@ -40,47 +40,35 @@ const state: QueryPageState = {
   generation: 0,
 };
 
-function runManualQuery(query: string) {
+function runManualQuery(engine: Engine, query: string) {
   state.executedQuery = query;
   state.queryResult = undefined;
-  const engine = getEngine();
-  if (engine) {
-    runQuery(undoCommonChatAppReplacements(query), engine).then(
-      (resp: QueryResponse) => {
-        addQueryResultsTab(
-          {
-            query: query,
-            title: 'Standalone Query',
-            prefetchedResponse: resp,
-          },
-          'analyze_page_query',
-        );
-        // We might have started to execute another query. Ignore it in that
-        // case.
-        if (state.executedQuery !== query) {
-          return;
-        }
-        state.queryResult = resp;
-        raf.scheduleFullRedraw();
-      },
-    );
-  }
+  runQuery(undoCommonChatAppReplacements(query), engine).then(
+    (resp: QueryResponse) => {
+      addQueryResultsTab(
+        {
+          query: query,
+          title: 'Standalone Query',
+          prefetchedResponse: resp,
+        },
+        'analyze_page_query',
+      );
+      // We might have started to execute another query. Ignore it in that
+      // case.
+      if (state.executedQuery !== query) {
+        return;
+      }
+      state.queryResult = resp;
+      raf.scheduleFullRedraw();
+    },
+  );
   raf.scheduleDelayedFullRedraw();
 }
 
-function getEngine(): Engine | undefined {
-  const engineId = globals.getCurrentEngine()?.id;
-  if (engineId === undefined) {
-    return undefined;
-  }
-  const engine = globals.engines.get(engineId)?.getProxy('QueryPage');
-  return engine;
-}
-
-class QueryInput implements m.ClassComponent {
+class QueryInput implements m.ClassComponent<EngineAttrs> {
   private resize?: Disposable;
 
-  oncreate({dom}: m.CVnodeDOM): void {
+  oncreate({dom}: m.CVnodeDOM<EngineAttrs>): void {
     this.resize = new SimpleResizeObserver(dom, () => {
       state.heightPx = (dom as HTMLElement).style.height;
     });
@@ -94,7 +82,7 @@ class QueryInput implements m.ClassComponent {
     }
   }
 
-  view() {
+  view({attrs}: m.CVnode<EngineAttrs>) {
     return m(Editor, {
       generation: state.generation,
       initialText: state.enteredText,
@@ -104,7 +92,7 @@ class QueryInput implements m.ClassComponent {
           return;
         }
         queryHistoryStorage.saveQuery(text);
-        runManualQuery(text);
+        runManualQuery(attrs.engine, text);
       },
 
       onUpdate: (text: string) => {
@@ -115,8 +103,15 @@ class QueryInput implements m.ClassComponent {
   }
 }
 
-export const QueryPage = createPage({
+export class QueryPage implements m.ClassComponent<PageWithTraceAttrs> {
+  private engine?: Engine;
+
+  oninit({attrs}: m.CVnode<PageWithTraceAttrs>) {
+    this.engine = attrs.trace.engine.getProxy('QueryPage');
+  }
+
   view() {
+    const engine = assertExists(this.engine);
     return m(
       '.query-page',
       m(Callout, 'Enter query and press Cmd/Ctrl + Enter'),
@@ -128,7 +123,7 @@ export const QueryPage = createPage({
             `define a string, please use ' (single quote) instead. Using double quotes ` +
             `can cause subtle problems which are very hard to debug.`,
         ),
-      m(QueryInput),
+      m(QueryInput, {engine}),
       state.executedQuery === undefined
         ? null
         : m(QueryTable, {
@@ -137,13 +132,13 @@ export const QueryPage = createPage({
             fillParent: false,
           }),
       m(QueryHistoryComponent, {
-        runQuery: runManualQuery,
+        runQuery: (q: string) => runManualQuery(engine, q),
         setQuery: (q: string) => {
           state.enteredText = q;
           state.generation++;
           raf.scheduleFullRedraw();
         },
       }),
     );
-  },
-});
+  }
+}