-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Logs UI] Replace deprecated getInjectedVar with NP spaces API #74280
[Logs UI] Replace deprecated getInjectedVar with NP spaces API #74280
Conversation
|
||
export type PluginKibanaContextValue = CoreStart & InfraClientStartDeps; | ||
|
||
export const createKibanaContextForPlugin = (core: CoreStart, pluginsStart: InfraClientStartDeps) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ℹ️ This in combination with the useKibanaContextForPlugin()
hook below aims at providing a typed Kibana context value as recommended by the kibana_react
plugin.
import { DataPublicPluginStart } from '../../../../src/plugins/data/public'; | ||
import { HomePublicPluginSetup } from '../../../../src/plugins/home/public'; | ||
import { | ||
import type { CoreSetup, CoreStart, Plugin as PluginClass } from 'kibana/public'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ℹ️ These explicit type imports just try to make sure we don't accidentally pull runtime code into the eagerly loaded entry point bundle.
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, works as expected 🎉
|
||
// This is a rather crude way of guarding the dependent providers against | ||
// arguments that are only made available asynchronously. Ideally, we'd use | ||
// React concurrent mode and Suspense in oder to handle that more gracefully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed 👍 (Nit: there's a small typo in the comment with oder
)
@elasticmachine merge upstream |
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
async chunks size
History
To update your PR or re-run it, just comment with: |
… (#74517) * Replace getInjectedVar() with NP spaces API * Fix typo in comment * Fix typo in comment Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Summary
This replaces the use of
getInjectedVar()
in order to access the active space's id withgetActiveSpace()
from thespaces
plugin. It introduces some crude guard against the new asynchronous availability of the id which differs from the previous, synchronous availability.closes #68142