jarvisaoieong / redux-architecture

elm architecture in redux with redux-loop
http://jarvisaoieong.github.io/redux-architecture
265 stars 7 forks source link

Questionning fractal architectures: adding some new requirements #1

Closed slorber closed 8 years ago

slorber commented 8 years ago

Moved to https://github.com/slorber/scalable-frontend-with-elm-or-redux

jgoux commented 8 years ago

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 :

  1. MODIFY_{RANDOM_GIF, PAIR_OF_RANDOM_GIF, PAIR_OF_PAIR_OF_RANDOM_GIF}
  2. if the action contains a deep action of type NEW_GIF then
  3. MODIFY_COUNTER with the action INCREMENT_COUNTER

As we love predictability, we'll assume that an action containing a "higher-order action" is of shape : { type: SOME_RANDOM_TYPE, payload: { action } }

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. :+1:

I put the code in this gist : https://gist.github.com/jgoux/ecbecad888b9e481a7e4

slorber commented 8 years ago

@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 findDeepActionOfType should have typed parameters and return something typed. I'm looking at Elm language documentation and I see no instanceof equivalent nor casting but a strongly typed language. As you may know, Elm philisophy is somehow that Elm code can not fail.

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 NEW_GIF action to it. Now you are incrementing the counter also when we interact with this new widget, because you have a deeply nested actions naming conflict. These 2 widgets have to know about each others because they have to avoid naming conflicts, and they become coupled :)

This was a nice try however and is probably applicable in most javascript projects :)

minedeljkovic commented 8 years ago

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 NEW_GIF action to it. Now you are incrementing the counter also when we interact with this new widget, because you have a deeply nested actions naming conflict. These 2 widgets have to know about each others because they have to avoid naming conflicts, and they become coupled :)

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.

jarvisaoieong commented 8 years ago

Thank you all for contributing the idea. I think @jgoux 's proposal is a workable solution. It is intuitive. Here I propose an alternative solution used the high order function concept on reducer. I've pushed the code on the hor (high order reducer) branch. The commit is here. Please check it.

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,
});

action log for easy understanding the flow:

ghost commented 8 years ago

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.

jgoux commented 8 years ago

@slorber

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 NEW_GIF action to it. Now you are incrementing the counter also when we interact with this new widget, because you have a deeply nested actions naming conflict. These 2 widgets have to know about each others because they have to avoid naming conflicts, and they become coupled :)

This could be avoided by introducing namespaces in the actions names, you may have something like Widget1/NEW_GIF and Widget2/NEW_GIF, exactly like the middlewares do : @@MiddlewareName/INIT

I think it covers the majority of the cases in JS, but I have no idea about the typed solution. ^_^

slorber commented 8 years ago

@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...

gdotdesign commented 8 years ago

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

jgoux commented 8 years ago

Thanks a lot for this example @gdotdesign ! :smile:

slorber commented 8 years ago

@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 newGifCounterHOR but the second mailbox is actually the effects. The initial update function is decoupled but you wrap and couple it in a meaningful way for your app.

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.

gdotdesign commented 8 years ago

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 Counter will become Counter a and it will be usable in different places.

I'll update the example in the morning, and post some explanation.

slorber commented 8 years ago

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 :)

gdotdesign commented 8 years ago

So I updated the example by making the Counter typed, meaning when initializing a model of it you need to pass it both the address to send the action to and the action itself as well. To demonstrate this I added to the example a Decrement action for the global, and an other counter to use this action:

-- 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 Global module and can be freely used wherever we want.

You can check out the diff here https://github.com/gdotdesign/global-actions-example/commit/c55ef50830047c90691966040cdc3c77d7778d54

Explanation

This is essentially is the same as Elms List a or Array a so what in Elm is a list of something or array of something in this example becomes counter of a specific action. The beauty of this system is that it restricts the types that are used in the model to have specific interactions, in this case the addresses that doesn't have the same type as the action cannot be used and will give a compile error.

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.

tomkis commented 8 years ago

Using redux-elm there are two options:

1) Unwrap action in the counter reducer (assuming it's higher in the reducer hierarchy) 2) Use saga as orchestrator

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);
slorber commented 8 years ago

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:

There is now a button in your application. The button can be either active or inactive (by clicking on it). When a NEW_GIF action is fired, the counter should be incremented by 2 when button is active, and by 1 when button is inactive.

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.

slorber commented 8 years ago

@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

jarvisaoieong commented 8 years ago

thank you for your contribution. That is a valuable challenge. I will submit my solution based on HoF today. Good job.