Angular 2アンチパターン集

2016/12/03 に Qiita に投稿した記事のアーカイブです。本文中のリンクは動作しないことがあります。

@armorik83 です。Angular アドベントカレンダーの 4 日目となる本日は、かつて好評だった拙記事"AngularJS アンチパターン集"にあやかって、Angular 2（以下、単に Angular と表記したときは 2.0 以上 4 未満を指す）について気をつけた方がよい点――アンチパターンをまとめたいと思います。

Angular 2.x でアンチパターンは起こりうるか

Angular 1 系とは大きく API が変わった Angular 2 系。API が大きく変わった理由として、Web 標準により近い構成を取れるようにする目的がありました。例えば、angular.module()ではなく TypeScript のimport fromをベースとするソース分割の仕組みであったり、ES2015 classを標準とした Component や Service の定義であったり、$qではなく標準のPromiseを使ったりなどです。

このように API がより Web 標準に近づくことによって、それぞれの開発者がバラバラなスタイルで書いてしまう可能性が下がりました。公式のチュートリアルを確認し、angular-cliが生成するファイルの作法に則っておけば、まず間違いない構成で開発を進めることができます。

ではこれで、Angular を用いて開発する上で何も支障なく進められるかというと、そうでもありません。Angular 1 に比べて Web 標準という前提が整い API の数も整理されたとはいえ、その API の使い方次第では、あまり望ましくない結果に辿り着いてしまうことがあります。

望ましくない結果は、主にチームで開発する上での API 習熟度の差によって起こりやすい、言い換えると見よう見まねで解釈したときに起こりやすい、というのが筆者の経験則です。本稿では筆者の業務経験（Angular 2 歴 1 年）に基づいたアンチパターンを掲載します。

アンチパターン集

Component の初期化処理を constructor に書く

問題点

Component の初期化処理をconstructorに書くべきではありません。

@Component({
  selector: "my-foo",
  template: ` {{ value }} `,
})
export class Foo {
  @Input() value: number;
  constructor() {
    console.log("constructor", this.value); // undefined
  }

  ngOnInit() {
    console.log("ngOnInit", this.value); // 期待される値
  }
}

@Input()評価のタイミングはconstructorより後のため、@Input()で受ける値をconstructorで期待するとundefinedとなります。

https://plnkr.co/edit/X1rrqoMLy54z2jn4lslw

###解決策
Angular ではngOnInit()やngOnDestroy()などといったLifecycle Hooksと呼ばれる API がありますが、このうちのngOnInit()を初期化処理の記述に用いましょう。

@Input()が絡まない値はconstructor内でも記述できますが、「この内容はconstructor、この内容はngOnInit()」という判断を毎回迫られることになり、特にチームで開発する場合にレビュー基準がブレやすいことから、一律でngOnInit()に書くと決めておいたほうがよいです。

Component のスーパークラスに ngOnInit の共通処理を書く

問題点

Angular 2 からは Component や Service をclass構文で記述できるようになりました。そのため、多くの Component の共通処理をextends AbstractComponentなどとして別クラスにまとめたい状況があると思います。

class AbstractComponent {
  // ...
}

@Component({
  selector: "my-foo",
})
export class FooComponent extends AbstractComponent {
  constructor() {
    super();
  }
}

この場合、注意が必要です。Lifecycle Hooks にあるngで始まる名前のメソッドを継承した場合、予期せぬ動きをする場合があります。

class AbstractComponent {
  ngOnInit() {
    // なにか共通処理
  }
}

@Component({
  selector: "my-foo",
})
export class FooComponent extends AbstractComponent {
  constructor() {
    super();
  }

  ngOnInit() {
    super.ngOnInit();
  }
}

筆者が頻繁にハマったのは Router が絡んだケースで、大きなアプリケーションになるほど、この継承による思わぬ挙動が発見しづらいバグとして潜みます。

解決策

解決策としてはそもそも継承せずに、例えば共通処理を関数として切り出して全ての Component がその処理を呼ぶ、というのが一番安全です。もし継承したい場合は、constructor内や Lifecycle Hooks と被らない名前のメソッドだと経験的には問題が起こっていません（保証はできません）。

もし Lifecycle Hooks 周りの挙動で不思議な動きをした場合、その周辺にextendsが絡んでいないかを確認するとよさそうです。

