🙅‍♂️

2年前の自分に送る!べからず集

2023/12/20に公開

本稿はAngular Advent Calendar 2023 20日目の記事です。
19日目の記事は@sosukesuzukiさんのPrettier の Angular サポートの仕組みと built-in control flow 対応でした。

2年前の自分に送る!べからず集

筆者がエンジニアに転職し、Angularを始めてはや2年と少しが経過。
プロダクトも初期段階&フロントエンドをリードするメンバー不在の中、暗中模索の毎日でした。
「このク〇コードは誰が書いたn……自分やわ」という羞恥と申し訳なさにまみれながら、少しでも良いコードを書こうと思考と試行を重ねる日々。
そんな過去の反省からみえてきたコンポーネント設計のプラクティスがいくつかあります。
年末の禊もかねて、2年前の自分に伝えたい「べからず(ではどうすべきか)」集を公開します。

この記事で登場するサンプルアプリケーション

v17のキャッチアップしたかったので説明用に「クリスマスに食べたいメニュー」を投票するサンプルアプリを作りました。機能は以下のとおりです。

  • 食べたいメニュー登録(メニュー名、推薦文、投稿者を入力)
    • 編集機能はあとから追加
  • 登録されたメニュー一覧表示
  • 食べたいメニューに投票
  • メニュー削除


一覧画面


登録画面(ダイアログ)

なお、簡便のため登録されたメニューはストアに保持しているだけです。ブラウザリロード等で消えますのであしからず。

Observableを購読したままにしない!

これはRxJSを採用しているプロジェクトならやりがち大定番ですね。でも知らないと大量発生する厄介者です。
Observableは購読(subscribe)したあと何もせず放置しておくとメモリに残り続け、メモリリークの原因になります。
そのため、購読した後に解除の処理を入れてやる必要があります。
でもこれって知らないと対処しないですよね。筆者が関わるプロダクトでも既存のコードに解除の処理はありませんでした。後になって購読解除の必要性を知り、購読解除を入れる修正をしまくった苦い経験があります。

購読解除しよう

主に2つの方法があります。

  1. 明示的にunsubscribeする
  2. takeUntilオペレータを挟みこむ

まずは1から。ObservableをsubscribeするとSubscriptionが返されるので、解除用のプロパティに突っ込んでunsubscribeする方法。

export class AddComponment implements OnInit, OnDestroy { 
  // 抜粋
  private subscription!: Subscription

  onAdd() {
    const input: AddMenuInput = {
      name: this.name(),
      recommendation: this.recommendation(),
      createdBy: this.createdBy(),
    }
    this.subscription = this.menuService.add(input)
    .subscribe(() => {
      this.added.emit()
    })
  }

  ngOnDestroy(): void {
    this.subscription.unsubscribe()
  }
}

次に2の方法。まとめて脳死で対応できるため、普段はこちらを採用しています。

  • 購読解除用のSubjectを用意
  • 各Observableのpipeの中でtakeUntilオペレータを挟みこむ
  • 購読解除用のSubjectをnextすることで、まとめて購読解除
export class AddComponment implements OnInit, OnDestroy { 
  // 抜粋
  private readonly onDestory$ = new Subject<void>()

  onAdd() {
    const input: AddMenuInput = {
      name: this.name(),
      recommendation: this.recommendation(),
      createdBy: this.createdBy(),
    }
    this.menuService.add(input)
    .pipe(takeUntil(this.onDestory$))
    .subscribe(() => {
      this.added.emit()
    })
  }

  ngOnDestroy(): void {
    this.onDestory$.next()
  }
}

今後はもっと購読解除が楽になりそう?

上で紹介した方法は必要とはいえ、ボイラープレートが増えてめんどくさいですよね。
それをより楽にすべく、Angular v16から導入されたSignals関連のAPIにtakeUntilDestroyedオペレータがあります。
takeUntilDestroyedを紹介した記事によると、コンポーネントのngOnDestroyで書く処理を省略できる他、サービスやディレクティブでもお手軽に購読解除できるようです。
とはいえ、injection context(constructorやプロパティ)以外での使用はDestroyRefを挟み込まないといけないので、減る手間はngOnDestoryを書かなくてよいことぐらいか。

export class AddComponment implements OnInit { 
  // 抜粋
  private readonly destroyRef = inject(DestroyRef)

