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.

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
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
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
2 changes: 2 additions & 0 deletions x-pack/legacy/plugins/siem/public/apps/start_app.tsx
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 */
Copy link
Member

Choose a reason for hiding this comment

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

May want to add a comment here as to why this is being disabled -- when I added displayName's for the below three components (AppPluginRoot, StartApp, and SiemApp) they were not being shown in the react dev tools Component tree, so I assume this is the reason?


import { createHashHistory } from 'history';
import React, { memo, FC } from 'react';
import { ApolloProvider } from 'react-apollo';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,33 +51,35 @@ 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,
}),
[]
)}
/>
);
};

AlertsTableComponent.displayName = 'AlertsTableComponent';

export const AlertsTable = React.memo(AlertsTableComponent);

AlertsTable.displayName = 'AlertsTable';
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,5 @@ export const FieldsBrowser = React.memo<Props>(
);
}
);

FieldsBrowser.displayName = 'FieldsBrowser';
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, EuiIcon, EuiToolTip, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { uniqBy } from 'lodash/fp';
import * as React from 'react';
Expand Down
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 { EuiLink, EuiTableRow, EuiTableRowCell, EuiText, EuiToolTip } from '@elastic/eui';
import * as React from 'react';
import ReactMarkdown from 'react-markdown';
Expand Down
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 { shallow } from 'enzyme';
import * as React from 'react';

Expand Down
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 React from 'react';
import { EuiFlexGroup, EuiFlexItem, EuiLink } from '@elastic/eui';
import { Columns } from '../../paginated_table';
Expand Down
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 React from 'react';
import { EuiFlexGroup, EuiFlexItem, EuiLink } from '@elastic/eui';

Expand Down
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 chrome from 'ui/chrome';
import React, { useEffect, useState } from 'react';

Expand Down
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 * as React from 'react';

import { EuiTableDataType } from '@elastic/eui';
Expand Down
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 { EuiButtonIcon, EuiToolTip } from '@elastic/eui';
import * as React from 'react';

Expand Down
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 { EuiButtonIcon, EuiLink } from '@elastic/eui';
import { omit } from 'lodash/fp';
import * as React from 'react';
Expand Down
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 * as React from 'react';

import { defaultToEmptyTag } from '../../empty_value';
Expand Down
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, EuiToolTip } from '@elastic/eui';
import * as React from 'react';

Expand Down
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 */

Comment on lines +7 to +8
Copy link
Member

Choose a reason for hiding this comment

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

Just noting the consistency of naming our connected components. Seems it's hit or miss on if we set the displayName after calling connect(). Here in AuthenticationTable we do not set displayName after connecting, but we do for UncommonProcessTable. The linter rule does not enforce it for connected components, so we'll need to keep an eye out in reviews (or perhaps see if the updated react-redux can handle setting the displayName?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

with react-redux@7 we will be able to migrate from connect to hooks, so the linter will be able to properly enforce the rule 💪

import { has } from 'lodash/fp';
import React, { useCallback } from 'react';
import { connect } from 'react-redux';
Expand Down
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 React, { useCallback } from 'react';
import { connect } from 'react-redux';
import { ActionCreator } from 'typescript-fsa';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ export const KpiNetworkBaseComponent = React.memo<{
);
});

KpiNetworkBaseComponent.displayName = 'KpiNetworkBaseComponent';

export const KpiNetworkComponent = React.memo<KpiNetworkProps>(
({ data, from, id, loading, to, narrowDateRange }) => {
return loading ? (
Expand Down
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 React from 'react';
import numeral from '@elastic/numeral';
import { NetworkHttpEdges, NetworkHttpFields, NetworkHttpItem } from '../../../../graphql/types';
Expand Down
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 React from 'react';
import moment from 'moment';
import { TlsNode } from '../../../../graphql/types';
Expand Down
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 euiDarkVars from '@elastic/eui/dist/eui_theme_dark.json';
import { mount, ReactWrapper } from 'enzyme';
import toJson from 'enzyme-to-json';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ const Attributes = React.memo<AttributesProps>(({ children }) => {
);
});

Attributes.displayName = 'Attributes';

export const StatefulEvent = React.memo<Props>(
({
actionsColumnWidth,
Expand Down
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 { IconType } from '@elastic/eui';
import { get } from 'lodash/fp';
import React from 'react';
Expand Down
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 * as React from 'react';

import { TimelineNonEcsData } from '../../../../graphql/types';
Expand Down
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 { get } from 'lodash/fp';
import React from 'react';
import styled from 'styled-components';
Expand Down
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 React from 'react';

import { RowRenderer } from './row_renderer';
Expand Down
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 { get } from 'lodash/fp';
import React from 'react';

Expand Down
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 { get } from 'lodash/fp';
import React from 'react';

Expand Down
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 { get } from 'lodash/fp';
import React from 'react';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ const TimelineRefetchComponent = memo<OwnProps>(
}
);

TimelineRefetchComponent.displayName = 'TimelineRefetchComponent';

export const TimelineRefetch = compose<React.ComponentClass<TimelineRefetchProps>>(
connect(null, {
setTimelineQuery: inputsActions.setQuery,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,5 @@ export const ManageTimelineContext = memo<ManageTimelineContextProps>(
);
}
);

ManageTimelineContext.displayName = 'ManageTimelineContext';
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,5 @@ export const UseUrlState = React.memo<UrlStateProps>(props => {
};
return <UrlStateRedux {...urlStateReduxProps} />;
});

UseUrlState.displayName = 'UseUrlState';
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export const GlobalTimeComponent = React.memo(
}
);

GlobalTimeComponent.displayName = 'GlobalTimeComponent';

const mapStateToProps = (state: State) => {
const timerange: inputsModel.TimeRange = inputsSelectors.globalTimeRangeSelector(state);
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,5 @@ export const AllTimelinesQuery = React.memo<OwnProps>(
);
}
);

AllTimelinesQuery.displayName = 'AllTimelinesQuery';
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,5 @@ export const TimelineDetailsComponentQuery = React.memo<TimelineDetailsProps>(
);
}
);

TimelineDetailsComponentQuery.displayName = 'TimelineDetailsComponentQuery';
2 changes: 2 additions & 0 deletions x-pack/legacy/plugins/siem/public/mock/kibana_react.ts
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 React from 'react';
import { KibanaContextProvider } from '../../../../../../src/plugins/kibana_react/public';

Expand Down
4 changes: 4 additions & 0 deletions x-pack/legacy/plugins/siem/public/mock/test_providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,14 @@ export const TestProviders = React.memo<Props>(
)
);

TestProviders.displayName = 'TestProviders';

export const TestProviderWithoutDragAndDrop = React.memo<Props>(
({ children, store = createStore(state, apolloClientObservable) }) => (
<I18nProvider>
<ReduxStoreProvider store={store}>{children}</ReduxStoreProvider>
</I18nProvider>
)
);

TestProviderWithoutDragAndDrop.displayName = 'TestProviderWithoutDragAndDrop';
Loading