Input にオブジェクトを渡す

問題点

この状況は比較的軽い問題なので行うことも多々あると思いますが、注意が必要です。

@Component({
  selector: "my-foo",
  template: ``,
})
export class Foo {
  @Input() primitive: number;

  ngOnChanges() {
    console.log("this.primitive", this.primitive);
  }
}

@Component({
  selector: "my-bar",
  template: ``,
})
export class Bar {
  @Input() obj: { value: number };

  ngOnChanges() {
    console.log("this.obj.value", this.obj.value);
  }
}

@Component({
  selector: "my-app",
  template: `
    <my-foo [primitive]="primitive"></my-foo>
    <my-bar [obj]="obj"></my-bar>
  `,
})
export class App {
  primitive: number;
  obj: { value: number };

  ngOnInit() {
    this.obj = {};
    let i = 0;
    let j = 0;

    setInterval(() => {
      i += 1;
      this.primitive = i;
    }, 1000);

    setInterval(() => {
      j += 1;
      this.obj.value = j;
    }, 1000);
  }
}

たとえば上のようなケースです。ここでは毎秒値を変えながらFooとBarに渡していますが、毎秒ngOnChanges()が発火するFooに対して、オブジェクトを受け取るBarは最初の一度しかngOnChanges()が発火しません。

https://plnkr.co/edit/Kz30iJsKpYSk70yAVR1c

解決策

オブジェクトや配列ではなく、プリミティブな値を渡すように修正します。

ngOnChangesは参照が変化したときに発火します。すなわち、プリミティブな値（number, string, boolean）が変更されたとき、またはオブジェクト、配列の参照が変わった時に限ります。オブジェクトや配列の中身が変わっただけでは発火しないのです。このため、ngOnChanges()が絡む時にはバグの温床となりやすいため、オブジェクト、配列は直接渡さないことを推奨します。

または、次のように書くこともできます。

@Component({
  selector: "my-foo",
  template: ``,
})
export class Foo {
  @Input() primitive: number; // こうではなく

  ngOnChanges() {
    console.log("this.primitive", this.primitive);
  }
}

@Component({
  selector: "my-foo",
  template: ``,
})
export class Foo {
  private _primitive: number;

  // setterに直接@Input()を付与
  @Input() set primitive(val: number) {
    this._primitive = val;
    console.log("this._primitive", this._primitive);
  }
}

@Input()をプロパティではなく、setter に直接付与することで、このプロパティが変更されたときに限定的に処理を実行することもできます。setter 方式の場合は、値を保持したい場合別名のプロパティに再格納する必要があるので、ngOnChanges()の引数SimpleChangesを使うか setter 方式を使うかは、状況や複雑さに応じて判断してください。

一方で、ngDoCheck()という Lifecycle Hooks とKeyValueDiffersまたはIterableDiffersを組み合わせることでもこの問題を解決できます。ただし、ngDoCheck()は非常に高負荷になりやすい API であり各 Differs の初期化も煩雑であることから、チーム開発時に高い習熟を求められ、経験上あまりお勧めはしません。

テンプレート内に DI した Service をそのまま書く

問題点

Angular のテンプレート構文の解釈はよくできており、JavaScript と同様の動きをしているように見えます。

@Injectable()
export class Service {
  getValue(): string {
    return "value";
  }
}

@Component({
  selector: "my-app",
  template: ` <p>{{ service.getValue() }}</p> `,
})
export class App {
  constructor(public service: Service) {}
}

たとえばこの<p>{{service.getValue()}}</p>は、正しくService#getValue()メソッドが呼ばれ、ブラウザ上には"value"と表示されます。

これは動作するため一見問題ないように見えますが、パフォーマンス上で問題となってきます。テンプレート中に関数呼び出しが書かれると、Change Detection（変更検知）のたびに一度呼び出されてその結果を比較して変更を認識するので、パフォーマンスが劣化するのです。

https://plnkr.co/edit/7xqdry9B1fqZy4Zxx7et

解決策

多少面倒でも、一旦プロパティに受けるのが望ましいです。

@Component({
  selector: "my-app",
  template: `
    <p>{{ value }}</p>
    <!--DIしたServiceを使わない-->
  `,
})
export class App {
  value: string;
  constructor(public service: Service) {}

  ngOnInit() {
    this.value = this.service.getValue(); // 一旦受ける
  }
}

テンプレート内は文字列なので TypeScript のコンパイルで検知できないというデメリットも存在します。ただしこれは、ngc (Angular compiler-cli)を使っていれば TypeScript のコンパイルエラーとして検知できるようになります。

そのため、なにより「AoT (Ahead-of-time compilation)を意識せずに Angular の開発を続ける」ことこそがアンチパターンともいえます。

Component の Input に関数を渡す

問題点

Component の@Input()に関数を渡すのは避けましょう。@Input()には値を与えるようにすべきです。「今回は特例として」などと言いながら書いてしまっても、数ヶ月もすれば何故そう書いたのか分からなくなるものです。

@Component({
  selector: "my-foo",
  template: ``,
})
export class Foo {
  @Input() callback: Function;
  ngOnInit() {
    this.callback("hello");
  }
}

@Component({
  selector: "my-app",
  template: ` <my-foo [callback]="callback"></my-foo> `,
})
export class App {
  callback: Function;

  ngOnInit() {
    this.callback = (str: string) => {
      console.log("this.callback", str); // hello
    };
  }
}

https://plnkr.co/edit/IgcglJ9E0KLUEZlksgEp

解決策

素直に@Output()で実装できないか検討しなおすのがよいでしょう。どうしても@Output()では実現できない場合、それは Angular として複雑なことをやろうとしている可能性があります。落ち着いて Angular-way で進められないか考えたほうが、他の開発者へ実装の意図を示す上でも望ましいでしょう。

@Component({
  selector: "my-foo",
  template: ``,
})
export class Foo {
  @Output() ev = new EventEmitter();
  ngOnInit() {
    this.ev.emit("hello");
  }
}

@Component({
  selector: "my-app",
  template: ` <my-foo (ev)="handle($event)"></my-foo> `,
})
export class App {
  handle(ev: string) {
    console.log(ev); // hello
  }
}

Angular に慣れている方からすれば「そんなばかな」と思うかもしれませんが、実際に起こったケースです。

https://plnkr.co/edit/X1rrqoMLy54z2jn4lslw

ViewChild で子 Component のメソッドを直接呼ぶ

問題点

@ViewChild()を使うことで、親 Component は子 Component のインスタンス参照を得られます。これによって、子 Component が持つメソッドを直接呼ぶことができます。

@Component({
  selector: "my-bar",
  template: ``,
})
export class Bar {
  someMethod() {
    console.log("hello");
  }
}

@Component({
  selector: "my-foo",
  template: `<my-bar></my-bar>`,
})
export class Foo {
  @ViewChild(Bar) barRef: Bar;

  ngOnInit() {
    this.barRef.someMethod(); // hello
  }
}

これを行うと、せっかくのコンポーネント指向による疎な実装から、一気に結合度が上がってしまいます。親子は常にペアであることが必須となってしまいます。たとえ呼べるからといって、子 Component のメソッドを無闇に呼ぶのはやめましょう。

https://plnkr.co/edit/tv2QR1Y5BrbRwZwGi6IR

解決策

そもそも行わないことが第一ですが、どうしてもそういった実装が思いついてしまう場合、その処理は実は親に書くべきではないかと検討し直したり、代わりに@Outputで実現できないかを考え直しましょう。

筆者のケースで実際どうしようもなかったのは、子 Component の<input>に対するfocusを当てる操作で、これは泣く泣くコメント付きで許容しました。

設計段階に立ち返ればもっとスマートな解決法がある気もしますが、やむを得ずこういった状況になった場合、作り直すための工数などから勘案して、何が一番要件を満たしつつ安全になるか検討するとよいです。

ChangeDetectionStrategy を常に書かない

問題点

ChangeDetectionStrategyとは、その Component の change detector（変更検知）を一度きりにするか、常時にするか指定するための API です。

@Component({
  selector: "my-app",
  template: `<my-foo></my-foo>`,
  changeDetection: ChangeDetectionStrategy.OnPush,
})
export class App {}

このように記述します。ChangeDetectionStrategy.OnPushは一度きり、ChangeDetectionStrategy.Defaultは常時でこれが初期値です。記述しない場合はDefaultが適用されますが、これは常に記述することをお勧めします。

なぜ

