-
Notifications
You must be signed in to change notification settings - Fork 24
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
Version 4.x.x #70
Comments
@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? |
I actually really like how hyperapp works -- its very similar: https://github.com/hyperapp/hyperapp |
@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_? |
Hello @Dattaya thank you for your feedback :) Easiest way is to compose all the actions with following pattern 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 |
Thanks, your comment was very helpful! In case anyone's interested, here's how 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); |
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. |
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 |
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 likeredux-side-effects
orredux-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
: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 frameworkredux-elm
you have to have installedredux
,react
,redux-saga
andrxjs
as deps. The thing is thatredux-elm
defines a concept of Component with its lifecycle. Sagas are being automatically mounted whencomponentWillMount
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:
redux-elm
should not be a library anymore but rather a toolset of enhancer/hoc/function...redux-elm
should become a multirepo providing all the bindings independentlyredux-elm
should get rid of concepts called "Component" and "Updater"redux-elm
should be renamed toredux-prism
redux-elm
should not automatically register/unregister Sagas, it should be user's responsibilityredux-elm
should play well withreact
and specificallyreact-redux
The idea is simple, instead of concept called
Updater
we will use plain old reducers andredux-elm
will provide helper functions for unwrapping actions for nested reducers.redux-elm
will provide a helper function forredux-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
The text was updated successfully, but these errors were encountered: