-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
middleware receive immutable versions of state and actions #63802
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,13 +4,12 @@ | |
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import { MiddlewareFactory } from '../../types'; | ||
import { ImmutableMiddlewareFactory } from '../../types'; | ||
import { pageIndex, pageSize, isOnHostPage, hasSelectedHost, uiQueryParams } from './selectors'; | ||
import { HostListState } from '../../types'; | ||
import { AppAction } from '../action'; | ||
|
||
export const hostMiddlewareFactory: MiddlewareFactory<HostListState> = coreStart => { | ||
return ({ getState, dispatch }) => next => async (action: AppAction) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will now be an Immutable version of AppAction. |
||
export const hostMiddlewareFactory: ImmutableMiddlewareFactory<HostListState> = coreStart => { | ||
return ({ getState, dispatch }) => next => async action => { | ||
next(action); | ||
const state = getState(); | ||
if ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,44 +4,25 @@ | |
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import { | ||
createStore, | ||
compose, | ||
applyMiddleware, | ||
Store, | ||
MiddlewareAPI, | ||
Dispatch, | ||
Middleware, | ||
} from 'redux'; | ||
import { createStore, compose, applyMiddleware, Store } from 'redux'; | ||
import { CoreStart } from 'kibana/public'; | ||
import { appReducer } from './reducer'; | ||
import { alertMiddlewareFactory } from './alerts/middleware'; | ||
import { hostMiddlewareFactory } from './hosts'; | ||
import { policyListMiddlewareFactory } from './policy_list'; | ||
import { policyDetailsMiddlewareFactory } from './policy_details'; | ||
import { GlobalState, MiddlewareFactory } from '../types'; | ||
import { AppAction } from './action'; | ||
import { ImmutableMiddlewareFactory, SubstateMiddlewareFactory } from '../types'; | ||
import { EndpointPluginStartDependencies } from '../../../plugin'; | ||
|
||
const composeWithReduxDevTools = (window as any).__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ | ||
? (window as any).__REDUX_DEVTOOLS_EXTENSION_COMPOSE__({ name: 'EndpointApp' }) | ||
: compose; | ||
|
||
export type Selector<S, R> = (state: S) => R; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. moved type to |
||
|
||
/** | ||
* Wrap Redux Middleware and adjust 'getState()' to return the namespace from 'GlobalState that applies to the given Middleware concern. | ||
* | ||
* @param selector | ||
* @param middleware | ||
*/ | ||
export const substateMiddlewareFactory = <Substate>( | ||
selector: Selector<GlobalState, Substate>, | ||
middleware: Middleware<{}, Substate, Dispatch<AppAction>> | ||
): Middleware<{}, GlobalState, Dispatch<AppAction>> => { | ||
export const substateMiddlewareFactory: SubstateMiddlewareFactory = (selector, middleware) => { | ||
return api => { | ||
const substateAPI: MiddlewareAPI<Dispatch<AppAction>, Substate> = { | ||
const substateAPI = { | ||
...api, | ||
// Return just the substate instead of global state. | ||
getState() { | ||
return selector(api.getState()); | ||
}, | ||
|
@@ -66,7 +47,7 @@ export const appStoreFactory: (middlewareDeps?: { | |
* Any additional Redux Middlewares | ||
* (should only be used for testing - example: to inject the action spy middleware) | ||
*/ | ||
additionalMiddleware?: Array<ReturnType<MiddlewareFactory>>; | ||
additionalMiddleware?: Array<ReturnType<ImmutableMiddlewareFactory>>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @paul-tavares objections to replacing this with:
I don't think code outside of the store should dictating what middleware are used, how they are constructed, or what order they are applied in. Theoretical example of what i'm concerned about: Assume we learn that the |
||
}) => Store = middlewareDeps => { | ||
let middleware; | ||
if (middlewareDeps) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will now be an
Immutable
version of AppAction.