salsita / prism

React / Redux action composition made simple http://salsita.github.io/prism/
495 stars 24 forks source link

Version 4.x.x #70

Open tomkis opened 7 years ago

tomkis commented 7 years ago

Version 4.x.x

Software evolves and it's probably the right time for breaking change. There are obviously some issues and limitations in current version of redux-elm. The initial goal of this project was to blindly replicate The Elm Architecture in Redux ecosystem, there were couple major changes until we realized it would not make sense to do so, mainly because we wouldn't have the language support as solid foundation. That's why I am pretty confident that using something like redux-side-effects or redux-loop is generally not good idea in Redux.

These days, with proper architecture in Redux application, it's fairly easy to achieve very similar level of encapsulation like with Elmish architecture. The one issue still remains though and it's Component instantiation. We do have some workarounds eg. using mergeProps:

// Container:

const CounterContainer = connect(
  mapStateToProps,
  buildActionCreators({
    onIncrement: ActionTypes.INCREMENT
  }),
  (stateProps, dispatchProps, ownProps) => ({
    ...ownProps,
    ...stateProps,
    ...dispatchProps,
    onIncrement: () => dispatchProps.onIncrement(ownProps.counterId)
  })
)(Counter);

// Usage:

<CounterContainer counterId='topCounter' />
<CounterContainer counterId='bottomCounter' />

It's easy to tag all the outgoing actions by component instance ID. However, you still need to deal with tagged actions in reducers and sagas somehow and that's the tricky part.

What is wrong with current architecture?

The core problem of redux-elm is its complexness. It tries to solve all the problems in the universe and it's basically a kind of monolithic framework. Although we call the framework redux-elm you have to have installed redux, react, redux-saga and rxjs as deps. The thing is that redux-elm defines a concept of Component with its lifecycle. Sagas are being automatically mounted when componentWillMount kicks in, so we basically tightly couple the saga with React component instance. Same goes for Updaters, which mix together Sagas & Reducers.

That's why we need to deal with these kind of issues:

Can we do better?

Yes, we should totally re-think the library - from scratch:

  1. redux-elm should not be a library anymore but rather a toolset of enhancer/hoc/function...
  2. redux-elm should become a multirepo providing all the bindings independently
  3. redux-elm should get rid of concepts called "Component" and "Updater"
  4. redux-elm should be renamed to redux-prism
  5. redux-elm should not automatically register/unregister Sagas, it should be user's responsibility
  6. redux-elm should play well with react and specifically react-redux
  7. Use idiomatic redux architecture while redux-elm would be responsible only for instantiated Components

The idea is simple, instead of concept called Updater we will use plain old reducers and redux-elm will provide helper functions for unwrapping actions for nested reducers. redux-elm will provide a helper function for redux-saga to spawn isolated saga and this needs to be spawned/cancelled manually.

For your idea, please have a look at example project. Keep in mind the API will most likely change (specifically those wrappers / un-wrappers).

If you are willing to try it locally, install redux-elm via npm: npm install redux-elm@next --save

Please, share your opinion!

cc @krzysztofpniak @namjul @ccorcos @jmatsushita @stratospark

stratospark commented 7 years ago

@tomkis1 We have some key parts of our app written with redux-elm. I am all for transitioning into a simpler architecture.

I'm wondering what the consensus is about the fractal architecture described in: https://blog.javascripting.com/2016/05/21/the-problem-with-redux-and-how-to-fix-it/. This was a key article that convinced me to go with redux-elm. Do the arguments listed in that article still apply? And would be solved by a redux-elm rewrite?

ccorcos commented 7 years ago

I actually really like how hyperapp works -- its very similar: https://github.com/hyperapp/hyperapp

Dattaya commented 7 years ago

@tomkis1, thanks for the great library. I hope it's fine if post here a question: Are there helpers to deal with a situation when a user can create any amount of counters dynamically (for example by pressing a button)? Or I should build my own smarter unwrapper and handler that can distinguish and initialize if necessary Counter1/Counter2/.../CounterN_?

tomkis commented 7 years ago

Hello @Dattaya thank you for your feedback :) Easiest way is to compose all the actions with following pattern Counter.1.Increment. Therefore you can us buildUnwrapper('Counter') and deal with unwrapping dynamic action right in the handler... so you would just map unwrap manually 1.Increment into const index = 1 and action = { type: 'Incremnet'} and pass it down the corresponding reducer.

The main idea is described here https://github.com/salsita/prism/tree/v3.1.0/docs/composition/dynamic-list although it's for older version of prism (called redux-elm) the idea remains the same, we have just decided not to give user facility to do use a helper function as part of the framework, because we rather put the burden on the user so that they can implement more complex use cases by themself.

Dattaya commented 7 years ago

Thanks, your comment was very helpful! In case anyone's interested, here's how countersPairReducer for dynamically created counters might look like:

const computeIndex = ({ type = '' }) => Number(type.substr(0, type.indexOf('.')));

const unwrapAction = (index, { type = '', ...other }) => ({
  type: type.replace(`${index}.`, ''),
  ...other,
});

const initialState = {
  counters: [],
};

export default buildReducer([{
  unwrapper: buildUnwrapper('Counter'),
  handler: (state, action) => {
    const index = computeIndex(action);

    return ({
      ...state,
      counters: state.counters.map((counter, i) => i === index
        ? counterReducer(counter, unwrapAction(index, action))
        : counter
      ),
    });
  },
}, {
  unwrapper: buildUnwrapper('AddCounter'),
  handler: (state, action) => ({
    ...state,
    counters: [
      ...state.counters,
      counterInitialState,
    ],
  }),
}, {
  unwrapper: buildUnwrapper('ResetCounters'),
  handler: (state, action) => ({
    ...state,
    counters: state.counters.map(() => counterInitialState),
  }),
}], initialState);
reem commented 7 years ago

What I really loved about the redux-elm library was that it gave a way to write a react/redux application using a truly fractal architecture. It seems like your linked "proper architecture" (mastermind) does not use a fractal architecture, it has reducers, components, sagas, etc. all separated from each other rather than grouped together. This seems like a rather different way of doing things!

While I can appreciate that the monolithic nature of redux-elm@3 was restricting, I think any new approach at v4 (or prism@1) should be designed with an eye towards enabling a truly fractal architecture, as this was (to me) the entire idea behind redux-elm, and copying the elm architecture was just a way to accomplish this goal.

ccorcos commented 7 years ago

If you're trying to go the declarative, pure functional approach, I think the best way to handle side-effects is the same way you handle rendering. This is how Elm 0.17 works, and here's a little example I built that could give some inspirations: https://github.com/ccorcos/arbol