Skip to content
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

Discussion: renderer process stores are not main process store mere proxies #64

Open
eric-burel opened this issue Oct 19, 2017 · 4 comments

Comments

@eric-burel
Copy link

eric-burel commented Oct 19, 2017

Hi,
I'd just like to raise this issue because it has been bugging me for a while. In the readme, you declare that the main process becomes the Single Source of Truth, and renderer mere proxies.

I am not sure about that actually. That would be true if this lib was based on state diffing, but here, as far as I understand, you use action replaying as a sync mechanism.

This is different since your synchronization is actually a loose synchronization. In turn, it has 2 consequences:

  • you can't guarantee the actual synchronization between stores when actions are missed (nobody cares because you should not miss actions in a normal electron use case but still, interprocess commmunication may theoritically fail)

  • however, thanks to its symetry, your model is far more scalable than a simple main/renderer process. In a big appliction, this allow to split state between the renderer (that purely handles UI) and the main (that handles data), or even to create store for background processes. The only shared part are data related reducers of the main, so that all stores works on the same data.

To sum it up:

  • main has its own sagas/observable/thunks/whatever, its own reducers, and shared reducers (e.g to handle loaded data) that correspond to the loosely synched state slice.
  • rendrer has its own sagas/observable/thunks, its own reducers, and the reducers it shares with the main.

So the main is not a single source of truth, but simply a redux-aware process that you usually use to handle async data loading.

Better part: why use the main? actually we could use a third process, so that both the main process and the renderer process are free to go.

  • renderer 1 has its store + relevant reducers, shared and not shared
  • renderer N same
  • main same (actually in your model the main is just a special process)
  • any bg process same

This way you can avoid to perform time consuming async action may lead to performance issue if you put them in the main.

I think that if this lib is to evolve, it could be in a direction that helps to spawn stores that communicate altogether, so that we can create a distributed redux-everywhere architecture, a kind of lightweight redux message queue.
Also, using this logic, I think that maybe actions should not forwarded by default, we should have to add an additional meta to tel that its a forwarded action, and to whome it should go (all, main, or whatever named store).

We would not need aliasedAction anymore also, because if only the ith process have the relevant reducers/sagas/whatever, it will be the only one to process it.

@RXminuS
Copy link

RXminuS commented Nov 9, 2017

Yeah this confused me at first as well especially since that's what https://github.com/samiskin/redux-electron-store does. I actually prefer this method because it means I can still use Immutable.js for the actual state in my renderers which helps with ensuring performance.

@hardchor
Copy link
Contributor

hardchor commented Nov 24, 2017

Hey @eric-burel,

Thanks so much for your thoughts, and apologies for replying so late.

you can't guarantee the actual synchronization between stores when actions are missed (nobody cares because you should not miss actions in a normal electron use case but still, interprocess commmunication may theoritically fail)

You are absolutely correct. In addition, the initial architecture of electron-redux did not accommodate for local actions (which were added later), getting us somewhat further away from the single-source-of-truth statement.

I am not sure about that actually. That would be true if this lib was based on state diffing, but here, as far as I understand, you use action replaying as a sync mechanism.

Even with state-diffing, you could lose information through IPC. Last time I checked, https://github.com/samiskin/redux-electron-store uses some sort of state synchronisation and state filtering (as opposed to action replaying), which is why I created this library.

So the main is not a single source of truth, but simply a redux-aware process that you usually use to handle async data loading.

That is also correct, and should probably get updated in the docs.

Better part: why use the main? actually we could use a third process, so that both the main process and the renderer process are free to go.

The main process being the central point for all (except for local) actions fits in very nicely with the electron architecture. Renderer processes cannot talk to each other directly, so the main process' store needs to be the central hub for all communication.

This way you can avoid to perform time consuming async action may lead to performance issue if you put them in the main.

In all honesty, I can't think of a scenario where an action(-producer) or reducer would block the main thread long enough for it to actually matter. Due to the nature of the event loop, async actions are non-blocking, and reducers are simple stateless functions. Adding an extra process (to become what is now the main store) would - imho - add more overhead than the performance penalties you've mentioned. However, if this becomes a problem in your specific use-case, this library should not stop you from working around this, which is leading onto the next point...

We would not need aliasedAction anymore also, because if only the ith process have the relevant reducers/sagas/whatever, it will be the only one to process it.

Bingo! aliasedAction was always a bit of a pain point, and having named stores that you can freely forward to would be a way better alternative. This would, however, require quite an API change. Maybe we should open a new issue with a 2.0 roadmap.

Long story short:

I'm more than happy to evolve the library, and this is a great starting point. I'd still prefer for the main store to take up this special role as an "action fan" (or at least some sort of other util that could be decoupled from the store, but lives on the main thread), but I'm open for a discussion around this.

@eric-burel
Copy link
Author

eric-burel commented Jan 5, 2022

Soooo I finally have a chance to try this:

1. Make action local as a default

Goal: adding electron-redux does nothing, until you explicitely start sharing actions. Gives a more intuitive control over the setup.

  • I modified the redux middleware to send only actions with scope 'shared': as a default, actions are local (normal redux behaviour), and only actions marked as shared will reach the main process
  • I've created a createSharedAction helper

This first step leads to a cleaner architecture.

2. Actions are not ordered

Goal: fix the issue that breaks the guarantee of actions being ordered, since they have to go through async process to reach the main

Second step was fixing the "async" issue: currently when actions are dispatched to the main process, they are also eliminated from the redux pipeline. Then the main process sends it back to the emitter with "local" scope.

  • What I've done instead: actions are marked with the current web content id. They are also sent to the main process. Then the main process sends the action back to all web contents EXCEPT the one that emitted the event in the first place.

