-
Notifications
You must be signed in to change notification settings - Fork 7
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
Questionning fractal architectures: adding some new requirements #1
Comments
Disclaimer : I'm not a Elm expert Starting from your needs, we can start building a new component (called RandomGifCounter) which will contain a RandomGif, a PairOfRandomGif, a PairOfPairOfRandomGif and a Counter : // VIEW
const view = ({ model, dispatch, ...props }) =>
<div className="random-gif-counter" {...props}>
<randomGif.view
model={model.randomGif}
dispatch={(action) => dispatch(actions.modifyRandomGif(action))}
/>
<pairOfRandomGif.view
model={model.pairOfRandomGif}
dispatch={(action) => dispatch(actions.modifyPairOfRandomGif(action))}
/>
<pairOfPairOfRandomGif.view
model={model.pairOfPairOfRandomGif}
dispatch={(action) => dispatch(actions.modifyPairOfPairOfRandomGif(action))}
/>
<counter.view
model={model.counter}
dispatch={(action) => dispatch(actions.modifyCounter(action))}
/>
</div> Just by seeing this, we can already assume the component will have at least 4 actions to forward the higher-order actions to its children : // ACTIONS
const MODIFY_RANDOM_GIF = "MODIFY_RANDOM_GIF"
const MODIFY_PAIR_OF_RANDOM_GIF = "MODIFY_PAIR_OF_RANDOM_GIF"
const MODIFY_PAIR_OF_PAIR_OF_RANDOM_GIF = "MODIFY_PAIR_OF_PAIR_OF_RANDOM_GIF"
const MODIFY_COUNTER = "MODIFY_COUNTER"
const modifyRandomGif = (action) =>
({ type: MODIFY_RANDOM_GIF, payload: { action } })
const modifyPairOfRandomGif = (action) =>
({ type: MODIFY_PAIR_OF_RANDOM_GIF, payload: { action } })
const modifyPairOfPairOfRandomGif = (action) =>
({ type: MODIFY_PAIR_OF_PAIR_OF_RANDOM_GIF, payload: { action } })
const modifyCounter = (action) =>
({ type: MODIFY_COUNTER, payload: { action } }) Now the fun part, we have to increment the Counter component each time one of the children component is modified, but only when the modification intended is to generate a new gif. The workflow to update the Counter will be like this :
As we love predictability, we'll assume that an action containing a "higher-order action" is of shape : Let's produce some code : // HELPERS
const findHOA = path(["payload", "action"])
const findDeepActionOfType = (action, actionType) => {
if (isNil(action)) {
return undefined
}
if (equals(actionType)(action.type)) {
return action
}
return findDeepActionOfType(findHOA(action), actionType)
}
// MODEL
const initialModel = {
randomGif: randomGif.initialModel,
pairOfRandomGif: pairOfRandomGif.initialModel,
pairOfPairOfRandomGif: pairOfPairOfRandomGif.initialModel
counter: counter.initialModel
}
// UPDATE
const update = (model = initialModel, action) => {
const notifyCounter = (action) => {
const newGifAction = findDeepActionOfType(action, randomGif.actionsTypes.NEW_GIF)
return newGifAction
? Effects.constant(modifyCounter, counter.actions.increment())
: []
}
switch (action.type) {
case MODIFY_RANDOM_GIF:
const {
model: rgModel,
effect: rgEffect
} = randomGif.update(model.randomGif, findHOA(action))
return loop(
{ ...model, randomGif: rgModel },
Effects.batch(concat(
[Effects.lift(rgEffect, modifyRandomGif)],
notifyCounter(action)
))
)
case MODIFY_PAIR_OF_RANDOM_GIF:
const {
model: porgModel,
effect: porgEffect
} = pairOfRandomGif.update(model.pairOfPairOfRandomGif, findHOA(action))
return loop(
{ ...model, pairOfRandomGif: porgModel },
Effects.batch(concat(
[Effects.lift(porgEffect, modifyPairOfRandomGif)],
notifyCounter(action)
))
)
case MODIFY_PAIR_OF_PAIR_OF_RANDOM_GIF:
const {
model: poporgModel,
effect: poporgEffect
} = pairOfPairOfRandomGif.update(model.pairOfPairOfRandomGif, findHOA(action))
return loop(
{ ...model, pairOfPairOfRandomGif: poporgModel },
Effects.batch(concat(
[Effects.lift(poporgEffect, modifyPairOfPairOfRandomGif)],
notifyCounter(action)
))
)
case MODIFY_COUNTER:
const {
model: cModel,
effect: cEffect
} = counter.update(model.counter, findHOA(action))
return loop(
{ ...model, counter: cModel },
Effects.lift(cEffect, modifyCounter)
)
default:
return loop(
model,
Effects.none()
)
}
} Conclusion
The children are totally unaware of each other, this is the role of the parent component to orchestrate the action dispatching dance, and to modify its children accordingly. 👍 I put the code in this gist : https://gist.github.com/jgoux/ecbecad888b9e481a7e4 |
@jgoux I was sure someone would come up with this solution :) However I think there is a significant problem: your solution could almost work for Javascript but does not work for Elm :) (as far as I know) const findDeepActionOfType = (action, actionType) => {
if (isNil(action)) {
return undefined
}
if (equals(actionType)(action.type)) {
return action
}
return findDeepActionOfType(findHOA(action), actionType)
} How do you build such a function with a statically typed language like Elm? Remember that So what I'm trying to say is that your proposed solution for this Elm architecture problem does not even work with Elm but only with unsafe languages like Javascript. Now taking the case of Javascript, it might be a good idea, until you start to have collisions between action names. Now imagine I introduce a new widget, totally decouped from RandomGif. Unfortunatly, it is also related to gifs manipulation, and someone add a This was a nice try however and is probably applicable in most javascript projects :) |
This is a problem, but this exact problem is even bigger in non fractal architectures. In non fractal every action is potentially in collision. In fractal, only ones that are handled with proposed solution. |
Thank you all for contributing the idea. Some valuable code highlight here: newGifCounterHOR: import _ from 'lodash';
import {loop, Effects} from '@jarvisaoieong/redux-loop';
import {newGifCount} from './newGifCounterActions';
import {NEW_GIF} from 'modules/randomGif/randomGifActions';
export default (path) => (reducer) => (state, action) => {
const {model, effect} = reducer(state, action);
// something happen in passed reducer
if (model !== state || !_.isEqual(effect, Effects.none())) {
if (_.get(action, path) === NEW_GIF) {
return loop(
model
,
Effects.batch([
effect,
Effects.constant(newGifCount()),
])
);
}
}
return loop(model, effect);
} root reducer: export default combineReducers({
counter,
counterPair,
counterList,
counterFancy,
randomGif: newGifCounterHOR('type')(randomGif),
randomGifPair: newGifCounterHOR('action.type')(randomGifPair),
randomGifList: newGifCounterHOR('action.type')(randomGifList),
newGifCounter,
}); |
would it be better that the work of count the new gif should be considered as an effect of an new gif manager? all others (including reducers and components) nerver should know the defail of effect, but just take the return value of effect. |
This could be avoided by introducing namespaces in the actions names, you may have something like I think it covers the majority of the cases in JS, but I have no idea about the typed solution. ^_^ |
@jgoux yes about naming conflicts is definitively manageable with plain JS, but as it does not work with static typing it's kind of hard to apply the same to Elm, or even Flow and Typescript. @jarvisaoieong @uxnow you are both talking about the same solution right? I like your proposals better as it avoids coupling components together and can also work in Elm or Flow. Somehow this is related to my initial Elm proposal of having 2 mailboxes (one for nested actions, ie local state, and one for flat actions (ie business events that the whole app can listen to). I guess somehow having 1 mailbox for flat actions, and one for "business effects" is somehow similar in the end... |
Hey, I put together a simple Elm implementation of this example with the 2 mailboxes approach, the issue with this as @jgoux pointed out is that the sub (child) components needs to know about the global mailbox and actions (if we want to stay typed, if they just sends strings as actions than any string address is acceptable). Also in this case I am counting counters not the actions, but from the implementation perspective it is irrelevant. You can see the source here: https://github.com/gdotdesign/global-actions-example |
Thanks a lot for this example @gdotdesign ! 😄 |
@gdotdesign great! will have to check that! Yes this is true that using a global mailbox like you did creates coupling. I think the deep components should have a single mailbox and application-coupled components should have both. But this is very similar to what @jarvisaoieong did with What bothers me a bit is that you still have to wrap/couple it in many places, in an error-prone way, and could easily forget one place. |
Actually I just remembers that Elms type system allows for decoupling you just need to pass the action type and the action down the three, so using extensible records (http://elm-lang.org/docs/records) the I'll update the example in the morning, and post some explanation. |
These extensible records look like traits or mixins no? I'm not sure it will be any useful at all to solve the problem but waiting to see :) |
So I updated the example by making the -- Model
type alias Model a =
{ counter: Int
, clickAddress : Signal.Address a
, clickAction : a
}
-- In Main
{ counter1 = Counter.init global.address Global.Increment
, counter2 = Counter.init global.address Global.Decrement This way the actual component is decoupled from the the You can check out the diff here gdotdesign/global-actions-example@c55ef50 ExplanationThis is essentially is the same as Elms You can check out https://github.com/gdotdesign/elm-ui/blob/master/source/Ui/Tagger.elm for a more real life example of a typed component, and this blog post that explains it (I hope) https://medium.com/@gdotdesign/elm-ui-making-the-ui-tagger-component-24ca787596a0. |
Using redux-elm there are two options:
Working example: const incrementCounter = model => model + (model > 10 ? 2 : 1);
// 1) Plain Old Elmish Solution
const counterReducer = patternMatch(0)
.case('TOP_LEVEL_RANDOM_GIF_UPDATED.NEW_GIF', incrementCounter)
.case('RANDOM_GIF_PAIR.[GifViewerId].NEW_GIF', incrementCounter)
.case('RANDOM_GIF_PAIR_OF_PAIR_UPDATED.[PairId].[GifViewerId].NEW_GIF', incrementCounter);
// 2) Using Elmish Saga Solution
const gifViewerSaga = iterable => iterable
.filter({ action } => action.type === 'NEW_GIF')
.map(() => ({ type: 'INCREMENT_COUNTER' }));
const counterReducer = patternMatch(0)
.case('INCREMENT_COUNTER', incrementCounter); |
thanks @gdotdesign I'll have to dig a bit deeper because I'm not used to Elm syntax so much yet but I see the point of wiring things together from outside the decoupled components. Maybe it's possible to do the same like @tomkis1 did in solution 1, by unwrapping actions at the top level manually but it seems more manageable to have a global inbox. @tomkis1 About solution 1, it seems to work but not sure it scales well as you introduce many newgif components to your app in multiple places you are very likely to forget one case at some point and not notice it immediately. About solution 2, it looks like the events are not nested anymore right? So is it still a fractal architecture? My point by opening this topic is that Elm is fractal with nested events, and Flux is global with flat events. I think the idea is to see how both worlds can be combined into a solution that works well for real world apps. Using 2 mailboxes/buses/channels seems to be a solution. @tomkis1 @gdotdesign , I see you both did: const incrementCounter = model => model + (model > 10 ? 2 : 1); and Global.Increment -> let by = if (counterCount model) > 10 then 2 else 1 I think I'll have to change this requirement because I wanted it to be more complex than that actually :) The initial idea of this requirement was to make so that this increment business rule can't be handled in the NewGif component. But I did not though it would be easy to handle it in the counter itself. My point with this rule was to see if sagas are useful in fractal architecture or if there is another possible solution, and find a requirement where a business rule seems to belong to nowhere (at least neither in NewGif or Counter). So here is the business rule modified:
The idea is that in your counter updater function, you can't access easily the state of the button, because the counter and the button are really far away in the DOM tree. So this rule does not seem to belong anywhere in the NewGif, the Counter or the Button component, but somewhere else. |
@jarvisaoieong I'm closing this issue because I've made a separate github repository for this problem. I try to make the problem clear and keep an up to date specification so that people can submit proposals. https://github.com/slorber/scalable-frontend-with-elm-or-redux I'll be happy if you all could submit your solutions there :) thanks |
thank you for your contribution. |
Moved to https://github.com/slorber/scalable-frontend-with-elm-or-redux
The text was updated successfully, but these errors were encountered: