Skip to content

Commit

Permalink
[SecuritySolution] Remove remaining usage of redux-observable (elas…
Browse files Browse the repository at this point in the history
…tic#175678)

## Summary

In this PR, we're removing all usages of `redux-observable` in favor of
simple middlewares. This work is part of [this tech debt
ticket](elastic#175427) which outlines
the motivation of this move.

A couple shortcuts had to be taken and I added comments further down to
explain the motivation.

### Oddities
Weirdly, the CI reports an increase in async chunks, instead of an
expected decrease due to removing a library.

| id |
[before](elastic@9629e66)
|
[after](elastic@477348a)
| diff |
| --- | --- | --- | --- |
| `securitySolution` | 11.2MB | 11.4MB | +157.1KB |

I'm not sure, why this is, so if anyone has any insights on how to
examine the async chunks, that would be helpful.

_edit:_ After hours of analyzing Kibana build stats at various stages of
this PR, I found that the changes that are responsible for this increase
in async chunk sum size is this commit:
janmonschke@cde023d
. It only consists of deleted files and deleted imports. Therefore it's
up to webpack to figure out the best way to chunk up the app. In other
words: I can't change it :(

### Tests

You might notice, that I didn't add tests to the middlewares. That is
because the epics didn't have tests either and their functionality is
tested in acceptance tests. As a follow-up of this PR, I will add tests
to all newly-introduced middlewares. My goal here is to keep the changes
as small as possible.

### Checklist

- [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: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
2 people authored and CoenWarmer committed Feb 15, 2024
1 parent b6493bb commit 89a1d79
Show file tree
Hide file tree
Showing 26 changed files with 674 additions and 1,094 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1080,7 +1080,6 @@
"redux-actions": "^2.6.5",
"redux-devtools-extension": "^2.13.8",
"redux-logger": "^3.0.6",
"redux-observable": "2.0.0",
"redux-saga": "^1.1.3",
"redux-thunk": "^2.4.2",
"redux-thunks": "^1.0.0",
Expand Down
85 changes: 0 additions & 85 deletions x-pack/plugins/infra/types/redux_observable.d.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import { OverviewCardWithActions, OverviewCard } from './overview_card';
import { StatusPopoverButton } from './status_popover_button';
import { SeverityBadge } from '../../severity_badge';
import { useThrottledResizeObserver } from '../../utils';
import { isNotNull } from '../../../../timelines/store/helpers';

export const NotGrowingFlexGroup = euiStyled(EuiFlexGroup)`
flex-grow: 0;
Expand Down Expand Up @@ -219,4 +218,8 @@ function hasData(fieldInfo?: EnrichedFieldInfo): fieldInfo is EnrichedFieldInfoW
return !!fieldInfo && Array.isArray(fieldInfo.values);
}

function isNotNull<T>(value: T | null): value is T {
return value !== null;
}

Overview.displayName = 'Overview';
35 changes: 0 additions & 35 deletions x-pack/plugins/security_solution/public/common/store/epic.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,18 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import type { CoreStart } from '@kbn/core/public';
import type { Storage } from '@kbn/kibana-utils-plugin/public';

import { createTimelineMiddlewares } from '../../timelines/store/middlewares/create_timeline_middlewares';
import { dataTableLocalStorageMiddleware } from './data_table/middleware_local_storage';
import { userAssetTableLocalStorageMiddleware } from '../../explore/users/store/middleware_storage';

export function createMiddlewares(storage: Storage) {
export function createMiddlewares(kibana: CoreStart, storage: Storage) {
return [
dataTableLocalStorageMiddleware(storage),
userAssetTableLocalStorageMiddleware(storage),
...createTimelineMiddlewares(),
...createTimelineMiddlewares(kibana),
];
}
37 changes: 12 additions & 25 deletions x-pack/plugins/security_solution/public/common/store/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,12 @@ import type {
Middleware,
Dispatch,
PreloadedState,
CombinedState,
AnyAction,
Reducer,
} from 'redux';
import { applyMiddleware, createStore as createReduxStore } from 'redux';
import { composeWithDevTools } from 'redux-devtools-extension/developmentOnly';
import type { EnhancerOptions } from 'redux-devtools-extension';
import { createEpicMiddleware } from 'redux-observable';
import type { Observable } from 'rxjs';
import { BehaviorSubject, pluck } from 'rxjs';
import type { Storage } from '@kbn/kibana-utils-plugin/public';
Expand All @@ -33,18 +31,14 @@ import {
SERVER_APP_ID,
} from '../../../common/constants';
import { telemetryMiddleware } from '../lib/telemetry';
import { appSelectors } from './app';
import { timelineSelectors } from '../../timelines/store';
import * as timelineActions from '../../timelines/store/actions';
import type { TimelineModel } from '../../timelines/store/model';
import { inputsSelectors } from './inputs';
import type { SubPluginsInitReducer } from './reducer';
import { createInitialState, createReducer } from './reducer';
import { createRootEpic } from './epic';
import type { AppAction } from './actions';
import type { Immutable } from '../../../common/endpoint/types';
import type { State } from './types';
import type { TimelineEpicDependencies, TimelineState } from '../../timelines/store/types';
import type { TimelineState } from '../../timelines/store/types';
import type { KibanaDataView, SourcererModel, SourcererDataView } from './sourcerer/model';
import { initDataView } from './sourcerer/model';
import type { AppObservableLibs, StartedSubPlugins, StartPlugins } from '../../types';
Expand Down Expand Up @@ -255,7 +249,7 @@ const stateSanitizer = (state: State) => {
export const createStore = (
state: State,
pluginsReducer: SubPluginsInitReducer,
kibana: Observable<CoreStart>,
kibana$: Observable<CoreStart>,
storage: Storage,
additionalMiddleware?: Array<Middleware<{}, State, Dispatch<AppAction | Immutable<AppAction>>>>
): Store<State, Action> => {
Expand All @@ -272,23 +266,18 @@ export const createStore = (

const composeEnhancers = composeWithDevTools(enhancerOptions);

const middlewareDependencies: TimelineEpicDependencies<State> = {
kibana$: kibana,
selectAllTimelineQuery: inputsSelectors.globalQueryByIdSelector,
selectNotesByIdSelector: appSelectors.selectNotesByIdSelector,
timelineByIdSelector: timelineSelectors.timelineByIdSelector,
timelineTimeRangeSelector: inputsSelectors.timelineTimeRangeSelector,
};

const epicMiddleware = createEpicMiddleware<Action, Action, State, typeof middlewareDependencies>(
{
dependencies: middlewareDependencies,
}
);
// TODO: Once `createStore` does not use redux-observable, we will not need to pass a
// kibana observable anymore. Then we can remove this `any` cast and replace kibana$
// with a regular kibana instance.
// I'm not doing it in this PR, as this will have an impact on literally hundreds of test files.
// A separate PR will be created to clean this up.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const kibanaObsv = kibana$ as any;
const kibana =
'source' in kibanaObsv ? kibanaObsv.source._value.kibana : kibanaObsv._value.kibana;

const middlewareEnhancer = applyMiddleware(
...createMiddlewares(storage),
epicMiddleware,
...createMiddlewares(kibana, storage),
telemetryMiddleware,
...(additionalMiddleware ?? [])
);
Expand All @@ -299,8 +288,6 @@ export const createStore = (
composeEnhancers(middlewareEnhancer)
);

epicMiddleware.run(createRootEpic<CombinedState<State>>());

return store;
};

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 89a1d79

Please sign in to comment.