Skip to content

Commit

Permalink
[Security Solutions] Removes tech debt of exporting all from linter r…
Browse files Browse the repository at this point in the history
…ule for cases plugin in the common section (#120310)

## Summary

See: #110903, #120234

This removes as many top level API `export *` spots from:
* `cases` plugin within the common section

as we can. This reduces the number of metrics and warning about undocumented functions and reduces the page load size from `cases/common/index.ts`. Look at the metrics from the build below and you will see drop off numbers across the board for required API documentation to the page load size.

In the file `cases/common/index.ts` I now put the advice of:

```
// Careful of exporting anything from this file as any file(s) you export here will cause your page bundle size to increase.
// If you're using functions/types/etc... internally or within integration tests it's best to import directly from their paths
// than expose the functions/types/etc... here. You should _only_ expose functions/types/etc... that need to be shared with other plugins here.

// When you do have to add things here you might want to consider creating a package such as kbn-cases-constants to share with
// other plugins instead as packages are easier to break down and you do not have to carry the cost of extra plugin weight on
// first download since the other plugins/areas of your code can directly pull from the package in their async imports.
// For example, constants below could eventually be in a "kbn-cases-constants" instead.
// See: https://docs.elastic.dev/kibana-dev-docs/key-concepts/platform-intro#public-plugin-api
```

Some of those that are exposed such as `throwErrors` might actually be small simple mistakes as `security_solution` is using it but it has a "copy" of the same utility within just its server section rather than within its common section. That can be done in a different cleanup PR and cases team can decide what to do moving forward with their API before or post 8.0.0 release.


For the metric increasing of:
| id | [before](f01106c) | [after](f2e5d6a) | diff |
| --- | --- | --- | --- |
| `cases` | 16 | 22 | +6 |

Running that suggestion:

```sh
node --max-old-space-size=6096 scripts/build_api_docs --plugin cases --stats exports
```

I see this:
<img width="1851" alt="Screen Shot 2021-12-03 at 9 30 45 AM" src="https://user-images.githubusercontent.com/1151048/144638952-43d50478-ea12-4ce1-8f73-585c735772b4.png">

I don't know if there is a way just yet to mark undocumented public API's but I don't feel concerned with it at the moment and if the case team wants to re-expose those or are going to support API's through documentation they can decide what to do. This PR is more about just removing as much as possible to start with and then go the other direction where the individual teams can decide what to expose and if the download weight is worth it or if it's just `export type` and holds no weight, etc...
  • Loading branch information
FrankHassanabad authored Dec 6, 2021
1 parent 9a8d888 commit 9110908
Show file tree
Hide file tree
Showing 280 changed files with 452 additions and 450 deletions.
33 changes: 25 additions & 8 deletions x-pack/plugins/cases/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,28 @@
* 2.0.
*/

// TODO: https://github.com/elastic/kibana/issues/110896
/* eslint-disable @kbn/eslint/no_export_all */

export * from './constants';
export * from './api';
export * from './ui/types';
export * from './utils/connectors_api';
export * from './utils/user_actions';
// Careful of exporting anything from this file as any file(s) you export here will cause your page bundle size to increase.
// If you're using functions/types/etc... internally or within integration tests it's best to import directly from their paths
// than expose the functions/types/etc... here. You should _only_ expose functions/types/etc... that need to be shared with other plugins here.

// When you do have to add things here you might want to consider creating a package such as kbn-cases-constants to share with
// other plugins instead as packages are easier to break down and you do not have to carry the cost of extra plugin weight on
// first download since the other plugins/areas of your code can directly pull from the package in their async imports.
// For example, constants below could eventually be in a "kbn-cases-constants" instead.
// See: https://docs.elastic.dev/kibana-dev-docs/key-concepts/platform-intro#public-plugin-api

export { CASES_URL, SECURITY_SOLUTION_OWNER, ENABLE_CASE_CONNECTOR } from './constants';

export { CommentType, CaseStatuses, getCasesFromAlertsUrl, throwErrors } from './api';

export type {
SubCase,
Case,
Ecs,
CasesContextValue,
CaseViewRefreshPropInterface,
} from './ui/types';

export { StatusAll } from './ui/types';

export { getCreateConnectorUrl, getAllConnectorsUrl } from './utils/connectors_api';
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/common/utils/connectors_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* Actions and connectors API endpoint helpers
*/

import { ACTION_URL, ACTION_TYPES_URL, CONNECTORS_URL } from '../../common';
import { ACTION_URL, ACTION_TYPES_URL, CONNECTORS_URL } from '../../common/constants';

/**
*
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/public/common/lib/kibana/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import moment from 'moment-timezone';
import { useCallback, useEffect, useState } from 'react';
import { i18n } from '@kbn/i18n';

import { DEFAULT_DATE_FORMAT, DEFAULT_DATE_FORMAT_TZ } from '../../../../common';
import { DEFAULT_DATE_FORMAT, DEFAULT_DATE_FORMAT_TZ } from '../../../../common/constants';
import { AuthenticatedUser } from '../../../../../security/common/model';
import { convertToCamelCase } from '../../../containers/utils';
import { StartServices } from '../../../types';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { TriggersAndActionsUIPublicPluginStart } from '../../../../triggers_actions_ui/public';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { actionTypeRegistryMock } from '../../../../triggers_actions_ui/public/application/action_type_registry.mock';
import { CaseActionConnector } from '../../../common';
import { CaseActionConnector } from '../../../common/ui/types';

const getUniqueActionTypeIds = (connectors: CaseActionConnector[]) =>
new Set(connectors.map((connector) => connector.actionTypeId));
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/cases/public/common/mock/test_providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import { merge } from 'lodash';
import { euiDarkVars } from '@kbn/ui-shared-deps-src/theme';
import { I18nProvider } from '@kbn/i18n-react';
import { ThemeProvider } from 'styled-components';
import { CasesContextValue, DEFAULT_FEATURES, SECURITY_SOLUTION_OWNER } from '../../../common';
import { DEFAULT_FEATURES, SECURITY_SOLUTION_OWNER } from '../../../common/constants';
import { CasesContextValue } from '../../../common/ui/types';
import { CasesProvider } from '../../components/cases_context';
import { createKibanaContextProviderMock } from '../lib/kibana/kibana_react.mock';
import { FieldHook } from '../shared_imports';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { ConnectorTypes, noneConnectorId } from '../../../common';
import { ConnectorTypes, noneConnectorId } from '../../../common/api';
import { parseStringAsConnector, parseStringAsExternalService } from './parsers';

describe('user actions utility functions', () => {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/public/common/user_actions/parsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
noneConnectorId,
CaseFullExternalService,
CaseUserActionExternalServiceRt,
} from '../../../common';
} from '../../../common/api';

export const parseStringAsConnector = (
id: string | null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import { noop } from 'lodash/fp';

import { TestProviders } from '../../common/mock';

import { CommentRequest, CommentType, SECURITY_SOLUTION_OWNER } from '../../../common';
import { CommentRequest, CommentType } from '../../../common/api';
import { SECURITY_SOLUTION_OWNER } from '../../../common/constants';
import { usePostComment } from '../../containers/use_post_comment';
import { AddComment, AddCommentProps, AddCommentRefObject } from '.';
import { CasesTimelineIntegrationProvider } from '../timeline_context';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { EuiButton, EuiFlexItem, EuiFlexGroup, EuiLoadingSpinner } from '@elasti
import styled from 'styled-components';
import { isEmpty } from 'lodash';

import { CommentType } from '../../../common';
import { CommentType } from '../../../common/api';
import { usePostComment } from '../../containers/use_post_comment';
import { Case } from '../../containers/types';
import { EuiMarkdownEditorRef, MarkdownEditorForm } from '../markdown_editor';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { CommentRequestUserType } from '../../../common';
import { CommentRequestUserType } from '../../../common/api';
import { FIELD_TYPES, fieldValidators, FormSchema } from '../../common/shared_imports';
import * as i18n from './translations';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import {
connectorsMock,
} from '../../containers/mock';

import { CaseStatuses, CaseType, SECURITY_SOLUTION_OWNER, StatusAll } from '../../../common';
import { StatusAll } from '../../../common/ui/types';
import { CaseStatuses, CaseType } from '../../../common/api';
import { SECURITY_SOLUTION_OWNER } from '../../../common/constants';
import { getEmptyTagValue } from '../empty_value';
import { useDeleteCases } from '../../containers/use_delete_cases';
import { useGetCases } from '../../containers/use_get_cases';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,12 @@ import classnames from 'classnames';

import {
Case,
CaseStatuses,
CaseType,
CommentRequestAlertType,
CaseStatusWithAllStatus,
FilterOptions,
SortFieldCase,
SubCase,
caseStatuses,
} from '../../../common';
} from '../../../common/ui/types';
import { CaseStatuses, CaseType, CommentRequestAlertType, caseStatuses } from '../../../common/api';
import { SELECTABLE_MESSAGE_COLLECTIONS } from '../../common/translations';
import { useGetCases } from '../../containers/use_get_cases';
import { usePostComment } from '../../containers/use_post_comment';
Expand Down
6 changes: 2 additions & 4 deletions x-pack/plugins/cases/public/components/all_cases/columns.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,14 @@ import {
import { RIGHT_ALIGNMENT } from '@elastic/eui/lib/services';
import styled from 'styled-components';

import { Case, DeleteCase, SubCase } from '../../../common/ui/types';
import {
CaseStatuses,
CaseType,
CommentType,
CommentRequestAlertType,
DeleteCase,
Case,
SubCase,
ActionConnector,
} from '../../../common';
} from '../../../common/api';
import { getEmptyTagValue } from '../empty_value';
import { FormattedRelativePreferenceDate } from '../formatted_date';
import { CaseDetailsLink } from '../links';
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/public/components/all_cases/count.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import React, { FunctionComponent, useEffect } from 'react';
import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { CaseStatuses } from '../../../common';
import { CaseStatuses } from '../../../common/api';
import { Stats } from '../status';
import { useGetCasesStatus } from '../../containers/use_get_cases_status';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { EuiBasicTable } from '@elastic/eui';
import styled from 'styled-components';
import { Case, SubCase } from '../../containers/types';
import { CasesColumns } from './columns';
import { AssociationType } from '../../../common';
import { AssociationType } from '../../../common/api';

type ExpandedRowMap = Record<string, Element> | {};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { filter } from 'lodash/fp';
import { AssociationType, CaseStatuses, CaseType } from '../../../common';
import { AssociationType, CaseStatuses, CaseType } from '../../../common/api';
import { Case, SubCase } from '../../containers/types';
import { statuses } from '../status';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { useGetReporters } from '../../containers/use_get_reporters';
import { useGetActionLicense } from '../../containers/use_get_action_license';
import { useConnectors } from '../../containers/configure/use_connectors';
import { useKibana } from '../../common/lib/kibana';
import { CaseStatuses } from '../../../common';
import { CaseStatuses } from '../../../common/api';
import { casesStatus, connectorsMock, useGetCasesMockState } from '../../containers/mock';
import { registerConnectorsToMockActionRegistry } from '../../common/mock/register_connectors';
import { useGetCases } from '../../containers/use_get_cases';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { mount } from 'enzyme';
import { AllCasesSelectorModal } from '.';
import { TestProviders } from '../../../common/mock';
import { AllCasesList } from '../all_cases_list';
import { SECURITY_SOLUTION_OWNER } from '../../../../common';
import { SECURITY_SOLUTION_OWNER } from '../../../../common/constants';

jest.mock('../../../methods');
jest.mock('../all_cases_list');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,8 @@ import {
EuiModalHeaderTitle,
} from '@elastic/eui';
import styled from 'styled-components';
import {
Case,
CaseStatusWithAllStatus,
CommentRequestAlertType,
SubCase,
} from '../../../../common';
import { Case, SubCase, CaseStatusWithAllStatus } from '../../../../common/ui/types';
import { CommentRequestAlertType } from '../../../../common/api';
import * as i18n from '../../../common/translations';
import { AllCasesList } from '../all_cases_list';
export interface AllCasesSelectorModalProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import React from 'react';
import { mount } from 'enzyme';
import { waitFor } from '@testing-library/react';

import { CaseStatuses, StatusAll } from '../../../common';
import { StatusAll } from '../../../common/ui/types';
import { CaseStatuses } from '../../../common/api';
import { StatusFilter } from './status_filter';

const stats = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import React, { memo } from 'react';
import { EuiSuperSelect, EuiSuperSelectOption, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { Status, statuses } from '../status';
import { CaseStatusWithAllStatus, StatusAll } from '../../../common';
import { CaseStatusWithAllStatus, StatusAll } from '../../../common/ui/types';

interface Props {
stats: Record<CaseStatusWithAllStatus, number | null>;
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/public/components/all_cases/table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import styled from 'styled-components';

import { CasesTableUtilityBar } from './utility_bar';
import { LinkButton } from '../links';
import { AllCases, Case, FilterOptions } from '../../../common';
import { AllCases, Case, FilterOptions } from '../../../common/ui/types';
import * as i18n from './translations';
import { useCreateCaseNavigation } from '../../common/navigation';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import React from 'react';
import { mount } from 'enzyme';

import { CaseStatuses } from '../../../common';
import { CaseStatuses } from '../../../common/api';
import { TestProviders } from '../../common/mock';
import { useGetTags } from '../../containers/use_get_tags';
import { useGetReporters } from '../../containers/use_get_reporters';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import { isEqual } from 'lodash/fp';
import styled from 'styled-components';
import { EuiFlexGroup, EuiFlexItem, EuiFieldSearch, EuiFilterGroup } from '@elastic/eui';

import { CaseStatuses, CaseStatusWithAllStatus, StatusAll } from '../../../common';
import { StatusAll, CaseStatusWithAllStatus } from '../../../common/ui/types';
import { CaseStatuses } from '../../../common/api';
import { FilterOptions } from '../../containers/types';
import { useGetTags } from '../../containers/use_get_tags';
import { useGetReporters } from '../../containers/use_get_reporters';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
UtilityBarText,
} from '../utility_bar';
import * as i18n from './translations';
import { AllCases, Case, DeleteCase, FilterOptions } from '../../../common';
import { AllCases, Case, DeleteCase, FilterOptions } from '../../../common/ui/types';
import { getBulkItems } from '../bulk_actions';
import { isSelectedCasesIncludeCollections } from './helpers';
import { useDeleteCases } from '../../containers/use_delete_cases';
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/public/components/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { MutableRefObject } from 'react';
import { Ecs, CaseViewRefreshPropInterface } from '../../../common';
import { Ecs, CaseViewRefreshPropInterface } from '../../../common/ui/types';
import { CasesNavigation } from '../links';
import { CasesTimelineIntegration } from '../timeline_context';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
import React from 'react';
import { EuiContextMenuItem } from '@elastic/eui';

import { CaseStatuses, CaseStatusWithAllStatus } from '../../../common';
import { CaseStatusWithAllStatus } from '../../../common/ui/types';
import { CaseStatuses } from '../../../common/api';
import { statuses } from '../status';
import * as i18n from './translations';
import { Case } from '../../containers/types';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as i18n from '../case_view/translations';
import { useDeleteCases } from '../../containers/use_delete_cases';
import { ConfirmDeleteCaseModal } from '../confirm_delete_case';
import { PropertyActions } from '../property_actions';
import { Case } from '../../../common';
import { Case } from '../../../common/ui/types';
import { CaseService } from '../../containers/use_get_case_user_actions';
import { useAllCasesNavigation } from '../../common/navigation';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { CaseStatuses } from '../../../common';
import { CaseStatuses } from '../../../common/api';
import { basicCase } from '../../containers/mock';
import { getStatusDate, getStatusTitle } from './helpers';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { CaseStatuses } from '../../../common';
import { CaseStatuses } from '../../../common/api';
import { Case } from '../../containers/types';
import { statuses } from '../status';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
EuiFlexItem,
EuiIconTip,
} from '@elastic/eui';
import { Case, CaseStatuses, CaseType } from '../../../common';
import { Case } from '../../../common/ui/types';
import { CaseStatuses, CaseType } from '../../../common/api';
import * as i18n from '../case_view/translations';
import { FormattedRelativePreferenceDate } from '../formatted_date';
import { Actions } from './actions';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import React from 'react';
import { mount } from 'enzyme';

import { CaseStatuses } from '../../../common';
import { CaseStatuses } from '../../../common/api';
import { StatusContextMenu } from './status_context_menu';

describe('SyncAlertsSwitch', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import React, { memo, useCallback, useMemo, useState } from 'react';
import { EuiPopover, EuiContextMenuPanel, EuiContextMenuItem } from '@elastic/eui';
import { caseStatuses, CaseStatuses } from '../../../common';
import { caseStatuses, CaseStatuses } from '../../../common/api';
import { Status } from '../status';
import { CHANGE_STATUS } from '../all_cases/translations';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
* 2.0.
*/

import { AssociationType, CommentType, SECURITY_SOLUTION_OWNER } from '../../../common';
import { AssociationType, CommentType } from '../../../common/api';
import { SECURITY_SOLUTION_OWNER } from '../../../common/constants';
import { Comment } from '../../containers/types';

import { getManualAlertIdsWithNoRuleId } from './helpers';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { isEmpty } from 'lodash';
import { CommentType } from '../../../common';
import { CommentType } from '../../../common/api';
import { Comment } from '../../containers/types';

export const getManualAlertIdsWithNoRuleId = (comments: Comment[]): string[] => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { useGetCaseUserActions } from '../../containers/use_get_case_user_action
import { useConnectors } from '../../containers/configure/use_connectors';
import { connectorsMock } from '../../containers/configure/mock';
import { usePostPushToService } from '../../containers/use_post_push_to_service';
import { CaseType, ConnectorTypes } from '../../../common';
import { CaseType, ConnectorTypes } from '../../../common/api';
import { useKibana } from '../../common/lib/kibana';

jest.mock('../../containers/use_update_case');
Expand Down
Loading

0 comments on commit 9110908

Please sign in to comment.