Skip to content

Commit

Permalink
[context] Unify Contexts, deprecate others (#161914)
Browse files Browse the repository at this point in the history
> Pre-req for #56406

## Summary

We've had a long-standing problem in Kibana around our use of React
context, particularly with EUI and i18n. There hasn't existed an
idempotent context structure, and that has lead to a lot of unexpected
results, (e.g. missing translations, inconsistent dark mode, excess
context providers, etc).

The biggest change coming from this PR is knowing exactly which provider
to use in a particular use case. This means, for example,
`ReactDOM.render` calls won't be missing `i18n` or `theme` due to a
missing context. It also allows consumers to use `darkMode` without
having to read the `uiSetting` themselves, instead allowing the context
to do it for them.

We also haven't been honoring the intended [`EuiProvider`
API](https://eui.elastic.co/#/utilities/provider#theming-and-global-styles)...
in some cases we've been creating and re-creating the Emotion caches,
often by copy/paste of the cache code. We've also been nesting
`EuiThemeProvider` contexts unnecessarily-- thinking we need to render a
theme provider in an isolated component-- which renders an additional
`span` element into the DOM.

This PR attempts to address this inconsistency by creating a set of
context providers divided by use case:


![diagram](https://github.com/elastic/kibana/assets/297604/e01c6296-1b7a-4639-ae96-946866950efe)

### `KibanaRootContextProvider`
A root context provider for Kibana. This is the top level context
provider that wraps the entire application. It is responsible for
initializing all of the other contexts and providing them to the
application. It's provided as a package for specific use cases, (e.g.
the `RenderingService`, cases where we replace the entire page content,
Storybook, testing, etc), but not intended for plugins.

### `KibanaRenderContextProvider`
A render context provider for Kibana. This context is designed to be
used with ad-hoc renders of React components, (usually with
`ReactDOM.render`).

### `KibanaThemeContextProvider`
A theme context provider for Kibana. A corollary to EUI's
`EuiThemeProvider`, it uses Kibana services to ensure the EUI Theme is
customized correctly.

### (deprecated) `KibanaStyledComponentsThemeProvider`
A styled components theme provider for Kibana. This package is supplied
for compatibility with legacy code, but should not be used in new code.

## Deprecation strategy
This PR does *not* change any use of context by consumers. It maps the
existing contexts in `kibanaReact` to the new contexts, (along with the
loose API). This means that we won't have completely fixed all of our
dark mode issues yet. But this is necessary to keep this PR focused on
the change, rather than drawing in a lot of teams to review individual
uses.

We should, however, see an immediate performance improvement in the UI
from the reduction in `EuiProvider` calls.

## Open questions
- [ ] Does it make sense to expose a `useTheme` hook from
`@kbn/react-kibana-context-theme` to replace `useEuiTheme`?

## Next steps
- [ ] Update deprecated uses to new contexts.
- [ ] Audit and update calls to `ReactDOM.render`.
- [ ] Add ESLint rule to warn for use of EUI contexts.
- [ ] Delete code from `kibanaReact`.
  • Loading branch information
clintandrewhall authored Jul 28, 2023
1 parent 0ecc28b commit 477505a
Show file tree
Hide file tree
Showing 112 changed files with 2,095 additions and 1,322 deletions.
9 changes: 9 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,12 @@ src/plugins/presentation_util @elastic/kibana-presentation
x-pack/plugins/profiling @elastic/profiling-ui
x-pack/packages/kbn-random-sampling @elastic/kibana-visualizations
packages/kbn-react-field @elastic/kibana-data-discovery
packages/react/kibana_context/common @elastic/appex-sharedux
packages/react/kibana_context/render @elastic/appex-sharedux
packages/react/kibana_context/root @elastic/appex-sharedux
packages/react/kibana_context/styled @elastic/appex-sharedux
packages/react/kibana_context/theme @elastic/appex-sharedux
packages/react/kibana_mount @elastic/appex-sharedux
x-pack/plugins/remote_clusters @elastic/platform-deployment-management
test/plugin_functional/plugins/rendering_plugin @elastic/kibana-core
packages/kbn-repo-file-maps @elastic/kibana-operations
Expand Down Expand Up @@ -1299,6 +1305,9 @@ x-pack/plugins/translations/translations
# Profiling api integration testing
x-pack/test/profiling_api_integration @elastic/profiling-ui

# Shared UX
packages/react @elastic/appex-sharedux

####
## These rules are always last so they take ultimate priority over everything else
####
1 change: 1 addition & 0 deletions .i18nrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
"newsfeed": "src/plugins/newsfeed",
"presentationUtil": "src/plugins/presentation_util",
"randomSampling": "x-pack/packages/kbn-random-sampling",
"reactPackages": "packages/react",
"textBasedEditor": "packages/kbn-text-based-editor",
"reporting": "packages/kbn-reporting/common",
"savedObjects": "src/plugins/saved_objects",
Expand Down
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,12 @@
"@kbn/profiling-plugin": "link:x-pack/plugins/profiling",
"@kbn/random-sampling": "link:x-pack/packages/kbn-random-sampling",
"@kbn/react-field": "link:packages/kbn-react-field",
"@kbn/react-kibana-context-common": "link:packages/react/kibana_context/common",
"@kbn/react-kibana-context-render": "link:packages/react/kibana_context/render",
"@kbn/react-kibana-context-root": "link:packages/react/kibana_context/root",
"@kbn/react-kibana-context-styled": "link:packages/react/kibana_context/styled",
"@kbn/react-kibana-context-theme": "link:packages/react/kibana_context/theme",
"@kbn/react-kibana-mount": "link:packages/react/kibana_mount",
"@kbn/remote-clusters-plugin": "link:x-pack/plugins/remote_clusters",
"@kbn/rendering-plugin": "link:test/plugin_functional/plugins/rendering_plugin",
"@kbn/repo-info": "link:packages/kbn-repo-info",
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type { InternalInjectedMetadataSetup } from '@kbn/core-injected-metadata-
import type { ThemeServiceSetup } from '@kbn/core-theme-browser';
import type { I18nStart } from '@kbn/core-i18n-browser';
import type { FatalErrorInfo, FatalErrorsSetup } from '@kbn/core-fatal-errors-browser';
import { CoreContextProvider } from '@kbn/core-theme-browser-internal';
import { KibanaRootContextProvider } from '@kbn/react-kibana-context-root';
import { FatalErrorsScreen } from './fatal_errors_screen';
import { getErrorInfo } from './get_error_info';

Expand Down Expand Up @@ -95,13 +95,13 @@ export class FatalErrorsService {
this.rootDomElement.appendChild(container);

render(
<CoreContextProvider i18n={i18n} theme={theme} globalStyles={true}>
<KibanaRootContextProvider i18n={i18n} theme={theme} globalStyles={true}>
<FatalErrorsScreen
buildNumber={injectedMetadata.getKibanaBuildNumber()}
kibanaVersion={injectedMetadata.getKibanaVersion()}
errorInfo$={this.errorInfo$}
/>
</CoreContextProvider>,
</KibanaRootContextProvider>,
container
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
"kbn_references": [
"@kbn/core-injected-metadata-browser-internal",
"@kbn/core-theme-browser",
"@kbn/core-theme-browser-internal",
"@kbn/core-i18n-browser",
"@kbn/core-fatal-errors-browser",
"@kbn/i18n-react",
"@kbn/core-injected-metadata-browser-mocks",
"@kbn/core-theme-browser-mocks",
"@kbn/test-subj-selector",
"@kbn/test-jest-helpers",
"@kbn/react-kibana-context-root",
],
"exclude": [
"target/**/*",
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { FormattedMessage } from '@kbn/i18n-react';
import type { I18nStart } from '@kbn/core-i18n-browser';
import type { OverlayStart } from '@kbn/core-overlays-browser';
import { ThemeServiceStart } from '@kbn/core-theme-browser';
import { CoreContextProvider } from '@kbn/core-theme-browser-internal';
import { KibanaRenderContextProvider } from '@kbn/react-kibana-context-render';

interface ErrorToastProps {
title: string;
Expand Down Expand Up @@ -71,7 +71,7 @@ export function showErrorDialog({

const modal = openModal(
mount(
<CoreContextProvider i18n={i18n} theme={theme}>
<KibanaRenderContextProvider i18n={i18n} theme={theme}>
<EuiModalHeader>
<EuiModalHeaderTitle>{title}</EuiModalHeaderTitle>
</EuiModalHeader>
Expand All @@ -94,7 +94,7 @@ export function showErrorDialog({
/>
</EuiButton>
</EuiModalFooter>
</CoreContextProvider>
</KibanaRenderContextProvider>
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import { render, unmountComponentAtNode } from 'react-dom';

import type { ThemeServiceStart } from '@kbn/core-theme-browser';
import type { I18nStart } from '@kbn/core-i18n-browser';
import { CoreContextProvider } from '@kbn/core-theme-browser-internal';
import type { IUiSettingsClient } from '@kbn/core-ui-settings-browser';
import type { OverlayStart } from '@kbn/core-overlays-browser';
import { KibanaRenderContextProvider } from '@kbn/react-kibana-context-render';
import { GlobalToastList } from './global_toast_list';
import { ToastsApi } from './toasts_api';

Expand Down Expand Up @@ -42,12 +42,12 @@ export class ToastsService {
this.targetDomElement = targetDomElement;

render(
<CoreContextProvider i18n={i18n} theme={theme}>
<KibanaRenderContextProvider i18n={i18n} theme={theme}>
<GlobalToastList
dismissToast={(toastId: string) => this.api!.remove(toastId)}
toasts$={this.api!.get$()}
/>
</CoreContextProvider>,
</KibanaRenderContextProvider>,
targetDomElement
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"@kbn/i18n",
"@kbn/utility-types",
"@kbn/core-theme-browser",
"@kbn/core-theme-browser-internal",
"@kbn/core-i18n-browser",
"@kbn/core-ui-settings-browser",
"@kbn/core-overlays-browser",
Expand All @@ -29,6 +28,7 @@
"@kbn/core-overlays-browser-mocks",
"@kbn/core-theme-browser-mocks",
"@kbn/core-mount-utils-browser",
"@kbn/react-kibana-context-render",
],
"exclude": [
"target/**/*",
Expand Down
Loading

0 comments on commit 477505a

Please sign in to comment.