daslaf / react-clean-architecture

220 stars 43 forks source link

Thanks! #3

Open leepowelldev opened 2 years ago

leepowelldev commented 2 years ago

This is by far the cleanest (no pun intended) implementation of the architecture I've seen. Very well explained and laid out. In a real world app, would you still split into data, domain, view etc directories?

daslaf commented 2 years ago

Hey @leepowelldev, thank you very much for your kind words!

I've started to empathize with the idea that folder structures is a form of bikeshedding. What I would recommend is to do whatever works for you. If you're starting a brand new application, you might not need to split things up in different folders. You might start putting everything in a single file, and then start splitting up into smaller files once the project grows. You can implement a CLEAN approach and have a good separation of concerns with layering regardless of your folder structure.

Personally, I prefer going with a vertical slice approach, where you split you app by feature, and each feature might have one or all of the layers (domain, data, views, etc). I don't have a full blown application implementing this that I can share publicly , but it's something I'm gonna do down the road in the next few months.

leepowelldev commented 2 years ago

Yes, you're right. Thanks for taking the time to reply. I keep coming back and using it as a reference to build my mental model. I think if I was building this out myself, from scratch, then I'm not sure if I would have naturally placed the CounterStore interface into the domain layer ... I think I may have placed it either into the controller or data directories. But I guess data is the next layer above controller and it shouldn't have it as an import dependency - would that be correct?

daslaf commented 2 years ago

I think if I was building this out myself, from scratch, then I'm not sure if I would have naturally placed the CounterStore interface into the domain layer ... I think I may have placed it either into the controller or data directories.

I guess this feels the most natural thing to do and I used to structure my code this way until I learned about dependency inversion. By defining the store interface in the domain, then the domain logic owns how the state should look, while keeping the layer free of infrastructure or data related code. It's just a little trick to keep dependencies flowing in a single direction. And when you look at a dependency diagram it's much more clear to see what depends on what, otherwise you end up with a mess of arrows and it makes it much more difficult to understand.

leepowelldev commented 2 years ago

You're right of course - I guess that's where I need to change my thinking 👍

leepowelldev commented 2 years ago

Hi. I was going through your article again last night (https://dev.to/daslaf/clean-architecture-for-react-apps-3g3m) and came across the following:

In reality this implementation really should go in the repository, but to be honest there's no clear consensus. Some articles say you should inject services here while other implementations hide the asynchrony in the repository, like I've done. I haven't tested both approaches extensively to make up my mind yet. If you ask me, it depends on how much you want to make this library agnostic or not.

In this section, were you referring to the debounced task being in the repository, or the calls to the counter service? Sorry, I'm a little confused about how it reads.

daslaf commented 2 years ago

I think I was talking about the actual service calls. For example, in some .NET videos I saw by Programming with Mosh, the repository implemented a method for getting a resource which had an async signature (it returned a Future/Promise). In other examples, the repository was treated kinda like plain old redux store, where you could only read and write synchronously.

In the implementation shown in my article, I follow the first approach, given the signature of loadInitialCounter.

Most libraries for state management in React will let you define async actions/functions. Now, let's go back the where the data store interface is declared:

interface CounterStore {
  // State
  counter: Counter | undefined;
  isLoading: boolean;
  isUpdating: boolean;

  // Actions
  loadInitialCounter(): Promise<Counter>;
  setCounter(counter: Counter): void;
  updateCounter(counter: Counter): Promise<Counter | undefined>;
}

Now, this is conceptually flawed in terms of CLEAN. Since the data store definition belongs in the domain layer, why should the domain know that loadInitialCounter is async? That looks like an implementation detail of the service. I had a lot of questions about this same topic when writing that part of the article and I hadn't reached a conclusion.

So anyways, this is where something like CQRS comes into play (think of a plain old redux store, no async actions, no thunks, no nothing). You can read synchronously the state, it might be there, it might not be there. If you want to change something, a command is dispatched, it doesn't guarantee that when issuing the command you'll get a response right away.

For whatever it's worth, I think understanding the tradeoffs of choosing one approach over the other is far more valuable than being conceptually correct with CLEAN or not.

Hope this helps clarifying some of the raw edges on the article. I think it might be time to update it.

leepowelldev commented 2 years ago

Now, this is conceptually flawed in terms of CLEAN. Since the data store definition belongs in the domain layer, why should the domain know that loadInitialCounter is async?

I guess you could also apply the opposite reasoning, and why should the domain layer know if loadInitialCounter is syncronous? Options could be that you define all actions as async by default (even if they don't need to be):

loadInitialCounter(): Promise<Counter>;
setCounter(counter: Counter): Promise<void;>
updateCounter(counter: Counter): Promise<Counter | undefined>;

or use a union and apply a maybe reasoning:

loadInitialCounter(): Promise<Counter> | Counter;
setCounter(counter: Counter): Promise<void> | void; 
updateCounter(counter: Counter): Promise<Counter | undefined> | Counter | undefined;

Both options would leave the implementation flexible enough to meet the interface requirements.