[APM] Some miscellaneous client new platform updates #51482
Conversation
smith
added
release_note:skip
💚 Build Succeeded
|
| </LocationProvider> | ||
| </Router> | ||
| </i18nCore.Context> | ||
| </PluginsContext.Provider> | ||
| </KibanaCoreContextProvider>, | ||
| document.getElementById(REACT_APP_ROOT_ID) |
There was a problem hiding this comment.
document.getElementById(REACT_APP_ROOT_ID) should be passed in a param domElement to align a bit better with the NP plugin pattern. This element reference is already in checkForRoot, so it can be passed as a result of that promise.
There was a problem hiding this comment.
I'm going to skip doing this for now, until we implement the mount method when we register the NP application.
| @@ -57,5 +31,5 @@ const checkForRoot = () => { | |||
| }); | |||
| }; | |||
| checkForRoot().then(() => { | |||
| plugin().start(core); | |||
| plugin().start(core, plugins); | |||
| }); | |||
There was a problem hiding this comment.
is it possible to use async/await here?
There was a problem hiding this comment.
You can only await inside of an async function, so if we wanted to do that here we would need to wrap everything inside an immediately executing async function, which would probably be just as complex as what we're doing already.
💔 Build Failed
|
| @@ -6,16 +6,16 @@ | |||
|
|
|||
| import { i18n } from '@kbn/i18n'; | |||
| import { useEffect } from 'react'; | |||
| import { capabilities } from 'ui/capabilities'; | |||
| import { useKibanaCore } from '../../../../../observability/public'; | |||
|
|
|||
| export const useUpdateBadgeEffect = () => { | |||
There was a problem hiding this comment.
Instead of doing this in a hook I think it would make more sense to update the badge from the plugin's start method outside React like you did for core.chrome.setHelpExtension (although still in separate file):
export class Plugin {
start(core: LegacyCoreStart){
setBadge(core);
ReactDOM.render(...);
}
}| @@ -74,6 +74,8 @@ export function KueryBar() { | |||
| }); | |||
| const { urlParams } = useUrlParams(); | |||
| const location = useLocation(); | |||
| const { data } = usePlugins(); | |||
| const autocompleteProvider = data.autocomplete.getProvider('kuery'); | |||
There was a problem hiding this comment.
Thanks for moving the unnecessary abstraction 👍
| }) | ||
| } | ||
| ] | ||
| }); |
There was a problem hiding this comment.
Perhaps move this to a separate file similar to chrome.setBadge
* Move `setHelpExtension` to plugin start method instead of plugin root * Move `setHelpExtension` to a separate file * Remove 'ui/modules' import * Use new platform capabilities in useUpdateBadgeEffect * Move useUpdateBadgeEffect to a utility function called in start * Add plugins and plugins context to new platform start * Use new platform plugins for KueryBar autocomplete provider * Add types for plugin and rename to ApmPublicPlugin * Add empty setup method to plugin * Move all context providers from App to render method * Remove some unnecessary mocks References elastic#32894.
💚 Build Succeeded |
* Move `setHelpExtension` to plugin start method instead of plugin root * Move `setHelpExtension` to a separate file * Remove 'ui/modules' import * Use new platform capabilities in useUpdateBadgeEffect * Move useUpdateBadgeEffect to a utility function called in start * Add plugins and plugins context to new platform start * Use new platform plugins for KueryBar autocomplete provider * Add types for plugin and rename to ApmPublicPlugin * Add empty setup method to plugin * Move all context providers from App to render method * Remove some unnecessary mocks References elastic#32894.
setHelpExtensionto plugin start method instead of plugin rootsetHelpExtensionto a separate fileReferences #32894.