[CTI] ensures isThreatIntelModuleEnabled query for Overview page is made only once #104523
Conversation
ecezalp
added
release_note:skip
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
| @@ -18,7 +18,7 @@ export const useCtiEventCounts = ({ | |||
| from, | |||
| setQuery, | |||
| to, | |||
| }: ThreatIntelLinkPanelProps) => { | |||
| }: Omit<ThreatIntelLinkPanelProps, 'isThreatIntelModuleEnabled'>) => { | |||
There was a problem hiding this comment.
We might be trying too hard here to reuse these types; if this hook and the component that share(d) props have independent use cases, they should each declare their own props.
There was a problem hiding this comment.
declared a new type for CtiEnabledModuleProps, which is the same Omit<ThreatIntelLinkPanelProps, 'isThreatIntelModuleEnabled'> it's used in 2 places and a bit more readable IMHO, let me know what you think
| export const ThreatIntelLinkPanel = React.memo(ThreatIntelLinkPanelComponent); | ||
| export const ThreatIntelLinkPanel = React.memo( | ||
| ThreatIntelLinkPanelComponent, | ||
| (prevProps, nextProps) => |
There was a problem hiding this comment.
Is this function implicitly excluding some prop from the comparison? If it is, a comment to that effect would be good for posterity. If it's not, then we can probably do away with this function.
| @@ -23,7 +23,7 @@ const warning = ( | |||
|
|
|||
| export const CtiNoEventsComponent = ({ to, from }: { to: string; from: string }) => { | |||
| const { buttonHref, listItems, isDashboardPluginDisabled } = useCtiDashboardLinks( | |||
| { ...emptyEventCountsByDataset }, | |||
| emptyEventCountsByDataset, | |||
There was a problem hiding this comment.
Was this the cause of many rerenders, or just an unnecessary spread you also noticed? Hard to tell from the PR.
There was a problem hiding this comment.
I have a hard time reproducing so I wasn't sure if this one was the cause or the other one (non-empty eventCountsByDataset) but I think it's a good idea to ensure that the first arg of useCtiDashboardLinks is identical in this case, and compared via lodash in the case of cti_with_events.tsx
|
latest changes:
|
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @ecezalp |
Summary
The initial query to check whether the Threat Intel module is enabled was being made multiple times (on the Overview page) as the CTI Dashboard Links component rerendered. This is an undesired behavior considering the answer would never change during those rerenders, and the query should be made only once.