  onAdd() {
    const input: AddMenuInput = {
      name: this.name(),
      recommendation: this.recommendation(),
      createdBy: this.createdBy(),
    }
    this.menuService.add(input)
    // injection contextであればdestroyRefは不要
    .pipe(takeUntilDestroyed(this.destroyRef))
    .subscribe(() => {
      this.added.emit()
    })
  }
}

仕様が確定したらこっちに切り替えていきたいですね!

is~プロパティ(状態フラグ)を増やさない!

コンポーネントに限らずですが、状態を手っ取り早く表現するために、is~(例:isOpened(開いているかどうか))プロパティを生やしがちです。そのコンポーネントの要件が変わらず、絶対にtrue/falseの2つの状態で表現できるなら採用してもよいでしょう。
しかしこの世は諸行無常、要件は変わっていくものです。今回のメニューアプリで考えてみましょう。
現在、メニューの登録はできますが編集機能がありません。これではメニュー名を間違えていたので直したいとなったとき、削除して登録しなおす必要があるため、それまでの投票数が消えてしまいますね。不足を補うべく、編集機能を追加しましょう。編集の場合の要件を以下とします。

  • 登録済みのデータを取得して各フォームにセットする
  • 投稿者フォームは変更不可(disabled)にする

実際に作成したUIがこちら。

編集画面(ダイアログ)

さて、UI自体はほとんど変わらないため、登録で使用したコンポーネントを再利用すべくisEditプロパティを追加しました。isEditがtrueの場合のロジックを追加します。たとえば2つ目の変更不可要件を達成しましょう。

// component.ts
// 登録用コンポーネント(AddCompooent)→ FormsComponentに名称変更
export class FormsComponent {
  // 追加
  isEdit: boolean = false
  
  // 処理が続く…
}

// component.html(抜粋)
  <mat-form-field color="accent">
    <mat-label>誰が登録した?</mat-label>
    <mat-select
      [value]="createdBy()"
      (selectionChange)="createdBy.set($event.value)"
      // 追加
      [disabled]="isEdit"
    >
      @for (family of families; track $index) {
        <mat-option [value]="family">{{ family }}</mat-option>
      }
    </mat-select>
  </mat-form-field>

ここまではよさそうです。
ときは少し流れて、新たな要求として「投稿者以外が編集できると困るので、詳細を確認できるだけの画面がほしい」となりました。画面を新たに用意すると工数がかかるため、さらにコンポーネントを流用してフォームをすべて変更不可(readonlyまたはdisabled)にすることで対応すべく、isDetailプロパティを生やしましょう。

// component.ts
export class FormsComponent {
  @Input()
  isEdit: boolean = false
  // 追加
  @Input()
  isDetail: boolean = false
  
  // 処理が続く…
}

// component.html(抜粋)
    <mat-form-field color="accent">
      <mat-label>メニュー名</mat-label>
      <input
        type="text"
        matInput
        required
        #nameControl="ngModel"
        [ngModel]="name()"
        (ngModelChange)="name.set($event)"
        // 追加
        [readonly]="isDetail"
      >
      @if (nameControl.hasError('required')) {
        <mat-error>メニュー名は必須</mat-error>
      }
    </mat-form-field>
  
    <mat-form-field color="accent">
      <mat-label>推薦文</mat-label>
      <textarea
        type="text"
        matInput
        #recommendationControl="ngModel"
        [ngModel]="recommendation()"
        (ngModelChange)="recommendation.set($event)"
        // 追加
        [readonly]="isDetail"
      ></textarea>
    </mat-form-field>

    <mat-form-field color="accent">
      <mat-label>誰が登録した?</mat-label>
      <mat-select
        [value]="createdBy()"
        (selectionChange)="createdBy.set($event.value)"
        // 追加
        [disabled]="isEdit || isDetail"
      >
        @for (family of families; track $index) {
          <mat-option [value]="family">{{ family }}</mat-option>
        }
      </mat-select>
    </mat-form-field>

臭くなってきましたね~。さらに要件が増えて、メニューに採用済み(isAdopted)の状態が増えた場合は…?

状態フラグは1つで2つの値(状態)を持ちます。フラグが2つに増えたら2 * 2 = 4つの状態、3つに増えたら2 * 2 * 2 = 8つの状態を考慮しなくてはなりません。指数関数的に増えていきます。最悪です。保守したくありませんね。
ここで強調しておきたいのは、状態フラグを使うなということではありません。要件の見通しがたつシンプルなコンポーネントであれば、状態フラグで十分なこともあります。
邪悪なのは「要件を整理せず」に「とりあえず」状態フラグを使用してしまうことです。特にあとから変更でフラグを加えていくのはスパゲティコードを生み出す原因です。やめておきましょう(過去の自分への戒め)

状態を型と値で表現しよう

ではどうすべきか。TypeScriptには便利な機能があるじゃないですか。さきほどの要件の状態を表現してみましょう。

export type MenuFormState = 'ADD' | 'EDIT' | 'DETAIL' | 'ADOPTED'

export class FormsComponent {
  @Input()
  state: MenuFormState = 'ADD'
  
  // 処理が続く…
}

そうです、ユニオン型です。これで状態を4つの型(値)で表現することができました。それどころか、受け取るプロパティも1つになりました。状態が追加された場合も、ユニオン型に追加するだけで済みます。追加のロジックもその型に応じたものを実装するだけです。はるかに保守しやすくなります。
一見複雑にみえる要件も、整理してあげれば状態を区分けして命名することが大抵の場合可能です。さらに踏み込んだ話をすると、状態を区分けすることで、その状態間の遷移にルールを設けることだってできます。
コンポーネントを設計する際は、要件からいくつの状態に区分けできるのか、その状態遷移に制約(条件、ルール)があるのかを意識すると、少しハッピーになれるかもしれません。

状態ごとにコンポーネントを分けよう

別の手段として、コンポーネントを分けてしまう手もあります。薄いコンポーネントなら1つの中に複数の状態を持っても気になりませんが、複雑なコンポーネントの場合は既存のコードを読むだけで大変です。
この場合は潔く状態ごとに別のコンポーネントを作ってしまいましょう。工数はかかりますが、単体のコンポーネントの保守はしやすいため、トータルでみるとこっちの方がコストが低いかもしれません。状態ごとに共通する部品があるなら、共通コンポーネントを作ってあげればよいでしょう。

どの手段をとるか悩ましいですが、設計の楽しい部分でもあり、開発者の腕の見せ所でもありますね!

無秩序にプロパティを増やすのはやめよう!

先の状態フラグと似た話ですが、プロパティを増やすと、コンポーネントはそれだけ複雑になっていきます。プロパティとはつまり状態です。状態を増やせば増やすほど、その管理が大変になります。
特にそれが変更可能な(readonly修飾子がない)場合顕著です。そのプロパティがいつ・どこで変更されるのか、コードを読む間いつも気にかけてなくてはなりません。メソッドのなかでそれらプロパティを参照している場合、メソッドの実行結果の一貫性を保証できません。プロパティが変われば結果も変わり得るからです。結果を読み解くのが大変になります。
無秩序にプロティが増えてきたなら、それはきっとデータフローと関連を整理できていない兆候です。

そのプロパティ、リレーのバトン置き場にしてない?

一番厄介なプロパティの増え方がこれです。本来はデータフローの中にある値を、「いったん」プロパティに格納するパターンです。実装を検討・試行錯誤する間はかまいませんが、必ずリファクタリングしましょう。
サンプルアプリとは別のケース(filter: 検索フィルターとでもしましょう)で考えます。以下は極端な例ですが、子からイベントで値受け取り→発火したメソッドでプロパティに格納、別のメソッド呼び出し→別のメソッドでプロパティ参照 という流れです。いわばプロパティが「バトン」ですね(近いことやってた過去の自分やばすぎい!)

// html
<child-component (changeFilter)="onChangeFilter($event)"></child-component>

// component
filter: Filter = { /** 初期値 */ }

onChangeFilter(filter: Filter) {
  this.filter = filter
  // 何か他の処理
  // ...
  this.getList()
}

getList() {
  this.searchService.getList(this.filter)
  // ...
}

これを改善するのに手っ取り早い方法は、関数の引数で値を受け取ることです。

// html
<child (changeFilter)="getList($event)"></child-component>

// component
getList(filter: Filter) {
  this.searchService.getList(filter)
  // ...
}

でもわざわざプロパティに格納していたということは、他のか所でも参照してるんじゃないの?おそらくその疑問は当たっています。この場合に、もしfilterの値の変更によって呼び出したいいくつかの処理があるなら、データフロー(パイプラインとも)にまとめるチャンスです。

// html
<child (changeFilter)="filter$.next($event)"></child-component>

// component
readonly filter$ = new BehaviorSubject<Filter>({ /** 初期値 */ })

ngOnInit() {
  this.filer$
  .pipe(
    // データの変換とか
    // 他のObservable合成とか
    concatMap(filter => this.searchService.getList(filter))
  )
  .subscribe({
    next: () => {
      // 正常系の処理
    },
    error: () => {
      // 異常系の処理
    }
  })
}

RxJSのBehaviorSubjectを使って変更を検知し、データフローを構築しました。それを購読するなかで、様々な処理を挟み込むことが可能です。「フィルターの値が変更されたら」「何を行うか」、その意図がより明確に、しかも1つのフローの中で完結しましたね。

(余談1)RxJSは怖くないよ

敷居が高く敬遠されてそう?なRxJSですが、使いこなせばとっても便利なライブラリです。非同期処理のみならず、様々な処理をエレガントに書きこなすことができます。上で書いたように、pipeのなかで起きたエラーをキャッチすることもできます。関心をぎゅっと1つにまとめるのに大いに役立ちます。
一般的なプログラミングパラダイムと異なるため難しい点はありますが、使いこなせばなにより楽しいです。こんなこともあんなことも優雅に実現可能です。ぜひお友達になりましょう。

(余談2)今後はSingalsでデータフローが構築できる?

サンプルアプリの機能を元にSignalsを使ってメニューを取得する例を書いてみました。親からIDを受け取って子でメニューを取得するイメージです。

export class MenuComponent {
  @Input({ required: true })
  id!: WritableSignal<number>

  private readonly menuService = inject(WishMenuService)
  
  readonly menu = computed(() => toSignal(this.menuService.get(this.id())))
}

実際に非同期通信で試していないため確かなことは言えませんが、RxJSを使わずともリアクティブにフローを構築できそうだなーと感じています。期待とワクワクが止まりませんね!

関連するプロパティはまとめよう

ただバラバラにプロパティを列挙するのではなく、関連するプロパティはオブジェクト等にまとめてしまいましょう。例を挙げると、

  • コンポーネントの初期表示時に取得するUIのためのデータで、その後不変なもの
  • ユーザー操作によって同じタイミングで変更されるデータ群

などがあります。まとめると何がよいのでしょうか?データのまとまりに意味と名前が与えられ、後から意味合いをくみ取りやすくなることが最大のメリットでしょう。先ほどのFilterの例で考えてみましょう。filterの中身が1つ1つのプロパティに分かれていたとします。

// component

// 元のプロパティ
// filter: Filter = { /** 初期値 */ }

name: string = ''
description: string = ''
status: Status = ''
// ...他のフィルターが続く

getList() {
  const filter = {
    name: this.name,
    description: this.description,
    status: this.status,
    // ...他のフィルターが続く
  }
  this.searchService.getList(this.filter)
  // ...
}

「さすがにこれはやらんやろ」という極端な例ではありますが、Filterという概念(モデルといっていいかも)が発見できていない場合、意図せずやってしまうケースがあります。そのときは気づいていなくても、他人から指摘された、あるいは後から自分で見て気づくこともあるでしょう。「このプロパティたちは1つの言葉で表せないか?」を常に意識しておくと、後から読んだ人(自分含めて)を助けることでしょう。

心の中で問いかけよう

プロパティが増えてきたなと感じたら、以下を心の中で問いかけながらデータをチェックしましょう。

  • そのプロパティ、状態の一時置き場ではないか?
  • 関数の引数・戻り値にできないか?
    • データの変換メソッドはPipeを使うのも手
    • 関数の戻り値・引数にできるなら、データフロー(パイプライン)にできないか?
  • readonlyにできないか?(不変なら考えることが減る)
  • 同じ名前の下にまとめられないか?

プロパティ(=状態)を減らしてスマートなコンポーネントを心がけましょう!

表示する側と中身を一緒にしない!

「表示する側」とは、遷移して表示するページや何かの操作によって開くダイアログ等を指します。表示する「媒体」と呼んでもいいでしょう。これらを一緒にすると何がマズいのでしょうか?

  • 中身を再利用できない
  • 同じ媒体でも表示する(呼び出される)か所によって、処理や与えたいデータが異なる可能性がある

ことが理由です。サンプルアプリで考えてみましょう。
先に示したように、今回はメニュー登録UIをダイアログで実装しました。しかし、スマートフォンのブラウザではダイアログの操作性がよくないため、スマートフォン用に登録UIをページで表示することにしました。このとき、側と中身のコンポーネントを分けておかないと、同じような中身を含んだコンポーネントをもう一つ量産することになります。これだと登録ロジックや登録データに変更があった際、2か所ともに修正を加えなくてはなりません。保守がめんどくさいですね。

export class AddDialogComponent {
  // フォームのプロパティ
  // 登録処理
}

export class AddPageComponent {
  // ここにもフォームのプロパティ
  // ここにも登録処理
}

他にも、同じページでも遷移元が異なれば、登録処理完了後の遷移先が異なる可能性もあります。分岐処理を書けば済む話かもしれませんが、バッドスメルが立ち込めていませんか?他にもページを表示するか所が増えたら?分岐処理の中身が複雑でコードの見通しが悪くならない?…ならないと言い切れるなら、側と中身を一緒にしてもいいかもしれません。

export class AddPageComponent {
  onAdd() {
    const input: AddMenuInput = {
      name: this.name(),
      recommendation: this.recommendation(),
      createdBy: this.createdBy(),
    }
    this.menuService.add(input)
    .pipe(takeUntilDestroyed(this.destroyRef))
    .subscribe(() => {
      if (遷移元が○○ページなら) {
        // ○○ページへ遷移
      } else if (遷移元がxxページなら) {
        // xxページへ遷移
      } else {
        // TOPページへ遷移
      }
      // さらに増える可能性…??
    })
  }
}

側と中身はコンポーネントを分けよう

最初から分けて作りましょう。これに尽きます。Atomic Deasignまでいくとやり過ぎ感ありますが、関心の見極めと分離は大切です。
中身のコンポーネントは中身の処理に集中し、側のコンポーネントは表示媒体を司り、表示前後のコントロールを担うべきです。中身は同じでも、表示場所が変わる・増える、表示媒体が変わることは往々にしてあります。

export class AddComponent {
  // 処理(の完了)はイベントで通知
  @Output()
  readonly added = new EventEmitter<void>()
  @Output()
  readonly cancel = new EventEmitter<void>()

  // フォームのプロパティ
  // 登録処理
}

@Component({
  standalone: true,
  imports: [
    CommonModule,
    AddComponent,
  ],
  template: `
    <app-add
      (cancel)="なにか処理"
      (added)="なにか処理"
    ></app-add>
  `,
})
export class AddDialogComponent {
  // イベントを受け取った後の処理に責務をもつ
  // ex ダイアログを閉じる
}

@Component({
  standalone: true,
  imports: [
    CommonModule,
    AddComponent,
  ],
  template: `
    <app-add
      (cancel)="なにか処理"
      (added)="なにか処理"
    ></app-add>
  `,
})
export class AddPageComponent {
  // イベントを受け取った後の処理に責務をもつ
  // ex 別のページへ遷移する
}

書く場所が分かれるため、記述量が増えて非効率に感じるかもしれません。しかし責務が分かれてそれぞれのロジックも薄くなる分、ユニットテストも書きやすくなります。ここではテストの詳細や方針について掘り下げませんが、テスト書きやすい=保守しやすいといっても過言ではないでしょう。

まとめ

  1. コンポーネントの状態を見極めよ!
  2. 状態に基づいて責務と関心を分けよ!
  3. v17の機能追加・改善いいかんじ!

状態、責務、関心…。古今東西、いくつもの書籍・記事で論じられているテーマですが、プロダクト開発で身をもってその大切さを痛感しているところです。2年前の自分が今回の内容を理解していれば、今の自分はもっと楽な思いをしていたでしょう。でも過去があるから今があるのです。人は痛みを乗り越えて育っていくのです。

今回、v17でサンプルアプリを作ってみた感触として、追加・改善された機能たちがいいかんじですね!開発者体験がよくなりそう!
年末年始、時間がある方はv17を触ってみてはいかがでしょうか?

ここまで読んでいただき、ありがとうございました。

Angular Advent Calendar 2023 21日目は@ngsmvnさんです!

Discussion