-
Notifications
You must be signed in to change notification settings - Fork 400
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
Added appEpics #1644
Added appEpics #1644
Conversation
… If Epic's name is the same the one in plugins this will override plugin's one
@@ -26,7 +26,7 @@ const {createEpicMiddleware} = require('redux-observable'); | |||
const SecurityUtils = require('../utils/SecurityUtils'); | |||
const ListenerEnhancer = require('@carnesen/redux-add-action-listener-enhancer').default; | |||
|
|||
module.exports = (initialState = {defaultState: {}, mobile: {}}, appReducers = {}, plugins, storeOpts) => { | |||
module.exports = (initialState = {defaultState: {}, mobile: {}}, appReducers = {}, appEpics = {}, plugins, storeOpts) => { |
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.
New arguments at the end, for compatibility reasons, please
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 signature is better. This doesn't break the binding with StandardApp or other apps. I should put it in that place instead of the bottom.
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.
Binding appEpics wouldn't be possible
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.
Please remove
@@ -72,5 +73,11 @@ describe('PluginsUtils', () => { | |||
expect(desc1.items[0].cfg).toExist(); | |||
|
|||
}); | |||
it('combineEpics', () => { | |||
const plugins = {MapSearchPlugin: MapSearchPlugin}; | |||
const appEpics = {appEpics: (actions$) => actions$}; |
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 sounds like an infinite loop. Is not executed but it should. Please remove it.
as discussed we removed this because is the correct usage
This pull lets pass an object with app specific Epics
If Epic's name, in appEpics, is the same the one in plugins this will override plugin's one