🥰

弱いフロントエンドエンジニア組織のための開発ルール

2024/07/14に公開

労力は最低限に、だけど、品質はそれなりに保てる開発ルールはどうしたらいいんだろう?考えたので、まとめたいと思います。

結論からすると

  • 着手する前にタスクを分解し、チームに見えるようにしておくこと
  • PRの差分はできる限り小さくすること
  • Container/Presentational Componentと分離しておくこと
  • data-testIdは使わないこと

を基本ルールとしました。
(最後だけ粒度がめちゃめちゃ小さいですが、蔓延っていて私のヘイトが溜まっていました)

"弱い"の定義から、それぞれの詳細を述べていこうと思います。

"弱い"とは?

※ここでは非難する意図は一切ないです

さまざまな事情で、開発に集中できなかったり、アーキテクチャ等を検討する余裕がなかったりする組織・エンジニアを指して、私自身のことも含めそう総称させてもらっています。

  • 家庭の事情により、業務時間以外で開発について知識を深ぼることができない
  • 職業エンジニアとして求められる成果を出し続けることを重要視している
  • 機能リリースや緊急対応ばかり求められる組織で、品質などに時間をかけられない

など、さまざまな事情があり、エンジニアとして常に成長ばかりを追い続けられないことは大いにあると思います。
私自身もフロントエンドエンジニアを主軸にしながらも、デザイナーとのコミュニケーションを得意領域にしていたり、組織の背景などもありながら、正直エンジニアとして邁進していくタイプではないです。

着手する前にタスクを分解し、チームに見えるようにしておくこと

開発に限らず仕事は全て見せながら行うを基本とすると良いと考えています。

  • 進捗が明らかなため、上司などが様々な調整が行いやすい
  • 遅れや問題が発生した場合、誰かに助けを求めやすい
  • 早い段階・小さな単位での不具合修正・仕様変更となるため工数を最小限に行える

と、開発者自身のことを守りながら開発をするための、第一歩になると思います。

PRの差分はできる限り小さくすること

10 lines of code = 10 issues.
500 lines of code = "looks fine."
Code reviews.

行数が大きければ多いほど、レビュワーの負担が増し、妥当なレビューを行うことはできません。
レビューで見つけられたはずの不具合や考慮漏れが、すり抜けていきます。

またコンフリクトの可能性や、大きな変更を残しているとい実装者の心理的な負担も減らせます。

小さくするには

  • 機能の独立性の鑑定
  • アーキテクチャ的にどこで分離できるかという観点

が必要かと思います。(これは上記のタスク分解ともつながります)

Container/Presentational Componentと分離しておくこと

弊社ではReactを採用しているため、その前提でお話しします。
コンポーネントの実装については、いろんな意見があるかと思います。
しかし、いずれにしてもこれを理解していないと、Reactを正しく利用できないのではないかと思い、採用しました。
何より、

  • 初学者であっても再現が可能である
  • storybookとの相性が良い
  • HTML・CSSが理解できるデザイナーであれば、コードからある程度イメージしやすい

というメリットが魅力的です。

さらに、コンポーネントだけではなく、PresenterViewと層を分けています。

View層

view=tsxです。ロジックは持ちません。

Container Component

一般的には、アプリケーションのロジックに関心を持ち、APIやstateに依存するコンポーネントのことを指します。

ここでは、Presenterに依存し、Presentationalコンポーネントにpropsを利用して値を受けたわす役割を持つコンポーネントを指しています。

Presentational Component

UIに関心を持つコンポーネントです。

stateなどには一切依存せず、propsによって何を表示するかを決める役割を持っています。

このコンポーネントをStorybookに表示・テストを行っています。

Presenter層

ロジックに関心を持つ層です。
APIの呼び出し、状態管理ライブラリ、ローカルStateを持ち、Container Componetへ値を渡します。

Viewに必要な値の変換(例えば、値が存在しているかどうかを、Booleanのフラグへと変更する)なども、この層で行います。

jestなどを利用したロジックの単体テストは、このPresenterに対して行うことになります。

実装例

usePresenter.ts
export const usePresenter = () => {
  const userName = useSelector(state => state.user.userName)

  const [open, setOpen] = useState(false);
  const onOpen = useCallback(() => {
    setOpen(true);
  }, []);
  const onClose = useCallback(() => {
    setOpen(false);
  }, []);

  return {
    userName,
    open,
    onOpen,
    onClose,
  };
};
View.tsx
// Presentational Component
// storybookのためにexportを行うが、storybook以外の他ファイルからは呼び出されない
export const UserNameView: React.FC<{
  userName?: string;
  open: boolean;
  onOpen: () => void;
  onClose: () => void;
}> = ({
  userName,
  open,
  onOpen,
  onClose,
}) => {
  if(!userName)
    return <Typography>no name</Typography>;

  return (
    <>
      <Typography>{userName}</Typography>
      <Button onClick={onOpen}>編集</Button>
      <EditDialog open={open} onClose={onClose} />
    </>
  );
};

