-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Solution][Sourcerer] Replace Sourcerer with Discover Data View Picker #210585
base: main
Are you sure you want to change the base?
Changes from 80 commits
6a1d3e3
f26acdf
cc44f6c
d0c1970
99f434a
85d7233
73db16d
eadb7e6
a0b30c7
73ee667
df8d885
6cf58d6
c39eb3b
7020ef4
3d47008
83667ba
28aa4f0
f92eab5
0805163
aeeaf71
f49c643
1f4aebf
63054f5
c7e0665
f261ab8
8c3e9a0
23439aa
770993c
fa80256
2baf50b
40b395c
c26acc7
1de086e
b088e6b
c85dec4
09daf62
829f255
639fee5
8e1dee0
10c8e8e
1b8774f
d58905c
0fc0db5
77be180
19a8011
83b194a
3b25db5
815ba6e
ac38c51
0e1e7a0
d3814a0
0506aac
ed274ee
1ec5817
d503cb6
3e0c647
90c5561
0f95d1e
4540f96
abf09aa
b3ecd5b
61e0787
787a836
871e2a3
eaabb1a
ce3e2b6
90fd3ae
128bfb9
517b20b
79868ca
742c340
1e82688
dd84f99
bec10ae
ddf2573
5f68a90
baecafd
38df1eb
119d6a2
6ff0437
d31acdc
5ac74b7
bac953a
7b9b2df
c96766b
f5beeca
a6381fd
b804725
23f23d6
b904a03
8eb497e
33406cf
082fc42
069393d
ded2d6e
1cf669e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,11 @@ import React from 'react'; | |
import type { ComponentType } from 'react'; | ||
import type { ReactElement } from 'react-markdown'; | ||
import type { DataView } from '@kbn/data-views-plugin/common'; | ||
import { DataViewPickerScopeName } from '../../../data_view_picker/constants'; | ||
import { useFullDataView } from '../../../data_view_picker/hooks/use_full_data_view'; | ||
import { DataViewErrorComponent } from './data_view_error'; | ||
import { useEnableExperimental } from '../../hooks/use_experimental_features'; | ||
|
||
import { useGetScopedSourcererDataView } from '../../../sourcerer/components/use_get_sourcerer_data_view'; | ||
import { SourcererScopeName } from '../../../sourcerer/store/model'; | ||
|
||
|
@@ -30,10 +34,19 @@ export const withDataView = <P extends WithDataViewArg>( | |
fallback?: ReactElement | ||
) => { | ||
const ComponentWithDataView = (props: OmitDataView<P>) => { | ||
const dataView = useGetScopedSourcererDataView({ | ||
const experimentalDataView = useFullDataView({ | ||
dataViewPickerScope: DataViewPickerScopeName.timeline, | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It does look, that experimentalDataView is getting initiated every time, in disregard of feature flag. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree, avoiding use of let would achieve the same thing, and is imo a good rule of thumb to follow |
||
|
||
let dataView = useGetScopedSourcererDataView({ | ||
sourcererScope: SourcererScopeName.timeline, | ||
}); | ||
|
||
const { newDataViewPickerEnabled } = useEnableExperimental(); | ||
if (newDataViewPickerEnabled) { | ||
dataView = experimentalDataView; | ||
} | ||
|
||
if (!dataView) { | ||
return fallback ?? <DataViewErrorComponent />; | ||
} | ||
|
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.
Nothing wrong with this pattern, but do you think using something like context would be better? Just a quick note, but still working through the code
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.
HOCs in general are much harder to reason about imo compared to hooks, and introduce unique to this pattern issues. ComponentWithDataView can and should be memoized, we should also make it a pattern to preserve the displayName of the passed in component, the amount of unknowns popping up in the tree is beginning to be a little confusing. Full suggestion: