Skip to content

Commit

Permalink
[Security Solutions][Lists] Trims down list plugin size by breaking o…
Browse files Browse the repository at this point in the history
…ut the exception builder into chunks by using react lazy loading (elastic#99989)

## Summary

Trims down the list plugin size by breaking out the exception builder into a dedicated chunk by using React Suspense and React lazy loading.

Before this PR the page load bundle size was `260503`, after the page load bundle size will be `194132`:

You can calculate this through:
```ts
node ./scripts/build_kibana_platform_plugins --dist --focus lists
cat ./x-pack/plugins/lists/target/public/metrics.json
```

Before
```json
[
  {
    "group": "@kbn/optimizer bundle module count",
    "id": "lists",
    "value": 227
  },
  {
    "group": "page load bundle size",
    "id": "lists",
    "value": 260503, <--- Very large load bundle size
    "limit": 280504,
    "limitConfigPath": "packages/kbn-optimizer/limits.yml"
  },
  {
    "group": "async chunks size",
    "id": "lists",
    "value": 0
  },
  {
    "group": "async chunk count",
    "id": "lists",
    "value": 0
  },
  {
    "group": "miscellaneous assets size",
    "id": "lists",
    "value": 0
  }
]
```

After:
```json
[
  {
    "group": "@kbn/optimizer bundle module count",
    "id": "lists",
    "value": 227
  },
  {
    "group": "page load bundle size",
    "id": "lists",
    "value": 194132, <--- Not as large bundle size
    "limit": 280504,
    "limitConfigPath": "packages/kbn-optimizer/limits.yml"
  },
  {
    "group": "async chunks size",
    "id": "lists",
    "value": 70000
  },
  {
    "group": "async chunk count",
    "id": "lists",
    "value": 1
  },
  {
    "group": "miscellaneous assets size",
    "id": "lists",
    "value": 0
  }
]
```

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
FrankHassanabad authored and yctercero committed May 25, 2021
1 parent f9654a7 commit 7e80099
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -444,3 +444,6 @@ export const ExceptionBuilderComponent = ({
};

ExceptionBuilderComponent.displayName = 'ExceptionBuilder';

// eslint-disable-next-line import/no-default-export
export default ExceptionBuilderComponent;
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,32 @@
* 2.0.
*/

export { BuilderEntryItem } from './entry_renderer';
export { BuilderExceptionListItemComponent } from './exception_item_renderer';
export { ExceptionBuilderComponent, OnChangeProps } from './exception_items_renderer';
import { EuiLoadingSpinner } from '@elastic/eui';
import React, { Suspense, lazy } from 'react';

// Note: Only use import type/export type here to avoid pulling anything non-lazy into the main plugin and increasing the plugin size
import type { ExceptionBuilderProps } from './exception_items_renderer';
export type { OnChangeProps } from './exception_items_renderer';

interface ExtraProps {
dataTestSubj: string;
idAria: string;
}

/**
* This lazy load allows the exception builder to pull everything out into a plugin chunk.
* You want to be careful of not directly importing/exporting things from exception_items_renderer
* unless you use a import type, and/or a export type to ensure full type erasure
*/
const ExceptionBuilderComponentLazy = lazy(() => import('./exception_items_renderer'));
export const getExceptionBuilderComponentLazy = (
props: ExceptionBuilderProps & ExtraProps
): JSX.Element => (
<Suspense fallback={<EuiLoadingSpinner />}>
<ExceptionBuilderComponentLazy
data-test-subj={props.dataTestSubj}
id-aria={props.idAria}
{...props}
/>
</Suspense>
);
2 changes: 1 addition & 1 deletion x-pack/plugins/lists/public/exceptions/transforms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { flow } from 'fp-ts/lib/function';
import { addIdToItem, removeIdFromItem } from '@kbn/securitysolution-utils';

import {
import type {
CreateExceptionListItemSchema,
EntriesArray,
Entry,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,14 @@ describe('When the add exception modal is opened', () => {
ReturnType<typeof helpers.defaultEndpointExceptionItems>
>;
let ExceptionBuilderComponent: jest.SpyInstance<
ReturnType<typeof ExceptionBuilder.ExceptionBuilderComponent>
ReturnType<typeof ExceptionBuilder.getExceptionBuilderComponentLazy>
>;
beforeEach(() => {
const emptyComp = <span data-test-subj="alert-exception-builder" />;
defaultEndpointItems = jest.spyOn(helpers, 'defaultEndpointExceptionItems');
ExceptionBuilderComponent = jest
.spyOn(ExceptionBuilder, 'ExceptionBuilderComponent')
.mockReturnValue(<></>);
.spyOn(ExceptionBuilder, 'getExceptionBuilderComponentLazy')
.mockReturnValue(emptyComp);

(useAsync as jest.Mock).mockImplementation(() => ({
start: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,28 +469,27 @@ export const AddExceptionModal = memo(function AddExceptionModal({
<EuiSpacer size="l" />
</>
)}
<ExceptionBuilder.ExceptionBuilderComponent
allowLargeValueLists={
!isEqlRule(maybeRule?.type) && !isThresholdRule(maybeRule?.type)
}
httpService={http}
autocompleteService={data.autocomplete}
exceptionListItems={initialExceptionItems}
listType={exceptionListType}
osTypes={osTypesSelection}
listId={ruleExceptionList.list_id}
listNamespaceType={ruleExceptionList.namespace_type}
listTypeSpecificIndexPatternFilter={filterIndexPatterns}
ruleName={ruleName}
indexPatterns={indexPatterns}
isOrDisabled={isExceptionBuilderFormDisabled}
isAndDisabled={isExceptionBuilderFormDisabled}
isNestedDisabled={isExceptionBuilderFormDisabled}
data-test-subj="alert-exception-builder"
id-aria="alert-exception-builder"
onChange={handleBuilderOnChange}
isDisabled={isExceptionBuilderFormDisabled}
/>
{ExceptionBuilder.getExceptionBuilderComponentLazy({
allowLargeValueLists:
!isEqlRule(maybeRule?.type) && !isThresholdRule(maybeRule?.type),
httpService: http,
autocompleteService: data.autocomplete,
exceptionListItems: initialExceptionItems,
listType: exceptionListType,
osTypes: osTypesSelection,
listId: ruleExceptionList.list_id,
listNamespaceType: ruleExceptionList.namespace_type,
listTypeSpecificIndexPatternFilter: filterIndexPatterns,
ruleName,
indexPatterns,
isOrDisabled: isExceptionBuilderFormDisabled,
isAndDisabled: isExceptionBuilderFormDisabled,
isNestedDisabled: isExceptionBuilderFormDisabled,
dataTestSubj: 'alert-exception-builder',
idAria: 'alert-exception-builder',
onChange: handleBuilderOnChange,
isDisabled: isExceptionBuilderFormDisabled,
})}

<EuiSpacer />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ jest.mock('../../../../detections/containers/detection_engine/alerts/use_signal_
jest.mock('../../../../detections/containers/detection_engine/rules/use_rule_async');
jest.mock('../../../../shared_imports', () => {
const originalModule = jest.requireActual('../../../../shared_imports');

const emptyComp = <span data-test-subj="edit-exception-modal-builder" />;
return {
...originalModule,
ExceptionBuilder: {
ExceptionBuilderComponent: () => ({} as JSX.Element),
getExceptionBuilderComponentLazy: () => emptyComp,
},
};
});
Expand All @@ -62,13 +62,14 @@ describe('When the edit exception modal is opened', () => {
const ruleName = 'test rule';

let ExceptionBuilderComponent: jest.SpyInstance<
ReturnType<typeof ExceptionBuilder.ExceptionBuilderComponent>
ReturnType<typeof ExceptionBuilder.getExceptionBuilderComponentLazy>
>;

beforeEach(() => {
const emptyComp = <span data-test-subj="edit-exception-modal-builder" />;
ExceptionBuilderComponent = jest
.spyOn(ExceptionBuilder, 'ExceptionBuilderComponent')
.mockReturnValue(<></>);
.spyOn(ExceptionBuilder, 'getExceptionBuilderComponentLazy')
.mockReturnValue(emptyComp);

(useSignalIndex as jest.Mock).mockReturnValue({
loading: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,27 +342,26 @@ export const EditExceptionModal = memo(function EditExceptionModal({
<EuiSpacer />
</>
)}
<ExceptionBuilder.ExceptionBuilderComponent
allowLargeValueLists={
!isEqlRule(maybeRule?.type) && !isThresholdRule(maybeRule?.type)
}
httpService={http}
autocompleteService={data.autocomplete}
exceptionListItems={[exceptionItem]}
listType={exceptionListType}
listId={exceptionItem.list_id}
listNamespaceType={exceptionItem.namespace_type}
listTypeSpecificIndexPatternFilter={filterIndexPatterns}
ruleName={ruleName}
isOrDisabled
isAndDisabled={false}
osTypes={exceptionItem.os_types}
isNestedDisabled={false}
data-test-subj="edit-exception-modal-builder"
id-aria="edit-exception-modal-builder"
onChange={handleBuilderOnChange}
indexPatterns={indexPatterns}
/>
{ExceptionBuilder.getExceptionBuilderComponentLazy({
allowLargeValueLists:
!isEqlRule(maybeRule?.type) && !isThresholdRule(maybeRule?.type),
httpService: http,
autocompleteService: data.autocomplete,
exceptionListItems: [exceptionItem],
listType: exceptionListType,
listId: exceptionItem.list_id,
listNamespaceType: exceptionItem.namespace_type,
listTypeSpecificIndexPatternFilter: filterIndexPatterns,
ruleName,
isOrDisabled: true,
isAndDisabled: false,
osTypes: exceptionItem.os_types,
isNestedDisabled: false,
dataTestSubj: 'edit-exception-modal-builder',
idAria: 'edit-exception-modal-builder',
onChange: handleBuilderOnChange,
indexPatterns,
})}

<EuiSpacer />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { createGlobalNoMiddlewareStore, ecsEventMock } from '../../../test_utils
import { getMockTheme } from '../../../../../../common/lib/kibana/kibana_react.mock';
import { NAME_ERROR, NAME_PLACEHOLDER } from './translations';
import { useCurrentUser, useKibana } from '../../../../../../common/lib/kibana';
import { ExceptionBuilder } from '../../../../../../shared_imports';

jest.mock('../../../../../../common/lib/kibana');
jest.mock('../../../../../../common/containers/source');
Expand Down Expand Up @@ -53,6 +54,9 @@ describe('Event filter form', () => {
};

beforeEach(() => {
const emptyComp = <span data-test-subj="alert-exception-builder" />;
jest.spyOn(ExceptionBuilder, 'getExceptionBuilderComponentLazy').mockReturnValue(emptyComp);

(useFetchIndex as jest.Mock).mockImplementation(() => [
false,
{
Expand All @@ -77,7 +81,7 @@ describe('Event filter form', () => {
it('should renders correctly with data', () => {
component = renderComponentWithdata();

expect(component.getByText(ecsEventMock().process!.executable![0])).not.toBeNull();
expect(component.getByTestId('alert-exception-builder')).not.toBeNull();
expect(component.getByText(NAME_ERROR)).not.toBeNull();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,26 +115,25 @@ export const EventFiltersForm: React.FC<EventFiltersFormProps> = memo(
);

const exceptionBuilderComponentMemo = useMemo(
() => (
<ExceptionBuilder.ExceptionBuilderComponent
allowLargeValueLists
httpService={http}
autocompleteService={data.autocomplete}
exceptionListItems={[exception as ExceptionListItemSchema]}
listType={EVENT_FILTER_LIST_TYPE}
listId={ENDPOINT_EVENT_FILTERS_LIST_ID}
listNamespaceType={'agnostic'}
ruleName={RULE_NAME}
indexPatterns={indexPatterns}
isOrDisabled={true} // TODO: pending to be validated
isAndDisabled={false}
isNestedDisabled={false}
data-test-subj="alert-exception-builder"
id-aria="alert-exception-builder"
onChange={handleOnBuilderChange}
listTypeSpecificIndexPatternFilter={filterIndexPatterns}
/>
),
() =>
ExceptionBuilder.getExceptionBuilderComponentLazy({
allowLargeValueLists: true,
httpService: http,
autocompleteService: data.autocomplete,
exceptionListItems: [exception as ExceptionListItemSchema],
listType: EVENT_FILTER_LIST_TYPE,
listId: ENDPOINT_EVENT_FILTERS_LIST_ID,
listNamespaceType: 'agnostic',
ruleName: RULE_NAME,
indexPatterns,
isOrDisabled: true, // TODO: pending to be validated
isAndDisabled: false,
isNestedDisabled: false,
dataTestSubj: 'alert-exception-builder',
idAria: 'alert-exception-builder',
onChange: handleOnBuilderChange,
listTypeSpecificIndexPatternFilter: filterIndexPatterns,
}),
[data, handleOnBuilderChange, http, indexPatterns, exception]
);

Expand Down

0 comments on commit 7e80099

Please sign in to comment.