The result is that shared actions behaves like "normal" actions (states update synchronously), their order is respected again.

Now that we have invoke method, starting with Electron 7, I also adopt the following pattern:

  • Use electron-redux for global data loading, for instance opening a project, getting the current user: data that will be reused in many places. I have redux-saga in the main process, with main-only sagas.
  • Keep normal, local redux actions for local actions
  • Use ipc as usual for more "local" actions

3. Create new renderer process with the same state as another one (or whatever state we want)

Goal: allow decoupling the main process state from renderers state

Final step: I'd like to be able to create new windows by duplicating the current window state. I think at the moment, when creating a new renderer, I use the main store => this forces the main store to be in sync with the renderer store. I'll change that, so that the main process store can have different data from the renderer store => this avoids duplication that can be problematic when the app grows big.

  • I've modified getInitialStateRenderer to get the state from the arguments passed to the renderer process during its creation
  • I use additionnalArguments to pass the serialized state of the current renderer, so that the new renderer has the same state
  • More broadly, this let's you initialize the state as you wish: pass it from another renderer process, keep using the main process state... whatever you want, you simply need to pass the right state to new BrowserWindow additional arguments.

Code samples:

1.

For the action creator, I add "meta.scope.shared" to shared action + I mark them with the webcontent id

export function createSharedAction(
    // NOTE: Parameters doesn't work as expected with overloaded methods
    ...creatorArgs: /*Parameters<typeof createAction>*/ any
) {
    // @ts-ignore
    const baseActionCreator = createAction(...creatorArgs);
    function sharedActionCreator(...args) {
        const action = baseActionCreator(...args) as any;
        if (!action.meta) action.meta = {};
        action.meta.scope = 'shared';
        // in renderer process, mark with current id
        // /!\ this is very important, it avoid duplicating the event in the
        // emitter process
        if (typeof window !== 'undefined') {
            const electron = require('renderer/api/electron').default;
            action.meta.webContentsId = electron.getCurrentWebContentsId();
        }
        return action;
    }
    return sharedActionCreator;
}

Then in the renderer middleware. The change is that I call "next(action)": shared actions behaves like normal redux action from the renderer standpoint.

        // send to main process
        ipcRenderer.send('redux-action', action);
        // NOTE: it's important that the main process
        // do not send the action again to the emitter renderer
        // otherwise it would duplicate actions
        return next(action);

2.

And finally the main process. Here the difference is that I filter webContents, to avoid duplicating actions:

 // filter out the emitter renderer (important to avoid duplicate actions)
    let allWebContents = webContents.getAllWebContents();
    if (action?.meta?.webContentsId) {
        allWebContents = allWebContents.filter(
            (w) => w.id !== action.meta.webContentsId
        );
    }

    allWebContents.forEach((contents) => {
        contents.send('redux-action', rendererAction);
    });

    return next(action);

3.

When I create a new renderer process, I do this to pass the state explicitely:

        const newWindow = new BrowserWindow({
            show: false,
            width: 1024,
            height: 728,
            icon: iconPath,
            webPreferences: {
                // NOTE: this configuration is not secure! but removing nodeIntegration means reworking "app/renderer/api" files so they
                // don't rely directly on ipcRenderer etc. (including for 3rd party packages)
                nodeIntegration: true,
                nodeIntegrationInWorker: true,
                enableRemoteModule: true,

                preload: path.join(__dirname, './preload.js'),
                contextIsolation: false, // true // will be the default in Electron > 12
                // We also pass the initial state
                additionalArguments: [
                    `--redux-state=${initialReduxStateSerialized}`,
                ],
            },
        });

Then in my preload-source.js:

// Get initial redux state from args
// /!\ state must be serializable/deserializable
let initialReduxStateSerialized: string = '{}';
const reduxArg = window.process.argv.find((arg) =>
    arg.startsWith('--redux-state=')
);
if (reduxArg) {
    initialReduxStateSerialized = reduxArg.slice('--redux-state='.length);
}

And eventually:

export function getInitialStateRenderer() {
    try {
        const initialReduxState = JSON.parse(
            electron.initialReduxStateSerialized
        );
        return initialReduxState;
    } catch (err) {
        console.error(err);
        console.error('Serialized state', electron.initialReduxStateSerialized);
        throw new Error('Non serializable Redux state passed to new window');
    }

This comment is bit rushed out because I don't think there are enough users of Electron + Redux to make this a full Medium article or a full PR, but I'll be happy to answer any question if someone encounter the same issues.

For the record, this architeture would also work in a normal web app, with a worker playing the role of the main process.

@eric-burel
Copy link
Author

eric-burel commented Jan 26, 2022

One last thing that gives me a hard time is reloading the page. Since it erases all state in the renderer, it creates some discrepencies with the main state. Maybe I should setup a temporary system to memorize the renderer current state (eg in smth persistent like localStorage) and reload it from there.

Update: after more work on this, I think I need to totally decouple main and renderer state, and build the renderer so it can "rebuild" its state on reload by fetching relevant data

  • The main process state acts as a cache to avoid calling my Python backend. I may even stop using it altogether, I think it adds a ton of complexity for nothing. It was necessary when calling the main process was complicated, but not needed anymore now that I've found pattern to share states between renderers differently.
  • The renderer process acts as a cache to avoid calling my Electron backend

While right now, the main acts as a copy of the renderer process, which is not the right way to use it as it is difficult to guarantee synchronicity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants