Zenn
🅰️

AngularJSアンチパターン集

2021/07/04に公開
Angular
tech

（150522追記）本稿の続編としてAngularJSモダンプラクティスを掲載しました。本稿は2014年9月に執筆し、情報がかなり古くなっています。続編では、AngularJS 1.4やAngular 2に関する情報をまとめ、入門者への新鮮なチュートリアル、熟練者の移行手引として作成しました。どうぞご覧ください。

この記事は記録のため残します。

AngularJS歴1年の筆者による個人的なAngularJSアンチパターン集です。自分のための戒めとメモを兼ねています。個人差があると思いますので、参考程度に。

また、筆者はTypeScriptで書いています。

Components

ComponentsのDI数が6以上になる

危険度★★★

angular.module('myApp')
.service('FooService', [
  '$q',
  '$resource',
  '$rootScope',
  'OtherServiceA',
  'OtherServiceB',
  'OtherServiceC',
  'OtherServiceD',
  FooService
]);

ComponentsとはController, Service, Factoryなどのこと。規模が大きくなるにつれて起こりがち。6以上という数字は、明確な根拠は示せないがこれまでの実感から。5つ目を加えた辺りから嫌な予感をした方がいい。

なぜ?

DI数が6以上になるとそのComponentに負わせている責務が多すぎる証拠。いわゆる肥えたコントローラ。全体の見通しが悪くなりテスト時に必要なモックの種類も増え、面倒になってくる。

他のControllerを作ったときに同じ機能が必要になるケースが多いので、再利用性も考えServiceに切り出しておきたい。

対策

各種メソッドがどのComponentを使っているのか、使っていないのかを明確にし、共通しているメソッドを早々にServiceに括り委譲する。これだけで、だいたいDIが2つほど減らせる。

DIアノテーションを書かない

危険度★★★

angular.module('myApp')
.service('FooService', function($q, $resource, $rootScope) {
  // ...
});

なぜ?

AngularJSでは引数名をパースして依存するComponentを決定するが、このままではminify時に崩れてしまう。

対策

この問題を回避するため、常にDIアノテーションを書くか、アノテーション記述支援のビルドツールを用いる。AngularJS 1.3以上ならng-appと共にng-strict-diを指定することでアノテーションを忘れたときに警告がでる。

ビルドツールはng-annotateなど。本稿では割愛。

angular.module('myApp')
.service('FooService', [
  '$q',
  '$resource',
  '$rootScope',
  FooService
]);

Controllerが直接$resourceに依存する

危険度★★★

function ExampleCtrl($resource)
{
  // ...
}

まず間違いなくアンチパターン。コアサービスである$resourceはControllerで直接使わず、真っ先にServiceに括るべき。（コアサービスとは、ここではngモジュールと、angular-resource.jsなどのng名前空間モジュールを指す）

なぜ?

サーバーAPIからデータを取ってくる行動は複数のControllerで起こりうる。Controllerで直接$resourceを扱うと、重複コードが生まれる原因となりやすい。

APIのURLの管理やパラメータの管理など一元的に書く情報も多いので、Serviceに不慣れな初期段階でも、せめてやっておく。

対策

Controller内で$resourceを使うメソッドをServiceに切り出す。他にも、全体的に$で始まるコアサービスは何かとServiceでラップすることが多い。ラップするといっても次のことを言ってるわけではない。

angular.module('myApp')
.factory('MyResource', ['$resource', function ($resource) {
  return $resource;
}]);

例としてはこんな感じ (Plunker)。

非同期処理をControllerでも書きたい場合、大抵は$promiseを返せば済むが、連鎖的に複雑なことをしたいときは$qを併用する。

主観によるラップすべきコアサービスと、Controllerでも使うコアサービス

  • Serviceでラップすべき
    • $q, $http, $location, $rootElement
    • $cookies, $resource
  • Directiveでラップすべき
    • $compile, $document, $interpolate, $parse
  • Controllerでも使う
    • $filter, $interval, $log, $rootScope, $scope, $timeout
    • $routeParams（Serviceに括ることも多い）
    • $window（DOMやGUI周りを触るならDirective）

サーバー取得データをControllerの変数に格納

危険度★☆☆

ts
class ExampleCtrl
{
  public data;

  // ...
}

$resourceをServiceに括っても、取得データをControllerに持たせるのは良くない。

なぜ?

Paginationといった変遷やルーティングによる画面の切り替えで「同じControllerを再表示するとき」にも初期化され、変数がundefinedになる瞬間が生まれる。画面上では一瞬表示が消える。

対策

Controllerインスタンスの破棄に関わらずデータを保持しておくための"SharedStore" Serviceを用意する。これはシングルトンだがグローバル変数ではなく、このServiceに依存しているComponentのみが情報を知りうる。画面上ではデータが保持され続けるので、表示が消えることもない。

なお、$resourceを用いるServiceとSharedStoreをひとまとめにするのも良くない（取得と保持は分離）。

反論

Controllerの変数に持たせて、$routeProviderのresolveを使えば画面から一瞬消える現象を防げるのでは。

この件については次を参照。

$routeProviderのresolveを使う

危険度★☆☆

angular.module('myApp')
.config(function($routeProvider) {
  $routeProvider.when('/', {
    templateUrl: 'sample.html',
    controller: /* ... */ ,
    resolve: {
      /* ... */
    }
  });
});

$routeProviderのresolveは長期的にみるとアンチパターン候補。使用する場合、問題を理解して使ったほうがよい。

なぜ?

現在のバージョン（AngularJS 1.2, 1.3系）のルート設定は頑固で、Controllerの実装が本体とConfigのresolveに散らばってしまう。そして考えずに書くとテストが困難となりやすく、DIアノテーションを別途用意する必要があるなど、運用直前に気付く問題が意外と潜んでいる。

対策

筆者個人的な感想としては、resolveよりも前述SharedStore Serviceを用いたほうがテスト、エラーハンドリング、再利用性の面で優れている。resolveはUX直結な要素なので、他で確保できれば別にこの機能にこだわる必要もないだろう。

サーバー取得データをService内で操作する

危険度★☆☆

Serviceの中で取得データ（例えば配列）を操作するのはアンチパターン候補。

なぜ?

AngularJSのAPIを使用しない純粋なJSのロジックがロックインされる。

対策

純粋なJSのロジックは独自ライブラリとしてAngularJSの外で書く。ただし、そのライブラリを用いるときは直接グローバル変数から持ってこず、一度Factoryにラップする。Factoryラップは、Serviceのテスト時にモックを使用しやすくするため。

angular.module('myApp')
.factory('myLibrary', function () {
  return myLibrary;
});

例外

この件は100%守るには厳しすぎる。どこまでService内でやるか、どこから独自ライブラリにするかは感覚的な問題。プロジェクトの方針にもよるだろう。

少しの操作なら構わないと思うが、数十行に渡る処理の中に一度もAngularJS APIが登場しなかったら、独自ライブラリを検討し始めるべき。

View

ControllerAs表記を用いない

危険度☆☆☆

賛否両論あります。

<div ng-controller="ExampleCtrl"></div>

<div ng-controller="ExampleCtrl as ex"></div>

なぜ?

複数のControllerがネストしたときに参照元で混乱が起きる。ネストしないと思っていてもDirectiveが増えるとまず起こりうる。

メリット

参照元が一目瞭然となる。$scopeをController内で使用する頻度が激減する。それにより$scope本来のAPI（$watchや$broadcastなど）を利用するときのみ意識が向けられる。

同じ対象についてのng-showやng-hideを3回以上書く

危険度★★☆

例えばログイン時と未ログイン時で表示を変える場合、さらにそれをヘッダとフッタに同じように書く場合。2回でも注意、3回だと今後見落とし発生の恐れあり。

なぜ?

ViewのHTML内はJS (TS) ソース以上に見通しが悪くなりがち。HTMLは従来のWebデザインと同じマークアップと捉えず、もはやソースコードと捉えるべき。一度量産してしまったHTMLの要素群をリファクタリングするのはJavaScriptをリファクタリングするより面倒。コピペに手が伸びたら思い留まる。

対策

AngularJSの鬼門とされるDirectiveのコツを早めに掴み、細かい部品も一個一個ケチらずDirectiveにする。「これはDirectiveにするほどでもないかな」と思ったものほどDirectiveにする価値がある。

