klarna / electron-redux

Use redux in the main and browser processes in electron
MIT License
743 stars 94 forks source link

Main store and sibling renderers don't get updated (alpha.7) #294

Closed Nantris closed 3 years ago

Nantris commented 3 years ago

Describe the bug

We're finding that Redux devtools says actions have occurred in our renderer, but the value in the store in the renderer does not reflect that except in the window where the value update was initiated.

The really confusing this is that even though the value in Redux's devtools indicates that it's been updated, as far as I can tell the value in the store has not actually been updated.

Our app has three windows that all come up at the same time, I'm not sure if that may be relevant.

With version one I know it was important to place the enhancer before/after other enhancers. Is it possible the issue is anything like that?

Our code looks like this:

configurestore.js


  middleware.push(sagaMiddleware);

  if (scope === 'renderer') {
    if (options.runStorage) enhancers.push(electronStorage());
    enhance
  }
  if (scope === 'main') {
    enhancers.push(require('electron-redux').mainStateSyncEnhancer());
  }

  const store = createStore(
    rootReducer,
    initialState,
    composeEnhancers(...enhancers)
  );

  storeProvider.register(store);

storeProvider.js

 const DEFAULT_STORE_OBJ = { getState: () => ({}) };

 class Store {
  constructor() {
    this.reset();
  }

  register = store => {
    this.store = store;
  };

  access = () => this.store;

  getState = () => this.store.getState();

  reset = () => {
    this.store = DEFAULT_STORE_OBJ;
  };
}

export const storeProvider = new Store();
Nantris commented 3 years ago

Important to note this occurs even without using the hack outlined in #293

Nantris commented 3 years ago

Update: This may be as simple as moving enhancers.push(applyMiddleware(...middleware)); to be before where we add the electron-redux enhancers.

Our app is basically a construction zone right now though, which is why I can only say this "may be as simple as" moving it. I think it works, just not entirely sure yet.

Nantris commented 3 years ago

Confirming that fix works.

Nantris commented 3 years ago

Mmmmm.... A lot of redux-saga code doesn't seem to fire with this configuration, not sure what to make of that.

Nantris commented 3 years ago

I've tried a lot of things to get this working, but no luck as of yet.

According to redux-saga's docs, the middleware should come after any other middleware. I'm not sure if electron-redux's enhancers are doing something related to middleware?

Any thoughts on how to accomodate redux-saga in 2.x?

https://redux-saga.js.org/docs/api/#middleware-api

Nantris commented 3 years ago

I wonder if this comment on async middleware AFTER redux-saga's middleware is relevant to the problems we're seeing? https://github.com/redux-saga/redux-saga/issues/1334#issuecomment-366675496

The current architecture is creating its own middlewares, and I'm guessing those would be async?

Nantris commented 3 years ago

Related to https://github.com/klarna/electron-redux/issues/260 I guess?

matmalkowski commented 3 years ago

@Slapbox Middleware creation is synchronous, the async part is only the store hydration -> in default, sync scenario, we will fetch the main store state with blocking ipc call (as in here), and with async scenario we will dispatch message that will than fetch the store with async messaging and than hydrate the store via a built in reducer. But middleware itself is a synchronous code for its creation, and same goes for the store enhancer

Nantris commented 3 years ago

Seems to be resolved by #285.