[Discussion] Regarding ADR-0009

[justindbaur]

Page

https://github.com/bitwarden/contributing-docs/blob/873d980e0caa21dd60db0ab467c5bf3fbb4f6e95/docs/architecture/adr/0009-angular-composition-over-inheritance.md

Regarding ADR-0009 I want to ensure I am clear about the process of creating a new component that has a view in each client.

I would first create a service that fully encapsulates all business logic like so:

interface IDoSomethingService {
  sayHello(name: string);
  getDataFromServer(): Observable<MyData>;
  cleanup();
}

Then I need to do the following for each client

// x3
@MyView("[client-name]-view.html")
class Component {
  textBox: HTMLElement;
  data: Observable<MyData>;

  constructor(private myService: IDoSomethingService) {}

  ngOnInit() => data = myService.getDataFromServer();

  submit() => myService.sayHello(textBox.value);

  ngOnDestroy() => myService.cleanup();
}

vs creating a base component that is nearly exactly the above component (minus the @MyView() and the implementation for each client would be like this:

// Browser
@MyView("browser-view.html")
class BrowserComponent implements Component {}

// Desktop
@MyView("desktop-view.html")
class DesktopComponent implements Component {}

// Web
@MyView("web-view.html")
class WebComponent implements Component {}

Is that a fair representation of what this ADR wants?

[MGibson1]

If we limit ourselves to services vs components, yes, that's it. I understand what you're talking about with boilerplate repitition being a drag, but it has a set of benefits for us that are totally worth it.

• separates visualization concerns more thoroughly
• allows components to natural differ between clients for native look-and-feel
• discourages inheritance pattern for unrelated components

In your particular example, you literally have the same component doing identical things and rendered three different ways. This doesn't really apply to the typical composition over inheritance warnings because the goals are identical (item 3 doesn't apply). It's just a presentational difference that can be swallowed by the template assigned. Sounds good. However, we've done this experiment and found that, by-and-large, smart components are well-aligned across clients only when first introduced and relatively small in scope. The clients don't tend to drift with respect to the original functionality, but rather with respect to added functionality that we want excluded in some set of clients.

---

Aside:

I also want to say that component vs service isn't limit of the ADR. It also applies to where divisions in components should exist -- it's better to compose components into multiple pages than have 1 abstract or virtual base component. A good example of where we're not following this today are the vault and organization_vault pages, where component behavior is being overridden rather than splitting up the large smart component into more presentational components.

[coroiu]

I also think this aligns to what we were discussing during the meeting, but there might be on more thing to consider: Page-level components, the components that do the actual compositing. In my head the steps to creating this new structure are the following:

1. Convert as many parts of the components as reasonably possible into presentational components. Hopefully these can eventually be shared between clients, which solves the issues:
   1. UI logic duplication when we no longer allow inheritance. These components should contain most of the purely presentational logic. Ex: Don't show delete button if cipher is already deleted.
   2. Template duplication, an issue we've always had.
2. Move data fetching and event handling into the page component - which in turns passes that data to the presentational components. This implementation can and will differ between clients, so this component is always unique to each client.
3. Simplify the page component.
   1. Extract all business logic into common/sdk services.
   2. If we notice significant client-specific code is needed to handle UI logic (common) or call domain-services (less common), then extract this into a client-service next to the component. Note: Components in the CL can also have these services.
   3. If we notice significant code duplication between client-services then extract this into a platform specific-service (i.e. `@bitwarden/angular`).

Presentational components have their own issues of course, eg. async actions don't always play nice with them since @Output is not able to capture the progress of the function call inside of the parent page component.

---

To add to @MGibson1 aside:
The example is not true across all of our clients, the web client now has its own separate components that follow (1) and (2) from above. The individual and organizational vault use the same presentational component and template, they just have separate page components that fetch the data from different places! (3) hasn't been done though, so there is a bit of duplication between those page components. The PR for this work can be found here: bitwarden/clients#4967

[Hinton]

I wonder if some of the confusion comes from the the incapability to share components between our three clients currently. We are working on resolving that issue in the somewhat near future for the Auth team with more teams at a later stage.

That should allow you to define Component once and share an identical template between the clients.

However until such a time, I believe it's tentatively alright to use the base component pattern with the knowledge that we'll have to remove it in the near future. And replace it with a component + different page level components. What we definitely want to avoid is base components that have different behaviors, or are extended multiple times within a single client!

Happy to have a chat about it next week.