開発初期には、書かないまま進めることが多いと想像します。ただ、パフォーマンス・チューニングの段階でChangeDetectionStrategy.OnPushを書き始めたときに問題が発生します。

ChangeDetectionStrategy.OnPushを付けてみたらバグが起こったとしましょう。このときOnPushを除去する修正はせずに、Defaultに書き換えるべきです。開発中期〜後期であとからOnPushを付けて回るとき、どの Component にバグが起こったか分からなくなってしまうためです。

修正時に常にDefaultを付けるようにすれば、次の 3 ケースの判別が付きやすくなります。

• changeDetectionが未定義のとき
  • 検証前
• OnPushのとき
  • 検証し問題がなかった
• Defaultのとき
  • 検証の上問題があったためOnPushは不採用

解決策

開発中期〜後期にchangeDetectionを意識し始めた場合は上記のようにすればよいでしょう。ではこの情報を開発前から知っていた場合はというと、これは最初からChangeDetectionStrategy.OnPushを意図的に書いておくとよいです。

こうすることで「開発中に想定した動きをしなかった →Defaultに変更することで動くようになる」という実装フローになり、あとからのパフォーマンス・チューニング時に何度もOnPushとDefaultを切り替えながらデバッグする手間を省けます。

ただし、これは実装着手時にOnPushである前提のため、かえって分かりにくいと感じる方もおられるかもしれません。パフォーマンス要件が緩い場合は、これを一切気にせず常にDefaultでもいいでしょう。これはパフォーマンス・チューニング時の煩雑だった経験による提案です。

*ngIfと*ngForを一つのタグに書く

問題点

一つのタグ、Component に対して*ngIfと*ngForの両方を書いてはいけません。

@Component({
  selector: "my-app",
  template: ` <p *ngIf="state" *ngFor="let v of values">{{ v }}</p> `,
})
export class App {
  state: boolean;
  values: number[];
  ngOnInit() {
    this.state = true;
    this.values = [0, 1, 2];
  }
}

これは例外が出るのですぐに気付いて修正できますが、慣れない間はやりがちです。

解決策

次のように入れ子にします。

@Component({
  selector: "my-app",
  template: `
    <ng-container *ngIf="state">
      <p *ngFor="let v of values">{{ v }}</p>
    </ng-container>
  `,
})
export class App {
  state: boolean;
  values: number[];
  ngOnInit() {
    this.state = true;
    this.values = [0, 1, 2];
  }
}

<div *ngIf="">と書きそうになりますが、ここでは<ng-container>をお勧めします。<ng-container>はテンプレート記述上では入れ子を宣言できますが、実際の DOM 生成では無視されるため無駄な<div>を吐かずに済むという利点があります。

いくつかのタグをまとめて*ngForしたいとき<template>でまとめる

問題点

次のようなマークアップを*ngForで繰り返したいとき、どうすればよいでしょう。

<dl>
  <dt>定義</dt>
  <dd>内容</dd>
</dl>

<dl *ngFor="...">
  <dt>定義</dt>
  <dd>内容</dd>
</dl>

これだと<dl>がいくつも生成されるので誤りです。こういった<dt><dd>を常にペアで繰り返したい場合、検索して Stack Overflow などに辿り着いてしまうと、古い情報が案内されていることがあります。

<dl>
  <template ngFor let-v [ngForOf]="values">
    <dt>{{v.term}}</dt>
    <dd>{{v.description}}</dd>
  </template>
</dl>

例えばこのようなもので、<template>を使うものです。これはお勧めできません。

解決策

これも前節と同じように<ng-container>を使ったほうが安全です。

<dl>
  <ng-container *ngFor="let v of values">
    <dt>{{v.term}}</dt>
    <dd>{{v.description}}</dd>
  </ng-container>
</dl>

<template>と<ng-container>の挙動は 2.x の時点では同じですが、今後<template>を<ng-template>に変更しようという動きが観察されます。まだはっきりと公式が非推奨と謳っているわけではありませんが、同様の挙動であれば少しでも安全な方を採用したほうがよいでしょう。

CSS に:host-contextや>>>を使う

問題点

:host-context(.foo)セレクタは、自身の host 階層を遡ってfooという class が付与された親がいるかどうかを探します。自分の直近でない親まで辿ってしまい依存することになるので、とても変更に弱い指定となります。

