klarna / electron-redux

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

Restarting Redux environment without triggering preventDoubleInitialization? #293

Open Nantris opened 3 years ago

Nantris commented 3 years ago

We just upgraded to the 2.0.0-alpha.9 and the new setup looks amazingly simple!

I've not gotten things working quite yet, and maybe won't be able to quite yet, but one thing I noticed is preventDoubleInitialization

https://github.com/klarna/electron-redux/blob/alpha/src/utils/misc.ts#L5

With electron-redux@1.x we had built in a custom functionality to restart the redux environment to avoid having to restart our entire project.

The core of our solution was just to use something like below to unregister all channels before we re-create our app's windows from scratch:

    channels.forEach(channel => {
      ipcMain.removeAllListeners(channel);
    });

Is there a way to achieve this with the new architecture? If not, might I suggest that previouslyInitialized (https://github.com/klarna/electron-redux/blob/alpha/src/utils/misc.ts#L5) be made into a global instead so users can manually reset it if needed?

Nantris commented 3 years ago

Turns out this is still very manageable although the solution forces us into using inline requires rather than import at the top of our file. It's a minor gripe, but it would be great to have this use case built into the architecture.

To facilitate restarting the Redux environment:

configurestore.js Use inline requires to work around the previouslyInitialized variable.

  if (scope === 'renderer') {
    enhancers.push(require('electron-redux').rendererStateSyncEnhancer());
  } else if (scope === 'main') {
    enhancers.push(require('electron-redux').mainStateSyncEnhancer()); 
  }

main.js Remove the IPC handler to prevent an Electron error about double registered handlers

  // destroy all your windows and then...
  ipcMain.removeHandler('electron-redux.INIT_STATE_ASYNC');
  ipcMain.removeAllListeners('electron-redux.INIT_STATE');
  ipcMain.removeAllListeners('electron-redux.ACTION');
matmalkowski commented 3 years ago

@Slapbox I think I saw a notification yesterday at night about a comment, but I don't see that comment here anymore 🤔

In terms of supporting this - I don't see any reason why we could not extend API to support this in the future - but we would need to understand how we plan to solve that and so on. If you are interested and have some ideas in mind, let me know and we could brainstorm it together

Nantris commented 3 years ago

Indeed, I commented but I found the issue. But I'm not really sure why removing some code I'd committed fixed it.

Would there be any downside to making previouslyInitialized a global so that users could manually reset it? Or otherwise what would you think of a function like, resetPreviouslyInitialized() to set it back to false?

The solution we have right now involves using inline requires which are a bit more difficult to reason about I feel.