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 (#99989) (#100073)

## 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

Co-authored-by: Frank Hassanabad <[email protected]>
  • Loading branch information
kibanamachine and FrankHassanabad authored May 13, 2021
1 parent 8d9110f commit 37effff
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 37effff

Please sign in to comment.