Directiveにする例

  1. 同じ条件式のng-showやng-hideをよく書く
  • Directive化が様々な事情で出来ないならば、せめて条件式を関数にラップして、その関数を評価させる。
  • ng-showとng-hideを毎回ペアで使うようならば、そのペアをテンプレートにする。ngModelをrequireに指定してng-model属性からデータのバインドが可能。独自Scopeを作っても問題ないが、ngModelControllerを利用すると簡便。
  1. Bootstrapを利用していて<div><div><div></div></div></div>のようなdiv入れ子が続いたとき。
  • ネストが浅くなるようテンプレートDirectiveを導入。テンプレートのみ利用、複雑な処理はしなくていい。transcludeを覚えると早い。
  1. 特定の表と動的に表示が変わる見出しが常にペアな場合。<h3>動的な見出し</h3><table><tr ng-repeat="...
  • <h3>{{head}}</h3>と書きがちだが、見出しと表をまとめてDirectiveに括りバインドを1箇所に限定すると、後々のデザイン変更、要素追加（件数表示、選択中表示など）に対応しやすい。
  1. パンくずリスト
  • 前述の表と似たような理由。パンくずリストは多くのViewで使うため、ng-repeatを隠蔽しておくとデザイン変更にも強く、扱いやすい。
  1. デバッグ用表示
  • <pre>{{ data | json }}</pre>とせずに、早めにデバッグ用のDirectiveを作っておく。
  • 開発時と運用時に、Directiveの変更で一斉に出力を変更できる。replaceにすれば表に出さないことも可。<pre>をひとつひとつ消して回ることはない。

Controllerのメソッドの引数に$eventを渡す

危険度★☆☆

<input ng-change="entry($event)">

Viewのng-clickやng-changeなどで$eventを引数に指定するのは良くない。

なぜ?

$eventを使う状況はほとんどがキー入力周りかGUI周りである。これらをController内で処理すると確実に肥える。再利用性も低い。

再利用が考えられるシーンとしては、新規追加用のフォームと既存編集用のフォームでControllerを分ける場合、一画面に複数のControllerの編集リスト（お気に入りとフォロー）がある場合など。

対策

DOMに関するもの、UIに関するものはDirectiveに任せるのが最適。Directive内にもScopeとControllerを定義できるので、compileとlinkだけでは捌けないほど複雑になってきたらそこで。もっと複雑になるなら更に部分的にServiceにする。

Controllerのメソッドの引数に$indexを渡すときは注意

危険度★★☆

<input ng-change="update($index)">

ng-repeatで列挙して、各要素から$index付きで処理をするときは対象となる配列に十分注意する。

なぜ?

FilterやSortを用いた場合などで、画面上の表示と格納されている配列の順番が食い違うことがある。そのまま処理すると意図していない対象が操作される。

対策

画面上の配列と処理に使う配列を同じものにする。（拙著の記事）

コーディングスタイル

angular.module()のモジュール名が文字列リテラルのまま

危険度★☆☆

angular.module('myApp', ['ngRoute']);

module()に文字列のままモジュール名を記述しない。

var appName = 'myApp';
angular.module(appName, ['ngRoute']);

なぜ?

変更が必要になった場合の置換の手間だけでなく、変数名の場合TSコンパイラによるtypo指摘の恩恵を受けられる。JSの場合も最初に文字列で書いてしまうと、後々まで同じスタイルで書きがち。例示やチュートリアルからコピペすると起こりがち。

各種Provider名やController名も文字列リテラルを繰り返し書くより、変数にしておく。$routeProviderなどの記述で役に立つ。

$broadcastのnameを文字列リテラルで表記する

危険度★★★

<button ng-click="$broadcast('MyEvent')">Click Me</button>

なぜ?

$broadcast, $emit, $onのリスナー名を文字列リテラルで指定すると、typo時に一切の警告が無い。AngularJSはそれが間違っているか正しいのかを判別できないからである。

対策

Broadcast専門のServiceを作成し、リスナー名の管理や$broadcastの実行を一元化して各Componentからは隠蔽する。ViewのHTMLにも直接$broadcastなどと書かず、ServiceをラップしたControllerのメソッドから使う（LoDの観点から）。

後記

以上、1年間のリファクタリングや1から書き直しといった個人的経験、Stack Overflowや様々な情報源で得た知見、AngularJSリファレンス本を読んだ上での様々な反省を元に、特に面倒だった部分を挙げました。

関連リンク

思いついたらまた増やします。（14/9/16執筆、9/17改稿しました。ご意見、ご質問、ツッコミなど歓迎します）

Discussion