Skip to content
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

Solution Feedback: fractal-component #45

Open
t83714 opened this issue Oct 9, 2018 · 4 comments
Open

Solution Feedback: fractal-component #45

t83714 opened this issue Oct 9, 2018 · 4 comments

Comments

@t83714
Copy link
Contributor

t83714 commented Oct 9, 2018

Has submitted my solution via #44

Created this issue to make it easier for anyone who wants to check it out.

The problem of building a scalable frontend application using Redux or Elm has been well described by @slorber here

fractal-component perfectly solved this problem by providing a fully decoupled component encapsulation mechanism --- a component created using fractal-component can be even published as NPM modules while still enjoy the convenience of predictable global Redux store.

You can actually have a look at the NPM module version of this example on CodePen --- see how whole example App's logic is implemented in a single HTML file by pulling UMD version of NPM modules from CDN :-)

How fractal-component achieves that? It powers your components with the following features:

  • Multicast Actions
  • Namespaced Actions
  • Serializable Symbol Action Type
  • Hot Plug Redux Reducer & Auto mount / unmount
  • Hot Plug Saga & Auto mount / unmount
  • Namespaced Redux Store
  • Auto Component State Management & Redux Store Mapping
  • Enhanced Server Side Rendering (SSR) Support

A typical structure of components created by fractal-component can be illustrated by the graph below:

Typical Container Container Component Structure Diagram

Run example App locally

npm install
npm start

Then, visit http://localhost:3000/

Giphy.com API key

The exampleApp comes with a testing Giphy.com API key in order to retrieve random Gifs from https://giphy.com/. The api key is limted to 40 requests per hour.

You can create your own API key from https://developers.giphy.com/ and replace the API key in Component RandomGif

@ghola
Copy link

ghola commented Oct 16, 2018

The solution is cumbersome and breaks the open/closed principle which states:

software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification

The claim fractal-component makes is that each component can potentially be published as an independent NPM module. This is the same as saying that the code is correctly encapsulated. While the open/closed principle and encapsulation are not the same thing, I for one consider one as a requirement for the other.

