logux / redux

Redux compatible API for Logux
https://logux.org/
MIT License
127 stars 16 forks source link

Redux DevTools support #10

Closed dangreen closed 5 years ago

dangreen commented 5 years ago

Why enhancer is applied like this:

https://github.com/logux/redux/blob/master/create-logux-creator.js#L148

?

ai commented 5 years ago

@dangreen what is a problem with this method? (maybe I misunderstood the question)

dangreen commented 5 years ago

@ai Enhancers like redux-devtools doesn't work with this method: 1) It wraps basic redux methods (getStore, dispatch, subscribe, replaceReducer) (ok, we can add other methods to your code https://github.com/logux/redux/blob/master/create-logux-creator.js#L149) 2) But it also need to wrap reducer, with your method this is not possible.

ai commented 5 years ago

Redux DevTools anyway will not work with Logux, because Logux has own actions log and own time travel.

dangreen commented 5 years ago

@ai If pass enhancer directly to createStore, redux-devtools is working, maybe not fully, but it shows actions log and state history.

ai commented 5 years ago

Even actions log will be wrong, because Redux DevTools doesn't care about Logux’s actions created time (in Redux new action goes to the end of the log, in Logux you can receive old action from the server to insert it in the middle of the log).

Same with state history. Because of Logux’s time travel it could be wrong.

dangreen commented 5 years ago

Ok, let's forget redux-devtools, how about other enhancers, which need to wrap redux methods and reducer? We are creating redux store, and we whant full comparability with all enhancers/middlewares.

ai commented 5 years ago

Yeap, we can pass smog methods to enhancers. What other methods we should add?

dangreen commented 5 years ago

We can do stuff like this

var middlewared = enhancer(function () {
    return {
        getState: store.getState,
        subscribe: store.subscribe,
        replaceReducer: store.replaceReducer,
        dispatch: dispatch
    }
})(reducer, preloadedState)
store.getState = middlewared.getState
store.subscribe = middlewared.subscribe
store.replaceReducer = middlewared.replaceReducer
store.dispatch = middlewared.dispatch

But why not simply

var store = createStore(hackReducer(reducer), preloadedState, enhancer)

?

ai commented 5 years ago

Do you want to send PR?

dangreen commented 5 years ago

Yes I want, but with second variant.

ai commented 5 years ago

Sure, let's try it. Tests will fail of I forgot something.

dangreen commented 5 years ago

All tests were passed.