mirror of
https://github.com/elastic/kibana.git
synced 2025-04-22 17:04:01 -04:00
584875e666
## Summary Epic: https://github.com/elastic/kibana-team/issues/1435 Closes https://github.com/elastic/kibana/issues/205413 Closes https://github.com/elastic/kibana/issues/205411 This PR creates a new way to expose stateful service dependencies needed for rendering React elements in Kibana. The concept of the changes is that `KibanaRenderContextProvider` should not be a shared module, but should be wrapped by a core service (the `RenderContextService` name is TBD). The next steps in this direction would be to coordinate teams to migrate away from directly using `KibanaRenderContextProvider`. ### Background Today, the dependencies for `KibanaRenderContextProvider` are declared as separate services which can be found in the `CoreStart` context. This has created a situation where enhancing the module with more dependencies creates widespread changes needed for the UI codebase. The @elastic/appex-sharedux team is looking to solve this situation by defining a less impactful way to make future enhancements. The solution is one that can be gradually migrated towards, so the SharedUX team can ask Kibana contributors to migrate towards in their own code. This PR offers a POC for that solution. ### Details of this POC The driving goal for this refactor is to lessen the impact across the Kibana codebase whenever the `KibanaRenderContext` module needs to require additional services from the `CoreStart` context. #### Rendering a React Element with `ReactDOM.render`: Before ```tsx const renderApp = ({ core, targetDomElement }: { core: CoreStart; targetDomElement: HTMLElement; }) => { // If `KibanaRenderContextProvider` needs to expand its scope, more services could be needed here, // updating all the places throughout the code to pass those is a ton of work 👎🏻 const { i18n, theme, analytics, userProfile, executionContext } = core; ReactDOM.render( <KibanaRenderContextProvider {...{ i18n, theme, analytics, userProfile, executionContext }}> <MyApplication /> </KibanaRenderContextProvider>, targetDomElement ); return () => ReactDOM.unmountComponentAtNode(targetDomElement); }; ``` #### Rendering a React Element with `ReactDOM.render`: After ```tsx const renderApp = ({ core, targetDomElement }: { core: CoreStart; targetDomElement: HTMLElement; }) => { // So much less code, so much more future-proof 👍🏻 ReactDOM.render(core.rendering.addContext(<MyApplication />), targetDomElement); return () => ReactDOM.unmountComponentAtNode(targetDomElement); }; ``` ### Alternatives considered See https://github.com/elastic/kibana/pull/209161 ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### FAQ 1. **Q**: This is React-centric. Does this give Kibana more commitment towards React? **A:** For now, yes. But if we want to Kibana to remain framework-agnostic we may be able to add more extensions to the RenderContextService that support other frameworks. 1. **Q:** Why not have a service that wraps `ReactDOM.render`? **A:** As we steer towards upgrading to React 18 in Kibana, staying agnostic of how React is rendered benefits us. React 18 has different ergonomics based on whether you want to update an existing tree or mount a new one. 1. **Q:** Does the API have to be named `rendering.addContext`? **A:** No, it does not. Please suggest a better name if you have one in mind. 1. **Q:** What are the next steps? **A:** Refer to the [Epic](https://github.com/elastic/kibana-team/issues/1435). This PR started as a POC but became ready for merge. After it is delivered to the codebase, the next steps are to improve documentation and engage in "sheparding." That is, socialize the new way of injecting dependencies into the context, support teams in their migration, and track the progress of migration. ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [x] Care is needed to ensure this doesn't not negatively impact performance with unnecessary re-renders. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> |
||
|---|---|---|
| .. | ||
| cli | ||
| cli_encryption_keys | ||
| cli_health_gateway | ||
| cli_keystore | ||
| cli_plugin | ||
| cli_setup | ||
| cli_verification_code | ||
| core | ||
| dev | ||
| platform | ||
| setup_node_env | ||