// Container Component
// usePresenterから値を取得し、Presentational Componentに値を渡すだけ
export const UserName: React.FC = () => {
  const { userName, open, onOpne, onClose } = usePresenter();

  return (
    <UserNameView
      userName={userName}
      open={open}
      onOpen={onOpen}
      onClose={onClose}
    />
  )
}

メリット

冒頭に述べたメリットも含め、改めてまとめます。

初学者であっても再現が可能である

ある程度テンプレート化できるので、Reactへの習熟度関係なく同様の分離が可能です。
最悪 「コンポーネントを返すところ以外は、全てusePresenterに入れて!」 という、雑なコミュニケーションでも済んでしまいます。

storybookと相性が良い

storybookをカタログとして利用するほか、ビジュアルリグレッションの役割を持っています。
当然テストであるため、UIがとりうる全てのパターンのstoryが必要となります。

コンポーネント内にstateを持つ
export const Component: React.FC = () => {
  const [flag, setFlag] = useState(false);

  return (
    <>
      <Button onClick={() => {setFlug(true);}}>編集</Button>
      {flag && <ComponetnA />}
      {!flag && <ComponetnB />}
    </>
  )
}

こういったコンポーネントでは、ComponentBをstoryとすることはできますが、ComponentAをstoryとするには工夫が必要です。

しかしPresentational Componentであれば、propsでフラグを渡すことができるため、ComponetnA,Bともにstoryとすることができます。

ロジックのテストが書きやすい

usePresenterの中にロジックがまとまっているため、単体テストとして書きやすくなります。
コンポーネントのテストはフロントエンド特有のものであり、バックエンドの開発を主に行っているメンバーとしては馴染みが薄いです。
ロジックのテストに集中できるということは、メンバーが流動的であるこの組織ではメリットであると感じています。

保守性が高い

ロジックの変更があった場合にも、usePresenterの返り値に変更がなければ、UIへの変更がないことが保証されます。
CIで保証するのはもちろんですが、人間が変更を加える・レビューを行う場合においても安全に行うことが可能です。

タスク分解・役割分担が行いやすい

presenterとviewとで分かれていることで、自然と関心が分離されるため、タスク分解や役割分担が可能になります。

  • まずはロジックを書き、レビューをしてもらう。その間に、viewを作成する。
  • デザイナーがviewを作成し、エンジニアがロジックを書く。

など、進め方も柔軟になり、それぞれの得意領域による役割分担も可能になります。

data-testIdは使わないこと

testing-libraryには以下の記述があります。

In the spirit of the guiding principles, it is recommended to use this only after the other queries don't work for your use case. Using data-testid attributes do not resemble how your software is used and should be avoided if possible.

https://testing-library.com/docs/queries/bytestid/

基本的には使わないほうが良いとされています。
しかし既存コードではよく散見されており、その中でもIconButtonを探し出すためにdata-testidを付与しているパターンが多かったです。

そこで、上から順番に対応をとるようにしています。

  • ByTextを使用する。一意にならない場合は、selecterも併用して良い。
  • MUIと書き方が乖離していないか確認し、必要であれば修正する
    MUIはアクセシビリティも考慮されており、書き方を合わせることで改善される場合も多いです。
  • aria-labelを付与し、ByLabelを使用する
    testing-libraryを使用して要素にアクセスできないということは、読み上げソフトなどで「どんな要素なのか」を伝えることができない場合が多いです。
    そのためaria-labelなどアクセシビリティ対応を追加することで、テストを可能にするとともに、アプリケーションのアクセシビリティも向上させることができます。
  • 上記の対応が取れない場合のみ、data-testidを利用する
    どうしてdata-testidでなければならないのか、記入してもらうようにしています。

まとめ

長くなりましたが、上記ルールは

  • 習熟度などに関わらず、誰でも守りやすいルールにする
  • 開発者の悩み・苦労を減らす

を念頭におきながら作りました。
組織で開発をしている限り、エンジニアのレベル、それぞれの特領域は様々だと思います。
そんな中でそれぞれの得意な領域を生かし、プロダクトの価値を高める活動へ時間が捻出できる。そんなチームを目指しています!

Discussion