klarna / electron-redux

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

Dealing with calling native libs from aliased actions #92

Open paulius005 opened 6 years ago

paulius005 commented 6 years ago

createAliasedAction is there so we can call action creators, or promises to run only on the main process. However, one still has to import that function into the shared actions file even though it only runs on the main process.

This can be problematic in the case that the thing that's supposed to only run on the main process contains native libraries, which if imported into the shared action file will get compiled with the renderer code.

What is the proposed way of getting around this?

Here is a video explanation as well: https://www.useloom.com/share/32eebc9d4e4a43c5b3f57da3fbbe44bc

paulius005 commented 6 years ago

Looking at: triggerAlias.js and createAliasedAction.js

We just have a dynamically created alias registry

I have worked with react-chrome-redux and it's the same concept but has a fixed manually created alias registry https://github.com/tshaddix/react-chrome-redux#3-optional-implement-actions-whose-logic-only-happens-in-the-background-script-we-call-them-aliases

I think a simple change like this to support manual additions to the registry would be able to solve this problem

@hardchor any interest in adding this type of functionality to electron-redux?

hardchor commented 4 years ago

Hey @paulius005 (and @percyhanna). Time electron-redux receives some much-needed love. I'm sure you've long since moved on, but I'd still like to pick this up (and be it for future readers / contributors).

Code separation is definitely an issue when it comes to aliased actions. I was never really happy with the implementation, and fixing this might require a completely different approach (I'm really just throwing around empty phrases - I have no idea how right now).

andreasdj commented 4 years ago

Hi @hardchor, we have an issue related to this. The triggerAlias method uses assert to validate that a valid trigger alias has been provided.

When using webpack with target electron-preload or electron-renderer and a locked down renderer process (i.e. without node integration) we get an error complaining that the assert module doesn't exist. It doesn't get bundled since assert is a nodejs module but since we don't want to expose node modules it's not available within the renderer process.

Do you have a recommended way to get around this. A simple update in the code would be to remove the dependency on the assert module and just throw an error if the current expressions inside the current asserts are false.

paulius005 commented 4 years ago

If this helps anyone, this is the code that we ended up writing to get around this:

import log from 'electron-log';

import aliases from '../aliases';

/*
 *
 * This is a modification of electron-redux's triggerAlias middleware
 * this will be resolved once this gets figured out:
 * https://github.com/hardchor/electron-redux/issues/92
 * We cannot keep our creators in the shared directory
 * because even if we use `createAliasedAction` we still end up
 * having to import them to the renderer script which ends up breaking compilation
 * or even causing weird side effects because we end up importing native modules
 * into front end code
 *
 */

const ALIASED = 'ALIASED';

export default store => next => action => {
  if (action.type === ALIASED) {
    if (!action.meta || !action.meta.trigger) {
      return log.error('No trigger defined');
    }

    const alias = aliases[action.meta.trigger];

    if (!alias) {
      return log.error(`Trigger alias ${action.meta.trigger} not found`);
    }

    const args = action.payload || [];
    const payload = alias(...args);

    // dispatch so we dont skip any middleware
    const dispatch = store.dispatch({ type: action.meta.trigger, payload });

    // need to check for promise, not all alias functions return promises, try/catch does not capture errors
    if (dispatch && dispatch.catch) {
      return dispatch.catch(err => {
        log.error('Error in aliased action, action was consumed anyway.', err);
      });
    }

    return dispatch;
  }

  return next(action);
};

Essentially we use the concept of aliases from webext-redux to separate out the concerns of the renderer and the main process