また、>>> (piercing operator)も、自身より下層の Scoped CSS にアクセスする機能なので過度に依存が増えてしまい、折角のコンポーネント指向を台無しにします。

解決策

使わないことが第一です。なによりコンポーネント指向としてそれぞれを独立して組むことがベストのため、スタイル周りで祖先や子孫に過干渉したくなる場合は構造を見直すのがよいです。せっかくの Scoped な CSS なんですから、スコープを最小限にするよう心がけましょう。

Angular における CSS のベストプラクティスは筆者自身もまだ模索中なので、今後のコミュニティからの知見に期待しています。

既存の Pipe を活用した新たな Pipe を定義する際に既存 Pipe を DI する

問題点

例えば、byte を受け取って、それを"100KB"や"1.23MB"など上手いこと返すFilesizePipeを作りたいとしましょう。この Pipe は数を扱うので Angular 標準のDecimalPipeを活用できると便利です。

このとき、いくつか失敗しやすいため解説します。

@Pipe({ name: "Filesize" })
export class FilesizePipe implements PipeTransform {
  constructor(private decimalPipe: DecimalPipe) {}

  transform(byteString: string): string {
    // 処理
  }
}

まず DI です。一見うまくいきそうですが、Pipe は Service ではないため DI できません。

@Pipe({ name: "Filesize" })
export class FilesizePipe extends DecimalPipe implements PipeTransform {
  transform(byteString: string): string {
    // 処理
  }
}

次に継承、これも失敗します。なぜならスーパークラスに@Pipe({name: 'Decimal'})が指定されておりバッティングするためです。

解決策

import { Pipe, PipeTransform, Inject, LOCALE_ID } from "@angular/core";

@Pipe({ name: "Filesize" })
export class FilesizePipe implements PipeTransform {
  private decimalPipe: DecimalPipe;

  constructor(@Inject(LOCALE_ID) locale: string) {
    this.decimalPipe = new DecimalPipe(locale);
  }

  transform(byteString: string): string {
    // 処理
  }
}

正解はこのようになります。Angular を書いていてあまり標準 API を new することはありませんが、Pipe の場合はこのように扱います。localeを DI して、それを与えて Pipe のインスタンスを生成するのがミソですね。これでDecimalPipeの処理を活用した独自 Pipe が定義できます。

モックテストが行えないという指摘があるかもしれませんが、Pipe はtransform()が引数と戻り値を扱えて単体テストが書きやすいため、あまりこの状況でモックは使わないでいいという判断をしています。どうしてもモックが使いたい場合、Pipe よりも Service が適している可能性があります。

この節はあまりアンチパターンと呼べるものではないですが、筆者がとにかくハマったので掲載しておきます。

Angular 2.x は親切

Angular 1 の頃から長く触ってきた身としては、Angular 2.x は Angular 1 に比べてとにかく親切になったという印象があります。ドキュメントは豊富だし、例外ログも充実しています。そのため、まずドキュメントに沿った使い方をしていれば、つまずくことは減らせます。まずは Angular-way で開発を進めることを推奨します。

慣れてきて凝った使い方をし始めるときにはミスが増えがちです。その API が何を意図して用意されたものなのか、もう一度見つめ直すのがよいでしょう。

Angular 2.x からは class ベースとなったため、開発中のアンチパターンはどちらかというとオブジェクト指向設計そのものの進め方であったり、根本的なアーキテクチャであったりに寄りがちです。そういった class ベースの開発のノウハウは Angular でも活かせますので、既に他の言語、フレームワークでの開発経験が豊富な方ならば、おおよそ何が問題となりうるか想像がつくでしょう。本稿もそういったアンチパターンは掲載しないようにしました。

また、Angular では RxJS を活用できるよう設計されているため、慣れた方は pub/sub ベースの設計を採り入れることもあるでしょうが、そういった書き方をする際のアンチパターンも Angular の本質から外れるため今回は割愛しています。RxJS については機会があればまた。

結び

2016 年最大の Angular ニュースといえば Angular 2.0.0 リリースですが、今やもう 2.2.x です。安定した Angular でこれから開発に着手される皆さんは、ドキュメントを確認しながら、どうぞ安全運転で開発を進めてください。

---

本稿は ng-japan の@laco0416, @Quramy, @teyosh に査読いただきました。ありがとうございます。