こんにちはwanoでフロントエンドエンジニアをさせていただいています。@Kosanです。

この記事はWano Group Advent Calendar2022 19日目の記事です。

弊社に転職してほぼ一年が経とうとしております。
この一年で、自分が記述したコードに対して沢山のアドバイスをいただきました。
せっかくなので今までいただいたレビュー内容を振り返っていきたいと思います。

分類

①変数・関数の命名について約40件
②既存のFE構成のルールやコンポーネントの責務について約30件
③TSやReactなど技術的なアドバイス約30件
(※集計期間2022/2月〜2022/9月のPR 　　総数　約120件)

(実際にレビューいただいた内容をまとめた際の私のNotion)

変数・関数の命名について

レビューを見返してみると、変数・関数の命名についての指摘が一番多かったです。
特に、入社してから最初の3ヶ月が特に多かった印象です。

例)

変数名の先頭が動詞or名詞の違いで受け取り方が違って見えるケースがあるので
変数の最初の単語の品詞が重要になってくる場合があります。

(捉え方は人それぞれですが)
先頭の単語が動詞にすることで、「関数っぽい」命名になりました。

const valitateStatus = ...

変数なので先頭を名詞にするよう気をつけます。

const valitationStatus = A

またフックなどからコンポーネントに関数を受け渡す際も
UIコンポーネント側から見た時に判別がつきやすい変数名にするとなお良いです。

クリック時の挙動などをフックから呼び出して、UIコンポーネントに直接受け渡していました。
この書き方だとUIのボタンが複数ある場合に本当に適切な関数なのかがコードから判断しづらくなってしまいます。

//container.ts
const useContainer = () => {
 const { openModal } = useHogeHook()
 return { openModal }
}

//App.tsx
 const { openModal } = useContainer()

　　return 
  <>
   <RedButton onClick={openModal}>
   <BlueButton onClick={openModal}>
  </>

挙動を示す関数をUI側でぱっと見で判別できるような関数名でラップしてあげることで、
どのコンポーネントにどの関数を渡せばいいかが一目瞭然となりました。

//container.ts
const useContainer = () => {
 const { openModal } = useHogeHook()

 const onClickRedButton = () => {
   openModal()
 }

 return { onClickRedButton , onClickBlueButton }
}

//App.tsx
 const { onClickRedButton , onClickBlueButton } = useContainer()

　　return 
  <>
   <RedButton onClick={onClickRedButton}>
   <BlueButton onClick={onClickBlueButton}>
  </>

上記のように、関数や変数を受け取った側が使いやすいかどうかを意識した
記述にすることでコードの見た目が改善されました。

既存のFE構成のルールやコンポーネントの責務について

FEの既存の構成が理解できておらず、アドバイスいただくことも多いです。
こちらは先ほどの項目とは逆で、PRを重ねれば重ねるほどアドバイスの量が増えていっている気がします。

例)
Open APIなどのような外部の型やUIライブラリは、
アプリケーションに入ってくる一番手前でFEで使い回すための型などに変換すると、保守性が高まります。

APIを叩いて情報を取得する際、外部の型をリターンして、ドメインロジックでそのまま利用するケースがありました。
こうするとドメインロジックが外部のライブラリや型に依存する形となってしまい、テストがしづらく、外部のものの変更に弱くなってしまいます。

import { HogeType } from "@hoge/openapi";

const fetchData (): Promise<{data: HogeType}> => {
  await axios.get<HogeType>()
   .then((res) => {
     return { data: hogeType }
   })
}

APIを叩く関数内で、FE用の型に変換してリターンするように修正しました。
こうすることで外部に変更が加えられても、ドメインロジック内ではFEで定義した型を利用するため
影響範囲が少なくなります。

//fetcher.ts
import { HogeType } from "@hoge/openapi";

const fetchData (): Promise<{data: FEHogeType}>=> {
  await axios.get<HogeType>()
   .then((res) => {
     return { data: convertHogeTypeToFEHogeType(hogeType) }
   })
}

//container.ts
const handleClick = () => {
  await fetchData() 
}

UIコンポーネントで利用するカラーやサイズなどは、
できるだけ既存のデザインシステムに定義した色から優先的に利用します。

当時はデザインシステムで定義されていないカラーを利用していました。

//index.tsx
return ( 
  <div className={"#EEEEEE"}></div>
)

今は下記のような流れに従ってカラーを定義しています。
①デザインに似たような色が既存のデザインシステムにあるか確認
②存在する場合はその色を利用する。
③存在しない場合は、新たにコンフィグ内に定義して利用。

//tailwind.config.js
module.exports = {
  theme: {
    extend: {
      colors: {
        red: { 100: #EEEEEE }
      }
    },
  },
};

//index.tsx
  return <div className={colors-red-100}></div>

終わりに

今までいただいたコードレビューを振り返えってみました。
改めて振り返ってみると、「あの頃できてなかったことが今できるようになってる！！」
みたいなことに気づくことが多くすごくポジティブな気分になりました。

反面、序盤からアドバイスいただきつつ、現在もまた、、なんて部分も多くありましたので
早めにキャッチアップできるように精進していきたいと思います。

改めてお時間とってレビューいただける環境に感謝しつつ、来年も楽しんでいきたいと思います。

転職して一年が経つので、今までに頂いたコードレビューを振り返ってみた

分類

変数・関数の命名について

既存のFE構成のルールやコンポーネントの責務について

終わりに