Fix context.pageName by fixing missing executionContext and add enabl…

…eExecutionContextTracking flag (elastic#204547) Resolves elastic#195778 ## 🐞 Summary This PR fixes missing executionContext in sharedux router by adding `SharedUXContext` to the `KibanaRootContextProvider` (inside of the `KibanaRenderContextProvider`). (More context provided in this elastic#195778 (comment)) It also introduces `enableExecutionContextTracking` to enable tracking logic to avoid creating a race condition for the existing custom execution context tracking implementations. I enabled this flag for the observability plugin and here is the difference: |Item|Screenshot| |---|---| |Before|![image](https://github.com/user-attachments/assets/83283d23-3347-45be-95c1-120cdfabb9c5)| |After|![image](https://github.com/user-attachments/assets/9de51645-6bf1-4537-baeb-6878e7bb3590)| ### 🧪 How to test - Go to the observability alerts page and check the kibana-browser request as shown above ### ✨ Possible future improvements Allowing this logic to be provided by the consumer so that we can get rid of custom implementations (Example: [APM custom execution context](https://github.com/elastic/kibana/blob/e9671937bacfaa911d32de0e8885e7f26425888a/x-pack/plugins/observability_solution/apm/public/components/routing/app_root/update_execution_context_on_route_change.ts#L21,L25)) --------- Co-authored-by: Anton Dosov <[email protected]> Co-authored-by: Davis McPhee <[email protected]> Co-authored-by: Marco Antonio Ghiani <[email protected]> Co-authored-by: Elena Stoeva <[email protected]>
gsoldevila · Dec 18, 2024 · 53748fd · 53748fd
1 parent c88b273
commit 53748fd
Show file tree

Hide file tree

Showing 16 changed files with 160 additions and 41 deletions.
diff --git a/packages/react/kibana_context/render/render_provider.tsx b/packages/react/kibana_context/render/render_provider.tsx
@@ -25,11 +25,11 @@ export type KibanaRenderContextProviderProps = Omit<KibanaRootContextProviderPro
 export const KibanaRenderContextProvider: FC<
   PropsWithChildren<KibanaRenderContextProviderProps>
 > = ({ children, ...props }) => {
-  const { analytics, i18n, theme, userProfile, colorMode, modify } = props;
+  const { analytics, executionContext, i18n, theme, userProfile, colorMode, modify } = props;
   return (
     <KibanaRootContextProvider
       globalStyles={false}
-      {...{ i18n, theme, userProfile, modify, colorMode }}
+      {...{ executionContext, i18n, theme, userProfile, modify, colorMode }}
     >
       <KibanaErrorBoundaryProvider analytics={analytics}>
         <KibanaErrorBoundary>{children}</KibanaErrorBoundary>

diff --git a/packages/react/kibana_context/root/BUILD.bazel b/packages/react/kibana_context/root/BUILD.bazel
@@ -26,6 +26,7 @@ DEPS = [
   "@npm//tslib",
   "@npm//@elastic/eui",
   "//packages/core/base/core-base-common",
+  "//packages/shared-ux/router/impl:shared-ux-router",
 ]
 
 js_library(

diff --git a/packages/react/kibana_context/root/root_provider.test.tsx b/packages/react/kibana_context/root/root_provider.test.tsx
@@ -15,6 +15,8 @@ import { useEuiTheme } from '@elastic/eui';
 import type { UseEuiTheme } from '@elastic/eui';
 import { mountWithIntl } from '@kbn/test-jest-helpers';
 import type { KibanaTheme } from '@kbn/react-kibana-context-common';
+import type { ExecutionContextStart } from '@kbn/core-execution-context-browser';
+import { executionContextServiceMock } from '@kbn/core-execution-context-browser-mocks';
 import { i18nServiceMock } from '@kbn/core-i18n-browser-mocks';
 import { I18nStart } from '@kbn/core-i18n-browser';
 import type { UserProfileService } from '@kbn/core-user-profile-browser';
@@ -25,11 +27,13 @@ describe('KibanaRootContextProvider', () => {
   let euiTheme: UseEuiTheme | undefined;
   let i18nMock: I18nStart;
   let userProfile: UserProfileService;
+  let executionContext: ExecutionContextStart;
 
   beforeEach(() => {
     euiTheme = undefined;
     i18nMock = i18nServiceMock.createStartContract();
     userProfile = userProfileServiceMock.createStart();
+    executionContext = executionContextServiceMock.createStartContract();
   });
 
   const flushPromises = async () => {
@@ -64,6 +68,7 @@ describe('KibanaRootContextProvider', () => {
       <KibanaRootContextProvider
         i18n={i18nMock}
         userProfile={userProfile}
+        executionContext={executionContext}
         theme={{ theme$: of(coreTheme) }}
       >
         <InnerComponent />
@@ -82,6 +87,7 @@ describe('KibanaRootContextProvider', () => {
       <KibanaRootContextProvider
         i18n={i18nMock}
         userProfile={userProfile}
+        executionContext={executionContext}
         theme={{ theme$: coreTheme$ }}
       >
         <InnerComponent />

diff --git a/packages/react/kibana_context/root/root_provider.tsx b/packages/react/kibana_context/root/root_provider.tsx
@@ -11,6 +11,8 @@ import React, { FC, PropsWithChildren } from 'react';
 
 import type { AnalyticsServiceStart } from '@kbn/core-analytics-browser';
 import type { I18nStart } from '@kbn/core-i18n-browser';
+import type { ExecutionContextStart } from '@kbn/core-execution-context-browser';
+import { SharedUXRouterContext } from '@kbn/shared-ux-router';
 
 // @ts-expect-error EUI exports this component internally, but Kibana isn't picking it up its types
 import { useIsNestedEuiProvider } from '@elastic/eui/lib/components/provider/nested';
@@ -25,6 +27,8 @@ export interface KibanaRootContextProviderProps extends KibanaEuiProviderProps {
   i18n: I18nStart;
   /** The `AnalyticsServiceStart` API from `CoreStart`. */
   analytics?: Pick<AnalyticsServiceStart, 'reportEvent'>;
+  /** The `ExecutionContextStart` API from `CoreStart`. */
+  executionContext?: ExecutionContextStart;
 }
 
 /**
@@ -44,20 +48,26 @@ export interface KibanaRootContextProviderProps extends KibanaEuiProviderProps {
 export const KibanaRootContextProvider: FC<PropsWithChildren<KibanaRootContextProviderProps>> = ({
   children,
   i18n,
+  executionContext,
   ...props
 }) => {
   const hasEuiProvider = useIsNestedEuiProvider();
+  const rootContextProvider = (
+    <SharedUXRouterContext.Provider value={{ services: { executionContext } }}>
+      <i18n.Context>{children}</i18n.Context>
+    </SharedUXRouterContext.Provider>
+  );
 
   if (hasEuiProvider) {
     emitEuiProviderWarning(
       'KibanaRootContextProvider has likely been nested in this React tree, either by direct reference or by KibanaRenderContextProvider.  The result of this nesting is a nesting of EuiProvider, which has negative effects.  Check your React tree for nested Kibana context providers.'
     );
-    return <i18n.Context>{children}</i18n.Context>;
+    return rootContextProvider;
   } else {
     const { theme, userProfile, globalStyles, colorMode, modify } = props;
     return (
       <KibanaEuiProvider {...{ theme, userProfile, globalStyles, colorMode, modify }}>
-        <i18n.Context>{children}</i18n.Context>
+        {rootContextProvider}
       </KibanaEuiProvider>
     );
   }

diff --git a/packages/react/kibana_context/root/tsconfig.json b/packages/react/kibana_context/root/tsconfig.json
@@ -24,5 +24,8 @@
     "@kbn/core-analytics-browser",
     "@kbn/core-user-profile-browser",
     "@kbn/core-user-profile-browser-mocks",
+    "@kbn/core-execution-context-browser",
+    "@kbn/core-execution-context-browser-mocks",
+    "@kbn/shared-ux-router"
   ]
 }
diff --git a/packages/shared-ux/router/impl/BUILD.bazel b/packages/shared-ux/router/impl/BUILD.bazel
@@ -0,0 +1,34 @@
+load("@build_bazel_rules_nodejs//:index.bzl", "js_library")
+
+SRCS = glob(
+  [
+    "**/*.ts",
+    "**/*.tsx",
+  ],
+  exclude = [
+    "**/test_helpers.ts",
+    "**/*.config.js",
+    "**/*.mock.*",
+    "**/*.test.*",
+    "**/*.stories.*",
+    "**/__snapshots__/**",
+    "**/integration_tests/**",
+    "**/mocks/**",
+    "**/scripts/**",
+    "**/storybook/**",
+    "**/test_fixtures/**",
+    "**/test_helpers/**",
+  ],
+)
+
+DEPS = [
+
+]
+
+js_library(
+  name = "shared-ux-router",
+  package_name = "@kbn/shared-ux-router",
+  srcs = ["package.json"] + SRCS,
+  deps = DEPS,
+  visibility = ["//visibility:public"],
+)
diff --git a/packages/shared-ux/router/impl/__snapshots__/route.test.tsx.snap b/packages/shared-ux/router/impl/__snapshots__/route.test.tsx.snap
diff --git a/packages/shared-ux/router/impl/index.ts b/packages/shared-ux/router/impl/index.ts
@@ -10,3 +10,5 @@
 export { Route } from './route';
 export { HashRouter, BrowserRouter, MemoryRouter, Router } from './router';
 export { Routes } from './routes';
+
+export { SharedUXRouterContext } from './services';
diff --git a/packages/shared-ux/router/impl/route.test.tsx b/packages/shared-ux/router/impl/route.test.tsx
@@ -9,15 +9,34 @@
 
 import React, { Component, FC } from 'react';
 import { shallow } from 'enzyme';
+import { useSharedUXRoutesContext } from './routes_context';
 import { Route } from './route';
 import { createMemoryHistory } from 'history';
 
+jest.mock('./routes_context', () => ({
+  useSharedUXRoutesContext: jest.fn().mockImplementation(() => ({
+    enableExecutionContextTracking: true,
+  })),
+}));
+
 describe('Route', () => {
+  beforeEach(() => {
+    jest.restoreAllMocks();
+  });
+
   test('renders', () => {
     const example = shallow(<Route />);
     expect(example).toMatchSnapshot();
   });
 
+  test('renders with enableExecutionContextTracking as false', () => {
+    (useSharedUXRoutesContext as jest.Mock).mockImplementationOnce(() => ({
+      enableExecutionContextTracking: false,
+    }));
+    const example = shallow(<Route />);
+    expect(example).toMatchSnapshot();
+  });
+
   test('location renders as expected', () => {
     // create a history
     const historyLocation = createMemoryHistory();

diff --git a/packages/shared-ux/router/impl/route.tsx b/packages/shared-ux/router/impl/route.tsx
@@ -15,6 +15,7 @@ import {
   RouteProps,
   useRouteMatch,
 } from 'react-router-dom';
+import { useSharedUXRoutesContext } from './routes_context';
 import { useKibanaSharedUX } from './services';
 import { useSharedUXExecutionContext } from './use_execution_context';
 
@@ -30,17 +31,18 @@ export const Route = <T extends {}>({
   render,
   ...rest
 }: RouteProps<string, { [K: string]: string } & T>) => {
+  const { enableExecutionContextTracking } = useSharedUXRoutesContext();
   const component = useMemo(() => {
     if (!Component) {
       return undefined;
     }
     return (props: RouteComponentProps) => (
       <>
-        <MatchPropagator />
+        {enableExecutionContextTracking && <MatchPropagator />}
         <Component {...props} />
       </>
     );
-  }, [Component]);
+  }, [Component, enableExecutionContextTracking]);
 
   if (component) {
     return <ReactRouterRoute {...rest} component={component} />;
@@ -52,7 +54,7 @@ export const Route = <T extends {}>({
         {...rest}
         render={(props) => (
           <>
-            <MatchPropagator />
+            {enableExecutionContextTracking && <MatchPropagator />}
             {/* @ts-ignore  else condition exists if renderFunction is undefined*/}
             {renderFunction(props)}
           </>
@@ -62,7 +64,7 @@ export const Route = <T extends {}>({
   }
   return (
     <ReactRouterRoute {...rest}>
-      <MatchPropagator />
+      {enableExecutionContextTracking && <MatchPropagator />}
       {children}
     </ReactRouterRoute>
   );
@@ -75,6 +77,12 @@ export const MatchPropagator = () => {
   const { executionContext } = useKibanaSharedUX().services;
   const match = useRouteMatch();
 
+  if (!executionContext && process.env.NODE_ENV !== 'production') {
+    throw new Error(
+      'Default execution context tracking is enabled but the executionContext service is not available'
+    );
+  }
+
   useSharedUXExecutionContext(executionContext, {
     type: 'application',
     page: match.path,

diff --git a/packages/shared-ux/router/impl/routes.tsx b/packages/shared-ux/router/impl/routes.tsx
@@ -13,6 +13,7 @@ import React, { Children } from 'react';
 import { Switch, useRouteMatch } from 'react-router-dom';
 import { Routes as ReactRouterRoutes, Route } from 'react-router-dom-v5-compat';
 import { Route as LegacyRoute, MatchPropagator } from './route';
+import { SharedUXRoutesContext } from './routes_context';
 
 type RouterElementChildren = Array<
   React.ReactElement<
@@ -28,42 +29,47 @@ type RouterElementChildren = Array<
 
 export const Routes = ({
   legacySwitch = true,
+  enableExecutionContextTracking = false,
   children,
 }: {
   legacySwitch?: boolean;
+  enableExecutionContextTracking?: boolean;
   children: React.ReactNode;
 }) => {
   const match = useRouteMatch();
 
   return legacySwitch ? (
-    <Switch>{children}</Switch>
+    <SharedUXRoutesContext.Provider value={{ enableExecutionContextTracking }}>
+      <Switch>{children}</Switch>
+    </SharedUXRoutesContext.Provider>
   ) : (
-    <ReactRouterRoutes>
-      {Children.map(children as RouterElementChildren, (child) => {
-        if (React.isValidElement(child) && child.type === LegacyRoute) {
-          const path = replace(child?.props.path, match.url + '/', '');
-          const renderFunction =
-            typeof child?.props.children === 'function'
-              ? child?.props.children
-              : child?.props.render;
-
-          return (
-            <Route
-              path={path}
-              element={
-                <>
-                  <MatchPropagator />
-                  {(child?.props?.component && <child.props.component />) ||
-                    (renderFunction && renderFunction()) ||
-                    children}
-                </>
-              }
-            />
-          );
-        } else {
-          return child;
-        }
-      })}
-    </ReactRouterRoutes>
+    <SharedUXRoutesContext.Provider value={{ enableExecutionContextTracking }}>
+      <ReactRouterRoutes>
+        {Children.map(children as RouterElementChildren, (child) => {
+          if (React.isValidElement(child) && child.type === LegacyRoute) {
+            const path = replace(child?.props.path, match.url + '/', '');
+            const renderFunction =
+              typeof child?.props.children === 'function'
+                ? child?.props.children
+                : child?.props.render;
+            return (
+              <Route
+                path={path}
+                element={
+                  <>
+                    {enableExecutionContextTracking && <MatchPropagator />}
+                    {(child?.props?.component && <child.props.component />) ||
+                      (renderFunction && renderFunction()) ||
+                      children}
+                  </>
+                }
+              />
+            );
+          } else {
+            return child;
+          }
+        })}
+      </ReactRouterRoutes>
+    </SharedUXRoutesContext.Provider>
   );
 };
diff --git a/packages/shared-ux/router/impl/routes_context.ts b/packages/shared-ux/router/impl/routes_context.ts
@@ -0,0 +1,18 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+import { createContext, useContext } from 'react';
+import { SharedUXRoutesContextType } from './types';
+
+const defaultContextValue = {};
+
+export const SharedUXRoutesContext = createContext<SharedUXRoutesContextType>(defaultContextValue);
+
+export const useSharedUXRoutesContext = (): SharedUXRoutesContextType =>
+  useContext(SharedUXRoutesContext as unknown as React.Context<SharedUXRoutesContextType>);
diff --git a/packages/shared-ux/router/impl/services.ts b/packages/shared-ux/router/impl/services.ts
@@ -50,7 +50,7 @@ export interface SharedUXExecutionContextSetup {
  */
 export interface SharedUXExecutionContextSetup {
   /** {@link SharedUXExecutionContextSetup} */
-  executionContext: SharedUXExecutionContextStart;
+  executionContext?: SharedUXExecutionContextStart;
 }
 
 export type KibanaServices = Partial<SharedUXExecutionContextSetup>;
@@ -63,12 +63,14 @@ const defaultContextValue = {
   services: {},
 };
 
-export const sharedUXContext =
+export const SharedUXRouterContext =
   createContext<SharedUXRouterContextValue<KibanaServices>>(defaultContextValue);
 
 export const useKibanaSharedUX = <Extra extends object = {}>(): SharedUXRouterContextValue<
   KibanaServices & Extra
 > =>
   useContext(
-    sharedUXContext as unknown as React.Context<SharedUXRouterContextValue<KibanaServices & Extra>>
+    SharedUXRouterContext as unknown as React.Context<
+      SharedUXRouterContextValue<KibanaServices & Extra>
+    >
   );
diff --git a/packages/shared-ux/router/impl/types.ts b/packages/shared-ux/router/impl/types.ts
@@ -33,3 +33,11 @@ export declare interface SharedUXExecutionContext {
   /** an inner context spawned from the current context. */
   child?: SharedUXExecutionContext;
 }
+
+export declare interface SharedUXRoutesContextType {
+  /**
+   * This flag is used to enable the default execution context tracking for a specific router.
+   * Enable this flag in case you don't have a custom implementation for execution context tracking.
+   * */
+  readonly enableExecutionContextTracking?: boolean;
+}