Angular Suite Module style guide¶

This document aims to provide a set of guidelines and good practices to be followed at all times when writing any kind of symbol for an Angular Suite Module (pipes, components, directives, services, modules, entities, etc) unless there is a good reason not to (to be agreed with your team, and properly discussed and documented during the PR stages).

Do note that this document acts as a complement to the Angular style guide, and as such, we might sometimes repeat some point made in said guide (reinforcing it, with proper reference to the item on the Angular Style Guide) or override it with a recommendation better suited to our use cases. With that mind, unless this document explicitly says it, everything that's described on the Angular Style Guide should be followed as if it was written by the Suite Team.

Finally, this document doesn't cover State Management, please take a look at Angular Suite Module State Management best practices instead.

General¶

Do organize your symbols by feature and not by type¶

Angular Style 04-01

Our AngularJs WebUi projects tends to organize symbols based on their type (i.e. controllers, views/templates, directives, services, entities). This means that when working on any given feature we need to switch between many folders that are not anywhere near one another. Angular promotes another type of structure, where we organize our code around features. Additionally, components generated by the ng cli will be inside their own folder, which would contain component logic, view, css and unit testing symbols.

Do follow T-DRY¶

Angular Style 04-05

Strive for D.R.Y. (don't repeat yourself) as much as possible. Except when you're sacrificing something else, be it readability, ability to react easier to requirement changes. Sure, code duplication introduces bugs and increases our cognitive load when refactoring and testing, but question yourself every time you get one of those "Hey, I can refactor this and reduce duplication!": does the abstraction make sense? is the refactored code as resistant as before? can anyone on your team read it as easily as before?

Some more resources on this:

Do follow YAGNI¶

Try to balance the code for the feature you're coding right now, with the cost of refactoring/improving/expanding it later.

YAGNI, Martin Fowler

Do make methods return types explicit¶

Even if the typescript tool chain is most of the time capable of inferring the return type of our methods, it is advisable to explicitly type them. This doesn't only improve our code by not relying on implicitness (that could change when we switch typescript versions), but also makes it easier to perform PR reviews.

Avoid

TypeScript
public processEvent(event: Event) {
    return this.externalService.postEvent(event);
}

Allowed

TypeScript
public processEvent(event: Event) : IPromise<ExternalResponse> {
    const preProcessEvent = (event: Event) => {
        return event.target.toString()
    }

    return this.externalService.postEvent(preProcessEvent(event));
}

Do note how this recommendation explicitly target methods. If you're using a const function or internal function, you might omit return types for brevity.

Do avoid `any` like the plague¶

Not a lot to say here. There are not a lot of situations where you should explicitly type a parameter or return type as any. Eslint rules should remind you of this recommendation.

Do embrace the `unknown`¶

If you find yourself reaching for the any type and typing an... ask yourself:

Do I need to invoke some method or access some property on this object?
Am I just using any to signal the TS tool chain that I don't know and don't care about this object type?

If your answers were No and Yes, then congrats: you can (and should) use the newly introduced Unknown type (TS 3)

Do embrace RxJS operators¶

Angular moves from the promise based approach to async code in favor of a rxjs based one. We won't go into details about the differences between them. However, we will list some operators you should have in mind when writing reactive code:

map
filter
tap
takeUntil
withLatestFrom
distinctUntilChanged
startWith
switchMap

Resources:

Do suffix your observable symbols with `$`¶

This makes it easier when scanning a template to find all the observable members. This convention can be found here.

Components¶

Do not use `OnChanges`¶

onChange is a lifecycle hook for Angular Components, which is triggered when a component receives a new value for any of its Input properties through a template binding. There are two consequences from that last statement:

It will only be triggered when the new value is provided via template binding. If you want to test onChange logic, you will need to manually update your component and call onChange or wrap your component on a testing template.
It will only be triggered when a new value is passed to the Input property. This is by design, but still can take some developers by surprise.

Instead, a getter/setter combination is preferable when dealing with changes triggered by a single property change. This way, change propagation logic will be enforced even when the component is accessed through a template export or ViewChild decorator.

Do not use inlined templates¶

Angular Style 05-04

Not a lot we can add to what the Angular Style Guide says, except for the fact that in our experience, Angular Language Services (the framework piece in charge of providing type checking and intellisense for templates) tends to work better on html files than on inlined templates.

Do not alias `Input`s and `Output`s¶

Angular Style 05-13

Do prefix your event handlers with `on`¶

Event handlers invoked from a template (i.e. when bound to an DOM event or a component's Output) should be prefixed with on. Additionally, it is desirable that the event handler method name is written on simple present tense.

TypeScript
@Component({
    template: ` <button (click)="onAcceptClick($event)">Accept</button> `
})
export class MyComponent {
    public onAcceptClick(event: HtmlClickEvent) {
        // ....
    }
}

Do not prefix your `Output`s¶

Angular Style 05-16

Related to the previous recommendation, event outputs from standard HTML elements (and also from Material components) are simply named after the event they model, without any prefix.

Do keep presentation logic on the component and not the template¶

Angular Style 05-17

More presentation logic on the component means less code duplication, easier to read templates and increased/more accurate unit test coverage.

Do use [INSERT METHODOLOGY] for organizing your SCSS/CSS code¶

TODO

Do separate your components on Smart and Dumb components¶

More often than not, we end up cramming all of our logic for one (or even worse, many!) use cases on one single view component. This means we'll have one component like mpm-jobs-grid which doesn't follow SRP and not only displays business entities, but also fetches them, deletes them, triggers modals, etc.

Dumb components are defined as components which only take data in via Input bindings and use Output event emitters to notify their parent component about any change or event that was triggered by the user on them.

Smart components can inject as many services as they required, will unpack observables received from them using the async pipe and pass them into dumb components Inputs and handle said dumb components Output events.

This separation afford us a lot of advantages, namely: Better performance by using OnPush change detection, presentation components isolation (so they can be showcased and composed using Storybook, for example), simpler and smaller components, easier unit testing (Yes, we can mock all things, but still is harder from a cognitive point of view).

This is all a TL;DR of course, we'll expand on this topic as we code through the Suite Angular Modules, but for now these resources might prove useful:

Do use `ChangeDetectionStrategy.OnPush` whenever possible¶

OnPush change detection is used in order to signal the Angular framework that our component code adheres to certain assumptions, and in consequence some optimizations can be performed, thus making our code faster and better performing.

This is all a TL;DR of course, we'll expand on this topic as we code through the Suite Angular Modules, but for now these resources might prove useful:

Do implement lifecycle hook interfaces¶

Angular Style 09-01

This will most likely be enforced by our ESLint rules, but still. Do not just implement lifecycle methods, but rather implement their corresponding interface so that other Angular infrastructure as schematics (and also our own) can benefit from it down the road.

If you subscribe to observables inside your component logic, your code becomes imperative instead of declarative. Not only that, but you have to take care of unsubscribing from any observable you subscribed to in order to prevent memory leaks. This is not only error prone but also repetitive.

Instead, "unpack" your observables in your template code by using either the ngrxPush pipe or the ngrxLet structural directive.

Usually the builtin async pipe would be recommended for unpacking observables, but said pipe has some issues such as handling multiple subscriptions to the same observable, compatibility with the *ngIf directive and no zoneless support.

Do unsubscribe from subscribed observables on the `OnDestroy` lifecycle hook¶

If you find yourself needing to manually subscribe to an observable on your component logic, take care of destroying said subscription by providing an onDestroy lifecycle hook method.

Do keep in mind the difference between HTML Attributes and DOM Properties¶

For more information read here.

Directives¶

Do prefer `HostListener`/`HostBinding` vs `host` metadata¶

Angular Style 06-03

Prefer a statically typed checked way of declaring host listeners, where you have all required information on a single place (the property/method) instead of having the logic and the framework binding on two different places.

Modules¶

Do honor the boundaries for each library and module type¶

More info can be found at Angular Modules.

Services¶

Do provide your services at `root`¶

Angular Style 07-03

Providing services at the root injector level help us make sure that all services are effectively singletons.

Entities¶

Do not write your Business Entities as Classes, but rather use Interfaces¶

ES6 (or any type, for that matter) Classes are not serializable, which means we need to incur on additional performance costs when de/serializing them for example to save them to local storage or to the NGRX Store.