rangle / angular-training-examples Goto Github PK

Repo structure for holding all knowledge components, while allowing for static serving and automatic rebuilds.
Keep all examples isolated and self-contained.

In Visual Studio Code, I'm seeing a couple of warnings:

We can fix the module.id problem referencing typings.d.ts and the decorator issue adding a tsconfig.json file defining only the property "experimentalDecorators": true.

PR coming.

From my perspective, when doing training we should try to keep our examples as simple as possible to not distract the students from what's really important to learn in every section.

For example, in the example of this section we have:

import { Component } from '@angular/core';

@Component({
  selector: 'app-root',
  template: `<input
    type="button"
    value="{{ label }}"
    (click)="onClick()">
    <span>{{ randomString }}</span>`,
  styles: [``]
})
export class AppComponent {
  label = 'randomize';
  randomString = randomString();
  onClick() {
      this.randomString = randomString();
  };
}

function randomString() {
  const length = (Math.floor(Math.random() * 25) + 10);
  let str = '';
  for (let i = 0; i < length; i += 1) {
      str += String.fromCharCode(Math.floor(Math.random() * 255))
  }
  return str;
}

Some people might be distracted by trying to understand how the function randomString works when the goal of this chapter is to focus on how the event system works in Angular. A simple counter might be enough to showcase events and keep people focused on what's really important. Also, calling a function from outside the class relying on hoisting is adding an additional layer of complexity from people coming from traditional OOP languages that are used to rely on methods.

I can submit a PR but I wanted to know first if you agree with me on this.

I have detected some inconsistencies between the code examples and the official style guide. Here are my findings and suggestions:

Filenames

• I've seen examples of a component called ChildComponent and the name of the file being child.ts. In that case the filename should be child.component.ts.
• Instead of naming the root module as example.module.ts we should follow the convention an name it app.module.ts.
• Instead of naming the root component index.html we should follow the convention and name it app.component.ts.
• Every component name should end with the suffix Component so instead of naming a component like:

component-1.ts

@Component({
  selector: 'component-1',
  template: 'Component 1',
})
export class Component1 {}

Do:

first.component.ts

@Component({
  selector: 'rio-first',
  template: 'First Component',
})
export class FirstComponent {}

Imports

• To avoid confusing students with the import paths, I think we should be more explicit and instead of doing import { AppComponent, ChildComponent } from './'; do:

 import { AppComponent } from './app.component';
 import { ChildComponent } from './child.component';

Selectors

• To avoid collisions, it's a good practice to always apply a prefix to all the selectors for components and directives, in this case rio-. So instead of having app-root use rio-app.

Module Decorator Properties

There's not an official recommendation about this, but every example that I see in angular.io declare the properties of the NgModule decorator in this order:

@NgModule({
  imports: [],
  providers: [],
  declarations: [],
  exports: [],
  bootstrap: []
})

This is just a recommendation but having the imports at the top and exports at the bottom (for feature modules) is similar to the order we have in a normal ES6 module. Also having declarations close to exports makes it easy to spot which elements are public or private to the module.

Routing Module

The Angular team is now recommending of creating a separate modules for routes. So instead of having a file like this:

routes.ts

import { Routes } from '@angular/router';
import { Component1 } from './component-1';
import { Component2 } from './component-2';

export const routes: Routes = [
  { path: '', redirectTo: 'component-1', pathMatch: 'full' },
  { path: 'component-1', component: Component1 },
  { path: 'component-2', component: Component2 },
  { path: '**', component: Component1 },
];

Do:

app-routing.module.ts

import { NgModule }     from '@angular/core';
import { RouterModule } from '@angular/router';

import { FirstComponent } from './first.component';
import { SecondComponent } from './second.component';

@NgModule({
  imports: [
    RouterModule.forRoot([
      { path: '', redirectTo: 'first', pathMatch: 'full' },
      { path: 'first', component: FirstComponent },
      { path: 'second', component: SecondComponent },
      { path: '**', component: FirstComponent },
    ])
  ],
  exports: [
    RouterModule,
  ],
})
export class AppRoutingModule {}

And then in your root module:

...
@NgModule({
  imports: [
    BrowserModule,
    AppRoutingModule,
  ],
  ...
})
export class AppModule {}

Others

• We should be consistent about using 2 spaces or 4 spaces for indentation. All the html files are using 4 spaces and the js files 2 spaces.

In the code example for the section 4.1-what-if we have this code example:

@Component({
  selector: 'app-root',
  template: `<input
    type="button"
    value="{{ label }}"
    (click)="onClick()">
    <span *ngIf="label !='show'">{{ someString }}</span>`,
  styles: [``]
})
export class AppComponent {
  label = 'show';
  someString = 'What if you could conditionally show components?';
  onClick() {
      if (this.label === 'show') {
        this.label = 'hide';
      } else {
        this.label = 'show';
      }
  };
}

In my opinion is not a good practice to have logic in the template like *ngIf="label !='show'", instead we should rely on boolean instances like *ngIf="isVisible" and delegate the logic to the class. Again, I can submit a PR if you agree.