Skip to content
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.

Sign up for GitHub

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

[SIEM] Add react/display-name eslint rule #53107

Merged
merged 7 commits into from
Jan 7, 2020
Merged

[SIEM] Add react/display-name eslint rule #53107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2f04edc
[SIEM] Add react/display-name eslint rule
patrykkopycinski Dec 16, 2019
4af9fd6
Merge branch 'master' of github.com:elastic/kibana into chore/siem-es…
patrykkopycinski Jan 4, 2020
bc1f53d
cleanup
patrykkopycinski Jan 4, 2020
1385da5
cleanup
patrykkopycinski Jan 4, 2020
6321638
Merge branch 'master' into chore/siem-eslint-display-name
elasticmachine Jan 6, 2020
a295ce0
Merge branch 'master' of github.com:elastic/kibana into chore/siem-es…
patrykkopycinski Jan 7, 2020
c2d48f2
fix properly displayNames
patrykkopycinski Jan 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@ module.exports = {
// 'prefer-template': 'warn',
'react/boolean-prop-naming': 'error',
'react/button-has-type': 'error',
'react/display-name': 'error',
'react/forbid-dom-props': 'error',
'react/no-access-state-in-setstate': 'error',
// This style will be turned on after most bugs are fixed
Expand Down
103 changes: 64 additions & 39 deletions x-pack/legacy/plugins/siem/public/apps/start_app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { createHashHistory } from 'history';
import React, { memo, FC } from 'react';
import { createHashHistory, History } from 'history';
import React, { memo, useMemo, FC } from 'react';
import { ApolloProvider } from 'react-apollo';
import { Store } from 'redux';
import { Provider as ReduxStoreProvider } from 'react-redux';
import { ThemeProvider } from 'styled-components';

Expand All @@ -23,7 +24,7 @@ import { Storage } from '../../../../../../src/plugins/kibana_utils/public';
import { DEFAULT_DARK_MODE } from '../../common/constants';
import { ErrorToastDispatcher } from '../components/error_toast_dispatcher';
import { compose } from '../lib/compose/kibana_compose';
import { AppFrontendLibs } from '../lib/lib';
import { AppFrontendLibs, AppApolloClient } from '../lib/lib';
import { StartCore, StartPlugins } from './plugin';
import { PageRouter } from '../routes';
import { createStore } from '../store';
Expand All @@ -32,48 +33,70 @@ import { MlCapabilitiesProvider } from '../components/ml/permissions/ml_capabili

import { ApolloClientContext } from '../utils/apollo_context';

const StartApp: FC<AppFrontendLibs> = memo(libs => {
const history = createHashHistory();
interface AppPluginRootComponentProps {
apolloClient: AppApolloClient;
history: History;
store: Store;
theme: any; // eslint-disable-line @typescript-eslint/no-explicit-any
}

const libs$ = new BehaviorSubject(libs);
const AppPluginRootComponent: React.FC<AppPluginRootComponentProps> = ({
theme,
store,
apolloClient,
history,
}) => (
<EuiErrorBoundary>
<I18nContext>
<ManageGlobalToaster>
<ReduxStoreProvider store={store}>
<ApolloProvider client={apolloClient}>
<ApolloClientContext.Provider value={apolloClient}>
<ThemeProvider theme={theme}>
<MlCapabilitiesProvider>
<PageRouter history={history} />
</MlCapabilitiesProvider>
</ThemeProvider>
<ErrorToastDispatcher />
<GlobalToaster />
</ApolloClientContext.Provider>
</ApolloProvider>
</ReduxStoreProvider>
</ManageGlobalToaster>
</I18nContext>
</EuiErrorBoundary>
);

const AppPluginRoot = memo(AppPluginRootComponent);

const StartAppComponent: FC<AppFrontendLibs> = libs => {
const history = createHashHistory();
const libs$ = new BehaviorSubject(libs);
const store = createStore(undefined, libs$.pipe(pluck('apolloClient')));
const [darkMode] = useUiSetting$<boolean>(DEFAULT_DARK_MODE);
const theme = useMemo(
() => ({
eui: darkMode ? euiDarkVars : euiLightVars,
darkMode,
}),
[darkMode]
);

return (
<AppPluginRoot store={store} apolloClient={libs.apolloClient} history={history} theme={theme} />
);
};

const AppPluginRoot = memo(() => {
const [darkMode] = useUiSetting$<boolean>(DEFAULT_DARK_MODE);
return (
<EuiErrorBoundary>
<I18nContext>
<ManageGlobalToaster>
<ReduxStoreProvider store={store}>
<ApolloProvider client={libs.apolloClient}>
<ApolloClientContext.Provider value={libs.apolloClient}>
<ThemeProvider
theme={() => ({
eui: darkMode ? euiDarkVars : euiLightVars,
darkMode,
})}
>
<MlCapabilitiesProvider>
<PageRouter history={history} />
</MlCapabilitiesProvider>
</ThemeProvider>
<ErrorToastDispatcher />
<GlobalToaster />
</ApolloClientContext.Provider>
</ApolloProvider>
</ReduxStoreProvider>
</ManageGlobalToaster>
</I18nContext>
</EuiErrorBoundary>
);
});
return <AppPluginRoot />;
});
const StartApp = memo(StartAppComponent);

export const ROOT_ELEMENT_ID = 'react-siem-root';

export const SiemApp = memo<{ core: StartCore; plugins: StartPlugins }>(({ core, plugins }) => (
interface SiemAppComponentProps {
core: StartCore;
plugins: StartPlugins;
}

const SiemAppComponent: React.FC<SiemAppComponentProps> = ({ core, plugins }) => (
<KibanaContextProvider
services={{
appName: 'siem',
Expand All @@ -84,4 +107,6 @@ export const SiemApp = memo<{ core: StartCore; plugins: StartPlugins }>(({ core,
>
<StartApp {...compose()} />
</KibanaContextProvider>
));
);

export const SiemApp = memo(SiemAppComponent);
58 changes: 28 additions & 30 deletions x-pack/legacy/plugins/siem/public/components/alerts_viewer/alerts_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,33 +51,31 @@ const defaultAlertsFilters: esFilters.Filter[] = [
},
];

export const AlertsTable = React.memo(
({
endDate,
startDate,
pageFilters = [],
}: {
endDate: number;
startDate: number;
pageFilters?: esFilters.Filter[];
}) => {
const alertsFilter = useMemo(() => [...defaultAlertsFilters, ...pageFilters], [pageFilters]);
return (
<StatefulEventsViewer
pageFilters={alertsFilter}
defaultModel={alertsDefaultModel}
end={endDate}
id={ALERTS_TABLE_ID}
start={startDate}
timelineTypeContext={useMemo(
() => ({
documentType: i18n.ALERTS_DOCUMENT_TYPE,
footerText: i18n.TOTAL_COUNT_OF_ALERTS,
title: i18n.ALERTS_TABLE_TITLE,
}),
[]
)}
/>
);
}
);
interface Props {
endDate: number;
startDate: number;
pageFilters?: esFilters.Filter[];
}

const AlertsTableComponent: React.FC<Props> = ({ endDate, startDate, pageFilters = [] }) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the intent of creating this intermediary AlertsTableComponent using React.FC and then wrapping as AlertsTable using React.memo vs the previous implementation of just using React.memo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React.memo propagates name of the wrapped component, but as we were passing an inline function to the memo we end up with Anonymus (Memo)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, perfect, that's what I was wondering! 🙂

For anyone passing through, @patrykkopycinski outlines this behavior in https://github.com/elastic/siem-team/issues/485.

const alertsFilter = useMemo(() => [...defaultAlertsFilters, ...pageFilters], [pageFilters]);
return (
<StatefulEventsViewer
pageFilters={alertsFilter}
defaultModel={alertsDefaultModel}
end={endDate}
id={ALERTS_TABLE_ID}
start={startDate}
timelineTypeContext={useMemo(
() => ({
documentType: i18n.ALERTS_DOCUMENT_TYPE,
footerText: i18n.TOTAL_COUNT_OF_ALERTS,
title: i18n.ALERTS_TABLE_TITLE,
}),
[]
)}
/>
);
};

export const AlertsTable = React.memo(AlertsTableComponent);
2 changes: 2 additions & 0 deletions x-pack/legacy/plugins/siem/public/components/event_details/columns.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* you may not use this file except in compliance with the Elastic License.
*/

/* eslint-disable react/display-name */

import {
EuiCheckbox,
EuiFlexGroup,
Expand Down
2 changes: 2 additions & 0 deletions x-pack/legacy/plugins/siem/public/components/fields_browser/category_columns.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* you may not use this file except in compliance with the Elastic License.
*/

/* eslint-disable react/display-name */

import {
EuiIcon,
EuiFlexGroup,
Expand Down
Loading