Skip to content

Commit 12d16f3

Browse files
maryam-saeidiDosantdavismcpheeMarco Antonio GhianiElenaStoeva
authored andcommitted
Fix context.pageName by fixing missing executionContext and add enableExecutionContextTracking 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]>
1 parent 04a00a0 commit 12d16f3

File tree

16 files changed

+160
-41
lines changed

16 files changed

+160
-41
lines changed

packages/react/kibana_context/render/render_provider.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ export type KibanaRenderContextProviderProps = Omit<KibanaRootContextProviderPro
2525
export const KibanaRenderContextProvider: FC<
2626
PropsWithChildren<KibanaRenderContextProviderProps>
2727
> = ({ children, ...props }) => {
28-
const { analytics, i18n, theme, userProfile, colorMode, modify } = props;
28+
const { analytics, executionContext, i18n, theme, userProfile, colorMode, modify } = props;
2929
return (
3030
<KibanaRootContextProvider
3131
globalStyles={false}
32-
{...{ i18n, theme, userProfile, modify, colorMode }}
32+
{...{ executionContext, i18n, theme, userProfile, modify, colorMode }}
3333
>
3434
<KibanaErrorBoundaryProvider analytics={analytics}>
3535
<KibanaErrorBoundary>{children}</KibanaErrorBoundary>

packages/react/kibana_context/root/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ DEPS = [
2626
"@npm//tslib",
2727
"@npm//@elastic/eui",
2828
"//packages/core/base/core-base-common",
29+
"//packages/shared-ux/router/impl:shared-ux-router",
2930
]
3031

3132
js_library(

packages/react/kibana_context/root/root_provider.test.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import { useEuiTheme } from '@elastic/eui';
1515
import type { UseEuiTheme } from '@elastic/eui';
1616
import { mountWithIntl } from '@kbn/test-jest-helpers';
1717
import type { KibanaTheme } from '@kbn/react-kibana-context-common';
18+
import type { ExecutionContextStart } from '@kbn/core-execution-context-browser';
19+
import { executionContextServiceMock } from '@kbn/core-execution-context-browser-mocks';
1820
import { i18nServiceMock } from '@kbn/core-i18n-browser-mocks';
1921
import { I18nStart } from '@kbn/core-i18n-browser';
2022
import type { UserProfileService } from '@kbn/core-user-profile-browser';
@@ -25,11 +27,13 @@ describe('KibanaRootContextProvider', () => {
2527
let euiTheme: UseEuiTheme | undefined;
2628
let i18nMock: I18nStart;
2729
let userProfile: UserProfileService;
30+
let executionContext: ExecutionContextStart;
2831

2932
beforeEach(() => {
3033
euiTheme = undefined;
3134
i18nMock = i18nServiceMock.createStartContract();
3235
userProfile = userProfileServiceMock.createStart();
36+
executionContext = executionContextServiceMock.createStartContract();
3337
});
3438

3539
const flushPromises = async () => {
@@ -64,6 +68,7 @@ describe('KibanaRootContextProvider', () => {
6468
<KibanaRootContextProvider
6569
i18n={i18nMock}
6670
userProfile={userProfile}
71+
executionContext={executionContext}
6772
theme={{ theme$: of(coreTheme) }}
6873
>
6974
<InnerComponent />
@@ -82,6 +87,7 @@ describe('KibanaRootContextProvider', () => {
8287
<KibanaRootContextProvider
8388
i18n={i18nMock}
8489
userProfile={userProfile}
90+
executionContext={executionContext}
8591
theme={{ theme$: coreTheme$ }}
8692
>
8793
<InnerComponent />

packages/react/kibana_context/root/root_provider.tsx

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import React, { FC, PropsWithChildren } from 'react';
1111

1212
import type { AnalyticsServiceStart } from '@kbn/core-analytics-browser';
1313
import type { I18nStart } from '@kbn/core-i18n-browser';
14+
import type { ExecutionContextStart } from '@kbn/core-execution-context-browser';
15+
import { SharedUXRouterContext } from '@kbn/shared-ux-router';
1416

1517
// @ts-expect-error EUI exports this component internally, but Kibana isn't picking it up its types
1618
import { useIsNestedEuiProvider } from '@elastic/eui/lib/components/provider/nested';
@@ -25,6 +27,8 @@ export interface KibanaRootContextProviderProps extends KibanaEuiProviderProps {
2527
i18n: I18nStart;
2628
/** The `AnalyticsServiceStart` API from `CoreStart`. */
2729
analytics?: Pick<AnalyticsServiceStart, 'reportEvent'>;
30+
/** The `ExecutionContextStart` API from `CoreStart`. */
31+
executionContext?: ExecutionContextStart;
2832
}
2933

3034
/**
@@ -44,20 +48,26 @@ export interface KibanaRootContextProviderProps extends KibanaEuiProviderProps {
4448
export const KibanaRootContextProvider: FC<PropsWithChildren<KibanaRootContextProviderProps>> = ({
4549
children,
4650
i18n,
51+
executionContext,
4752
...props
4853
}) => {
4954
const hasEuiProvider = useIsNestedEuiProvider();
55+
const rootContextProvider = (
56+
<SharedUXRouterContext.Provider value={{ services: { executionContext } }}>
57+
<i18n.Context>{children}</i18n.Context>
58+
</SharedUXRouterContext.Provider>
59+
);
5060

5161
if (hasEuiProvider) {
5262
emitEuiProviderWarning(
5363
'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.'
5464
);
55-
return <i18n.Context>{children}</i18n.Context>;
65+
return rootContextProvider;
5666
} else {
5767
const { theme, userProfile, globalStyles, colorMode, modify } = props;
5868
return (
5969
<KibanaEuiProvider {...{ theme, userProfile, globalStyles, colorMode, modify }}>
60-
<i18n.Context>{children}</i18n.Context>
70+
{rootContextProvider}
6171
</KibanaEuiProvider>
6272
);
6373
}

packages/react/kibana_context/root/tsconfig.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,8 @@
2424
"@kbn/core-analytics-browser",
2525
"@kbn/core-user-profile-browser",
2626
"@kbn/core-user-profile-browser-mocks",
27+
"@kbn/core-execution-context-browser",
28+
"@kbn/core-execution-context-browser-mocks",
29+
"@kbn/shared-ux-router"
2730
]
2831
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
load("@build_bazel_rules_nodejs//:index.bzl", "js_library")
2+
3+
SRCS = glob(
4+
[
5+
"**/*.ts",
6+
"**/*.tsx",
7+
],
8+
exclude = [
9+
"**/test_helpers.ts",
10+
"**/*.config.js",
11+
"**/*.mock.*",
12+
"**/*.test.*",
13+
"**/*.stories.*",
14+
"**/__snapshots__/**",
15+
"**/integration_tests/**",
16+
"**/mocks/**",
17+
"**/scripts/**",
18+
"**/storybook/**",
19+
"**/test_fixtures/**",
20+
"**/test_helpers/**",
21+
],
22+
)
23+
24+
DEPS = [
25+
26+
]
27+
28+
js_library(
29+
name = "shared-ux-router",
30+
package_name = "@kbn/shared-ux-router",
31+
srcs = ["package.json"] + SRCS,
32+
deps = DEPS,
33+
visibility = ["//visibility:public"],
34+
)

packages/shared-ux/router/impl/__snapshots__/route.test.tsx.snap

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/shared-ux/router/impl/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,5 @@
1010
export { Route } from './route';
1111
export { HashRouter, BrowserRouter, MemoryRouter, Router } from './router';
1212
export { Routes } from './routes';
13+
14+
export { SharedUXRouterContext } from './services';

packages/shared-ux/router/impl/route.test.tsx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,34 @@
99

1010
import React, { Component, FC } from 'react';
1111
import { shallow } from 'enzyme';
12+
import { useSharedUXRoutesContext } from './routes_context';
1213
import { Route } from './route';
1314
import { createMemoryHistory } from 'history';
1415

16+
jest.mock('./routes_context', () => ({
17+
useSharedUXRoutesContext: jest.fn().mockImplementation(() => ({
18+
enableExecutionContextTracking: true,
19+
})),
20+
}));
21+
1522
describe('Route', () => {
23+
beforeEach(() => {
24+
jest.restoreAllMocks();
25+
});
26+
1627
test('renders', () => {
1728
const example = shallow(<Route />);
1829
expect(example).toMatchSnapshot();
1930
});
2031

32+
test('renders with enableExecutionContextTracking as false', () => {
33+
(useSharedUXRoutesContext as jest.Mock).mockImplementationOnce(() => ({
34+
enableExecutionContextTracking: false,
35+
}));
36+
const example = shallow(<Route />);
37+
expect(example).toMatchSnapshot();
38+
});
39+
2140
test('location renders as expected', () => {
2241
// create a history
2342
const historyLocation = createMemoryHistory();

packages/shared-ux/router/impl/route.tsx

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
RouteProps,
1616
useRouteMatch,
1717
} from 'react-router-dom';
18+
import { useSharedUXRoutesContext } from './routes_context';
1819
import { useKibanaSharedUX } from './services';
1920
import { useSharedUXExecutionContext } from './use_execution_context';
2021

@@ -30,17 +31,18 @@ export const Route = <T extends {}>({
3031
render,
3132
...rest
3233
}: RouteProps<string, { [K: string]: string } & T>) => {
34+
const { enableExecutionContextTracking } = useSharedUXRoutesContext();
3335
const component = useMemo(() => {
3436
if (!Component) {
3537
return undefined;
3638
}
3739
return (props: RouteComponentProps) => (
3840
<>
39-
<MatchPropagator />
41+
{enableExecutionContextTracking && <MatchPropagator />}
4042
<Component {...props} />
4143
</>
4244
);
43-
}, [Component]);
45+
}, [Component, enableExecutionContextTracking]);
4446

4547
if (component) {
4648
return <ReactRouterRoute {...rest} component={component} />;
@@ -52,7 +54,7 @@ export const Route = <T extends {}>({
5254
{...rest}
5355
render={(props) => (
5456
<>
55-
<MatchPropagator />
57+
{enableExecutionContextTracking && <MatchPropagator />}
5658
{/* @ts-ignore else condition exists if renderFunction is undefined*/}
5759
{renderFunction(props)}
5860
</>
@@ -62,7 +64,7 @@ export const Route = <T extends {}>({
6264
}
6365
return (
6466
<ReactRouterRoute {...rest}>
65-
<MatchPropagator />
67+
{enableExecutionContextTracking && <MatchPropagator />}
6668
{children}
6769
</ReactRouterRoute>
6870
);
@@ -75,6 +77,12 @@ export const MatchPropagator = () => {
7577
const { executionContext } = useKibanaSharedUX().services;
7678
const match = useRouteMatch();
7779

80+
if (!executionContext && process.env.NODE_ENV !== 'production') {
81+
throw new Error(
82+
'Default execution context tracking is enabled but the executionContext service is not available'
83+
);
84+
}
85+
7886
useSharedUXExecutionContext(executionContext, {
7987
type: 'application',
8088
page: match.path,

0 commit comments

Comments
 (0)