[SIEM] Move public to new platform #48840
Conversation
|
Pinging @elastic/siem (Team:SIEM) |
XavierM
added
the
release_note:skip
This comment has been minimized.
This comment has been minimized.
XavierM
force-pushed
the
siem-public-new-platform
branch
from
32772e9 to
42ca93f
Compare
| signal: AbortSignal | ||
| ): Promise<Anomalies> => { | ||
| const [kbnVersion] = useKibanaUiSetting(DEFAULT_KBN_VERSION); |
There was a problem hiding this comment.
The first rule of Hooks Club is Don’t call Hooks from regular JavaScript functions...
We've got eslint-plugin-react-hooks -- any idea why wasn't this caught?
There was a problem hiding this comment.
maybe because useKibanaUISetting is not ordinary hook
There was a problem hiding this comment.
oh I know why it was working before because we were importing the npStart to get the uiSettings but now we are using the useKibanaCore and useKibanaPlugins hooks so that's why this error came up. So before it was not a real hook, it was working like a get for a config file.
| @@ -189,7 +178,6 @@ export const stopDatafeeds = async ({ | |||
| 'kbn-system-api': 'true', | |||
| 'content-type': 'application/json', | |||
| 'kbn-xsrf': kbnVersion, | |||
There was a problem hiding this comment.
No need to add now (as it's been missed before), but looks like these two API calls are the only ones missing the 'kbn-version': kbnVersion header. We're fine with just `'kbn-xsrf', but should probably be there for consistency with the others. Can refactor remainder in the next NP PR when this all changes again anyway.
There was a problem hiding this comment.
Maybe we should just create a generic function to build these headers everywhere.
💚 Build Succeeded |
💚 Build Succeeded |
* shim public new platform * fix import * fix ml api with kbn version * review I * fix bad coding with the hooks
* shim public new platform * fix import * fix ml api with kbn version * review I * fix bad coding with the hooks
There was a problem hiding this comment.
Searching in plugins/siem there's still 75 files with ui/* imports. Most of these are ui/chrome which you can replace with the NP API's: https://github.com/elastic/kibana/blob/master/src/core/MIGRATION_EXAMPLES.md#L354
| this.chrome = chrome; | ||
| } | ||
|
|
||
| public start(start: StartObject): void { |
There was a problem hiding this comment.
NP will inject two parameters here instead of an object: public start(coreStart: LegacyCoreStart, plugins: PluginsStart)
| @@ -31,8 +31,9 @@ type GenericValue = string | boolean | number; | |||
| * because the underlying `UiSettingsClient` doesn't support that. | |||
| */ | |||
| export const useKibanaUiSetting = (key: string, defaultValue?: GenericValue) => { | |||
There was a problem hiding this comment.
Not necessary for NP migration, but you could adopt some of the utilities from kibana_react instead of maintaining your own https://github.com/elastic/kibana/blob/master/src/plugins/kibana_react/README.md
|
you can also remove the dependency on 'ui/saved_objects/components/saved_object_finder' by importing src/plugins/kibana_react/public/saved_objects/saved_object_finder.tsx |
Summary
We moved SIEM public into the New platform shim
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
[ ] This was checked for breaking API changes and was labeled appropriately[ ] This includes a feature addition or change that requires a release note and was labeled appropriately