So how does it break it? If you look at the ToggleButton component, that ActionForwarder inside it (as well as it's own props) are specifically built to serve the requirements of the project, not the requirements of the component. That is, without the business rule requirement:

if ( ( counter >= 10 ) && ( buttonState === "active" ) ) { 
  increment by 2 
} 
else { 
  increment by 1 
}

that code would have no place being there. If the business rule changes in a significant way or if new, different business rules are added, using the current approach you would need to go back into the ToggleButton component and add code to it so it supports the new logic (add a new forwarder, possibly also new props or change the forwarder logic). The component is not closed for modification and can't really be an independent NPM module. It is too concerned with what it happening outside itself.

Additionally there's the problem of modifying actions in transit. Coming from the DDD world, commands and events are immutable. You can listen to an event and publish new events or send commands as a result, but you cannot re-publish the same event with modified data. I know in the redux world there is no distinction between events and commands, but thinking in terms of events and commands can be very useful. I know that DDD and the idea of bounded contexts appeals to you because I saw how you separated public actions from private ones. In DDD, a bounded context does this separation as well and it is considered a bad practice to publish a domain event (an internal event) outside of the bounded context. The correct way is to have a translator which listens to domain events, converts the ones which are of interest to external events. These external events are what define the public interface of that bounded context and are usually in a separate repository so that the event classes can be shared between multiple consuming bounded contexts. But I digress...

The solution is similar to the one I mention in the redux subspace feedback issue. You need to remove the forwarding params and the action forwarder from the ToggleButton component and build a new component which contains the following logic: it listens (using a saga) to:

  1. Two new action types declared in the ToggleButton component: BUTTON_ON and BUTTON_OFF. (alternatively you could also just have a single action BUTTON_TOGGLED). These actions are published by the ToggleButton component and are meant only for outside use. When one of these actions arrives, it sets it's internal state based on the message (or the message payload): isButtonEnabled = true/false;
  2. NEW_GIF actions. When such an action arrives, depending on the isButtonEnabled state, it dispatches 1 or 2 INCREASE_COUNT actions.

This component is clearly designed to serve just the application logic and the business rules. Publishing it as a separate NPM module is possible but most likely not of much use.

OR you could just create a new ActionForwarder that does the job I just described, because it certainly seems capable of doing it, except this time it would not be nested inside the ToggleButton component and it would be inside the App component like the rest of the forwarders.

@ghola
Copy link

ghola commented Oct 16, 2018

I also have an issue with all those ActionForwarders scattered around the App component. The logic is simple: when a NEW_GIF action arrives, dispatch a new INCREASE_COUNT action. The component I mention in the previous post does the exact thing with the added benefit of containing business logic related to the state of the ToggleButton. So you could replace all those ActionForwarders with this new component I'm suggesting (could be the App component itself). The code would be more readable and the logic would reside in more clearly defined place.

@t83714
Copy link
Contributor Author

t83714 commented Oct 16, 2018

Hi @ghola,

Thanks a lot for your feedback & really appreciated 😄

I think there are many ways to complete the functionality using fractal-component. fractal-component just provides the namespaced facility across actions, reducers & sagas to allow the encapsulation. And I am sure there must be a better way to do it in term of this sample app~

Here, I will try my best to explain my idea around this sample app implementation and please do let me know if anything doesn't make sense~

1> Regarding ToggleButton, I consider it as a special type of forwarder. i.e. it forward all actions like an ActionForwarder plus it tags the actions depends on its toggle status. The tagged actions will be consumed by Counter and depends on the tag to incease the count accordingly.

The the business rule requirement you've mentioned has been moved to Counter:

const { toggleButtonActive } = action;
const incresement =
state.count >= 10 && toggleButtonActive ? 2 : 1;
return { ...state, count: state.count + incresement };

I have to admit that it's a wild design and not ideal. The reason why I did that probably because of the followings:

  • I want to showcase the ActionForwarder's usage. And the potential component composition use. case.
  • Make ToggleButton's function generic & reusable for other senario. Therefore, it make sense and worth the effort to publish it as NPM module. 😄
  • Also, want to reduce the amount of glue code might be required outside components, especially, at App level. Thus, the CodePen single file version sample app would only require minimum amount of code.
  • In the end, I want to demonstrate / promote using configuration rather than glue code to connect components together --- this specifically for entry-level users as I think when creating reusable components as Component Authors, we should assume the majority of Component Users are entry level users. And configuration style may be easier (and less code) for them to use.

2> The same idea behinds Counter. I want to make it more generic so that:

  • Publish it as NPM module would make sense and worth the effort
  • Although it did cover the business logic, I hope we can potential reuse it in a more generic way in other projects.

e.g. we can move some of the business logic to Counter's properties to make it more generic ( and potentialy to be reused somewhere else):

<Counter 
  namespacePrefix="exampleApp/Counter" 
  threshold="10" //--- after 10 counts increase count by `multiplier`
  multiplier="2"  //--- times count by 2
/>

Thus, you essentially complete the business logic by configuration (in App component).
--- 😄 I am still trying to promote the configuration over glue class / code to reduce amount of the code required.

3> Regarding mutate the action in transit:

I think in Domain-Driven Design context, mutate the events or commends are considered as bad practice as:

  • You shouldn't modify event as it represents the past. You can't go back.
  • Command should be modified as Commands should be sent directly to the domain model

Here, I think if you look at it in a more abstract way, we probably can say:

  • Counter component can accept two type of command
    • One is INCREASE_COUNT type with toggleButtonActive = true
    • One is INCREASE_COUNT type with toggleButtonActive = false
  • Consider ToggleButton receive an INCREASE_COUNT event and then send out a brand new INCREASE_COUNT command with toggleButtonActive = true or false
    • We definely can create a new brand object to represent the new command. But there is really no technical impact of reuse existing data.

One reason we need Bounded Context Model in Domain-Driven Design context is that we want to avoid the concept confusion when modeling problems (e.g. the same word could have different meaning in different domain).

I had the similar problem when build fractal-component. However, my solution is use Symbol for action type (plus introduce a new facility to solve the Symbol serialization problem).
i.e. Two INCREASE_COUNT action type defined by two different components are never equal.

The sepration of public actions from private actions is actualy to solve some issues with multicast actions for nested components.

4> I am actually quite interested in the new component design that you've mentioned but hard to visualise it without seeing some code. Are you able to provide some sample code of your design? Many Thanks~ 😄

@ghola
Copy link

ghola commented Oct 17, 2018

Hey @t83714, thanks for the reply. Your arguments make sense, but I disagree with some of them. In the end it's a balance between the type of compromises you are willing to make.

I don't believe configuration over glue code is a winning strategy on the long term. It's too idealized to be viable in the real world. To make configuration work you need to author really smart components, and the smarter they are, the more dead weight they carry. They either become too abstract and generic which make them hard to understand, or they become packed with too many concrete features which only a few will make use of.

I will provide a MR with the component design I was mentioning above and then I will also post new business rules to be implemented 😄. Then we'll see how the two approaches are able to accommodate them and how many changes each of them needs to make (especially in the components which are publishable as NPM modules).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants