logux / redux

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

Using with redux-devtools-extension #8

Closed namjul closed 6 years ago

namjul commented 6 years ago

I am trying to use it with redux-devtools-extension and i am getting the following error: Uncaught TypeError: a.subscribe is not a function

I then tried adding redux's subsribe method here: https://github.com/logux/logux-redux/blob/master/create-logux-creator.js#L127

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

This solves the issue but nowredux-devtools-extension does not seam to find any store. Also using the normal store.dispatch throws an error. Has anybody managed to combine logux-redux with redux-devtools-extension?

ai commented 6 years ago

Nope, I didn't try it.

There is no really sense to use Redux DevTools. Logux had own time travel algorithm. Action undo will not undo actions on the server.

It is better to create special Logux DevTools since Logux will give more feature for DevTools (like hot reload not only for the client, but also for server).

Of course, it will be nice to fix this compatibility issues, but it is low priority.

namjul commented 6 years ago

Thx Andrey, i understand. I mainly use it to check current state. I tried redux-logger but it only works with normal dispatch. Do you know another approach to check next state after dispatch?

ai commented 6 years ago

Hm. Interesting question. Maybe I need to add extra event to logux-redux to get state update after action.

Because of time travel (it is async since log could be loaded from IndexedDB), Redux DevTools will not work anyway.

Thanks for the issue. I will try to bring some solution on this weekend.

namjul commented 6 years ago

I see. Looking forward to your solution.

vkalinichev commented 6 years ago

This issue points to fragility of current connecting solution: any enhancer can strip unoriginal methods from the redux store API. In large projects it can be a real headache.

@ai, maybe should look reconsider the way how logux is connecting to the store?

It may look like this:

import { createStore } from 'redux'
import { connectedLogux, createLoguxEnhancer } from 'logux-redux'

const enhanceByLogux = createLoguxEnhancer({
  subprotocol: '1.0.0',
  server: 'wss://localhost:1337',
  userId: 10
})

const store = createStore(
  connectedLogux(reducer),
  preloadedState,
  enhanceByLogux(enhancer)
)

I think it's more natural way to enhance redux store and avoid such problems

vkalinichev commented 6 years ago

Ah sorry, in this case it shouldn't help: store enhanced by logux will still be broken by redux-devtools-extension

vkalinichev commented 6 years ago

But as it described in redux-devtools-extensions docs it can be fixed like this:

const store = createStore(
  connectedLogux(reducer),
  preloadedState,
  compose(
    window.__REDUX_DEVTOOLS_EXTENSION__ ? window.__REDUX_DEVTOOLS_EXTENSION__() : noop => noop,
    enhanceByLogux(enhancer)
  )
)
ai commented 6 years ago

Hm. Interesting idea.

Logux doesn't enhance store, but replaces it completely. Is it a problem for this case? Or we just need to call prevState.dispatch to make it works?

namjul commented 6 years ago

I wondered once why logux-redux isnt a store enhancer. Regardless of this issue and if possible, being a store enhancer would fit "more" into the redux design, i think.

If this would also solve the issue i currently don't know.

ai commented 6 years ago

Logux need to replace the store in time travel. And there is no official Redux API for it.

But maybe there are some hacks to do it. How you think we can enhance it?

We need to add extra properties to the store and add time travel.

namjul commented 6 years ago

I see, maybe its not possible with the official Redux API only. I am still new to logux code so atm i wouldn't know how.

ai commented 6 years ago

@namjul maybe you want to create to change create-logux-creator.js to support Redux enhancer API?

It will be the best way to learn Logux internals and should not take a lot of the time. If you will have any questions, feel free to ask.

Do you want to try?

I will be able to try it only a few weeks later (preparing a talk about Logux).

namjul commented 6 years ago

Thx for the encouragement. I like the idea and i will give it a shot.

ai commented 6 years ago

Hi. Thanks for waiting. I added change event to the store 80f5cb7

  1. You need to move to GitHub master of Logux Redux, Logux Client and Logux Server. It will be unreleased 0.3 version with already big changes.
  2. Then you can manually subscribe for store change event:
store.on('change', (state, prevState, action, meta) => {
  console.log(state, prevState, action, meta)
})