【2023/1/29 ~ 2023/6/30】コードレビューで知ったことをざっくりまとめてみた part1

作った機能

フロントとバックエンド両方実装した。

注文履歴ページ
注文後の確認メールを送信する(メール自体はBeeProで作った。あとでslimで書き換えたけど)
キャンセル機能
注文詳細ページ

Rails

full_nameメソッドをモデルに実装する

full_nameは結構頻繁に使う汎用的な処理なので、モデルにメソッドとして定義した方良いです。モデルに書いたほうが変更箇所がメソッド内の一箇所に済むので、その点でも良いです。全てのメール送信処理において、notice_pre_registrati-というアドレスに送るわけではないので、使う側でどこに送りたいのかを指定します。

zip_codeとかも同様

  def full_name
    "#{last_name}#{first_name}"
  end

  def format_zip_code
    zip_code.gsub(/(\d{3})(\d{4})/, '\1-\2')
  end

アドレスはハードコーディングせずに設定で持った方が良い

global/mail.yml にもつか、ApplicationMailer に定数で持つ。今回の場合は、本番に応じて出しわけするので、ApplicationMailerにメソッドとして定義した。

↓ before

class ApplicationMailer < ActionMailer::Base
  default from: Global.mail.from
  layout "mailer"
end

class PreRegistrationMailer < ApplicationMailer
  # @param [PreRegistration] pre_registration
  def on_receipted(pre_registration)
    bcc = if Rails.env.production?
            "notice_pre_registrati-aaaaiepp6speoeayhvmbzqnkqu@wcdi-inc.slack.com"
          else
            "dev_test_notice_all-aaaaitbrgahkajy3dygm6mait4@wcdi-inc.slack.com"
          end

    mail(
      to: pre_registration.email,
      bcc:,
      subject: "hogehoge",
    )
  end

↓ after

class ApplicationMailer < ActionMailer::Base
  default from: Global.mail.from
  layout "mailer"

  private

  def slack_channel_address(address)
    bcc = if Rails.env.production?
       address
    else
      "dev_test_notice_all-aaaaitbrgahkajy3dygm6mait4@wcdi-inc.slack.com"
    end
  end
end

class PreRegistrationMailer < ApplicationMailer
  # @param [PreRegistration] pre_registration
  def on_receipted(pre_registration)
    mail(
      to: pre_registration.email,
      bcc: slack_channel_address(Global.mail.pre_registration_address,
      subject: "hogehoge",
    )
  end

サービス内でメール送信処理を行う

サービスを流用する祭に、メール送信処理もまた書かないといけない、もしくは書き忘れて障害が起きる可能性がある。そのため、まとまった処理ならサービスにまとめて流用性を出す。 ↓ before

class Checkout::CompletePaymentService
  attr_reader :checkout, :user
  def initialize(user:, checkout:)
    @user = user
    @checkout = checkout
  end
  def execute!
    ActiveRecord::Base.transaction do
      checkout.payment.confirm!
      checkout.paid!
    end
  end
end

↓ after

class Checkout::CompletePaymentService
  attr_reader :checkout, :user
  def initialize(user:, checkout:)
    @user = user
    @checkout = checkout
  end
  def execute!
    ActiveRecord::Base.transaction do
      checkout.payment.confirm!
      checkout.paid!
      CheckoutMailer.on_receipted(user, checkout).deliver_later
    end
  end
end

対象のリソースを置き換えるなら、putにした方が適切

/checkouts/{id}/cancelというエンドポイントにおいて、checkoutの一部を書き換えるのでpatchだと思うが、エンドポイントを見ると対象のリソースがcancel(checkout.statusの状態が対象リソース)と考えられるので、その場合は、putの方が適切。確かにRailsの場合、リソースのupdateのエンドポイントは/photos/:idだから、この場合だとpatchが適切だな。

Class.newで独自エラーが作れる

     rescue Checkout::NotCancelableError
        render json: Response::Error.new(error: { code: "forbidden" }), status: :forbidden
      end

  after_create :update_coupon_status!

  NotFailableError = Class.new(StandardError)
  NotCancelableError = Class.new(StandardError)

  def calc_price!
    self.subtotal_price = checkout_items.sum(&:subtotal_price)
Expand Down
Expand Up
    @@ -112,7 +113,7 @@ def fail!
  end

  def cancel!
    raise NotCancelableError unless paid?

    canceled!
  end

cancel処理は、単純なステータス変更以外にも様々な関連モデルの変更が予想されるので、サービスを通して処理をする

before

      def cancel
        checkout = token_user.checkouts.find(params[:id])
        checkout.cancel!

        render json: Response::Success.new(data: {
          checkout:,

// checkout.rb
  def cancel!
    raise NotCancelableError unless paid?

    canceled!
  end

after

      def cancel
        checkout = token_user.checkouts.find(params[:id])
        service = Checkout::CancelCheckoutService.new(
          checkout:,
        )
        service.execute!

        render json: Response::Success.new(data: {
          checkout:,

class Checkout::CancelCheckoutService
  attr_reader :checkout

  def initialize(checkout:)
    @checkout = checkout
  end

  def execute!
    checkout.with_lock do
      raise Checkout::NotCancelableError unless checkout.paid?
      checkout.canceled!
    end
  end
end

フロントの都合でDBのスキーマを変更しない。必要ならシリアライザーを使う。

フロントの都合でDBのスキーマを変更すると、密都合になって変更に弱くなる。スキーマもハイコンテキストになってしまい、スキーマを見た人も、「なんでこんなカラムあるの？」ってなってしまう。必要なら、シリアライザーを使う。

バックエンドのモデルとフロントエンドのモデルは基本的には責務は同じだが、ちょっと責務が違う部分がある。

キャンセル可能かどうかを判定する

キャンセル可能かどうかをシリアライザーに追加した場合、常にキャンセル可能を返すのではく、すでにキャンセルされたか、注文が完了したのかを条件に入れる。

before

  def is_cancelable
    true
  end

after

  def is_cancelable
    if object.canceled? || object.completed?
      false
    else
      true
    end
  end

Rspec

フロントエンド

エクスポートパスをアルファベット順に記載する

リソースに応じたパス名をつける

orderというリソースは存在しないので、/order-historyではなく、/checkoutsにする。

都度ラップコンポーネントを用意するのを避けるために、

都度Shopify用のラップコンポーネントを用意するのは、特定のユースケースに応じたコンポーネントを作らねばならず辛い。そのため、Providerコンポーネントを自作した方が良い。

↓ before

export const ShopifyOrderProductCard: BFC<Props> = ({

↓ after

import { Product } from "shared/models";
import { BFC } from "shared/types";

import { useShopifyProduct } from "../../hooks";

type Props = {
  product: Product;
  children?: (product: Product) => React.ReactNode;
};

export const ShopifyProductProvider: BFC<Props> = ({
  product,
  children,
}) => {
  const { product: shopifyProduct, isLoading } = useShopifyProduct(product.shopifyProductId);

  if (isLoading || !children) {
    return <div />;
  }

  return <>{children(shopifyProduct)}</>;
};

インデントを無駄に作って、ネストが深くならないようにする

コードを読み解くのが難しくなるので、インデントを無駄に作ってネストが深くならないようにする。

以下の場合三項演算子で条件分岐させていますが、ネストが深くなるので、論理和演算子で書いた方が良い。

↓ before

    <div className="p-3">
      <h1 className="text-lg font-bold mt-5">注文履歴</h1>
      {
        checkouts.length > 0 ? (

↓ after

      {checkouts.length === 0 && (
        <div className="mt-5">
          <div className="overflow-hidden bg-white shadow sm:rounded-lg rounded border-gray-200 py-4">
            まだ商品を購入されていません
          </div>
        </div>
      )}
      {checkouts.map((checkout, i) => (
        <div key={i} className="mt-5">

DOM内でswitch文を使うようなトリッキーな記述をしない。

コンポーネント自体が長い記述になっている場合、適切に抽象化した方が良い。

                          {((shippingStatus) => {
                            switch (shippingStatus) {
                              case CheckoutShippingStatus.Yet:
                                return `${parcel.formatShippingDate()}に配達予定です`;
                              case CheckoutShippingStatus.Partially:
                                return `${parcel.formatShippingDate()}に一部発送済みです`;
                              case CheckoutShippingStatus.Completed:
                                return `${parcel.formatShippingDate()}に配達済みです`;
                              case CheckoutShippingStatus.Pending:
                                return "保留中です";
                            }
                          })(parcel.shippingStatus)}

汎用性がない粒度でモデルにメソッドを生やさない

すでにあるメソッドを修正するか、出力内容が汎用的な日時形式であれば共通ヘルパーを作る。 ↓ before

  formatCheckedOutAtToDate() {
    return formatDate(this.checkedOutAt, "yyyy年MM月dd日");

日時のフォーマットは、可能な限り統一したフォーマットにする

全アイコンで共通のclassNameを使う場合、渡す際にclassNameを指定して、classNameの指定がいろんな箇所に散らばらないように一箇所にする。あと、使う側の都合で決まる値は、使う方で指定する。

↓ before

const items = [
  { id: "top", label: "TOP", href: routes.TOP, icon: <HiOutlineShoppingBag className={classNames("mr-2 h-7 w-7")} /> },
  { id: "mypage", label: "会員情報", href: routes.MYPAGE, icon: <HiOutlineUser className={classNames("mr-2 h-7 w-7")} /> },
  { id: "order-history", label: "注文履歴", href: routes.CHECKOUTS, icon: <AiOutlineHistory className={classNames("mr-2 h-7 w-7")} /> },
] as const;

 {item.icon}

↓ after

const items = [
  { id: "top", label: "TOP", href: routes.TOP, icon: HiOutlineShoppingBag },
  { id: "mypage", label: "会員情報", href: routes.MYPAGE, icon: HiOutlineUser },
  { id: "order-history", label: "注文履歴", href: routes.CHECKOUTS, icon: AiOutlineHistory },
] as const;

  <item.icon className="mr-2" size="2rem" />

履歴のカードコンポーネントなら、CheckoutHistoryCardという命名にする

Cardと

StopPropagationとe.preventDefaultはjsやるならマスターしておいた方が良い

スマホ表示なら、カードの左右のスペースはいらない。表示面積が狭くなるだけだから。

ボタンとかなら左右のスペースあった方が良いけど、カードの場合、表示面積が狭くなるから、左右のスペースはいらない。

スマホとPCで表示を分けたいなら、tailwindのsmプロパティを使って頑張る

特定のユースケースに限定されたコンポーネントをなるべく作らない

例えば、product表示コンポーネントがあるのに、checkoutしたproduct用のproduct表示コンポーネントを作ってしまうと、特定のユースケースに限定されたコンポーネントができてしまう。特定のユースケースに限定されたコンポーネントが何個もできると、一回しか使わないコンポーネントができたり、どのコンポーネント使えば良いの？ってなりがちである。コンポーネントを組み合わせて問題を解決するコンポーネント思考の考え方からも遠ざかる。

↓ before(CheckoutProductCardを作っている)

              {parcel.items.map(({ product }) => (
                <Link key={product.id} href={routes.PRODUCTS_SHOW(product.id)} noDecoration>
                  <ShopifyProductProvider product={new Product(product)}>
                    {(product) => <CheckoutProductCard product={product} className="mb-2 sm:mb-4" />}

↓ after(ProductCardにマッピングしている)

              {parcel.items.map((item) => (
                <Link key={item.id} href={routes.PRODUCTS_SHOW(item.product.id)} noDecoration>
                  <ShopifyProductProvider product={new Product(item.product)}>
                    {(product) => (
                      <ProductCardWideRaw
                        key={product.id}
                        title={product.title}
                        image={product.featuredImage?.url}
                        price={product.priceRange.min}
                        quantity={item.quantity}
                        smallImage
                        className="mb-2 sm:mb-4"
                      />
                    )}
                  </ShopifyProductProvider>
                </Link>
              ))}

購入した際の金額はproduct.priceを参照するのではなくて、checkout_item.priceを参照した方が良い

productの価格を参照してしまうと、購入後にproductの価格が変わった際に「あれ、あの時はこの金額で買ったんだけどな」と混乱を起こしてしまう。

エラー表示よりカード表示の方が優先度が高いので、カード表示の方を上に書く

先に書いた方が早くレンダリングされるため。(おそらく)

↓ before

      {checkouts.length === 0 && (
        <div className="mt-5">
          <div className="overflow-hidden bg-white shadow sm:rounded-lg rounded border-gray-200 py-4">
            まだ商品を購入されていません
          </div>
        </div>
      )}
      {checkouts.map((checkout) => <CheckoutCard key={checkout.id} checkout={checkout} className="mt-5" />)}

↓ after

      {checkouts.map((checkout) => <CheckoutCard key={checkout.id} checkout={checkout} className="mt-5" />)}
      {checkouts.length === 0 && (
        <div className="mt-5">
          <div className="overflow-hidden bg-white shadow sm:rounded-lg rounded border-gray-200 py-4">
            まだ商品を購入されていません
          </div>
        </div>
      )}

keyに指定するためだけにidを保持しているなら、hrefを使う

ハイコンテキストなコードになってしまう(なんでidを持っているの？)ので、keyに使うためだけならhrefでも一位性を保証できる。

↓ before

    { name: "特定商取引法に基づく表示", href: routes.DOCUMENTS_TOKUSHOHO },
  ],
  Sns: [
    { id: "twitter", href: routes.SOCTOC_TWITTER, icon: AiOutlineTwitter },
    { id: "instagram", href: routes.SOCTOC_INSTAGRAM, icon: AiOutlineInstagram },
  ]
};

<li key={item.id}>

↓ after

    { href: routes.SOCTOC_TWITTER, icon: AiOutlineTwitter },
    { href: routes.SOCTOC_INSTAGRAM, icon: AiOutlineInstagram },

  <li key={item.href}>

コンポーネント特有の表示ならそこに書けば良い

これ自分で書いといてあれだけど、配達予定ですって部分の文言って他のカードでも使いそうだな。ヘルパーとかで共通化しても良かったかも。

export const CheckoutHistoryCard: BFC<Props> = ({ checkout, className }) => {
  const shippingStatusSentence = useCallback((parcel: CheckoutParcel) => {
    switch (parcel.shippingStatus) {
      case CheckoutShippingStatus.Waiting:
        return `${parcel.formatShippingDate(true)}に配達予定です`;
      case CheckoutShippingStatus.Partially:
        return `${parcel.formatShippingDate(true)}に一部発送済みです`;
      case CheckoutShippingStatus.Completed:
        return `${parcel.formatShippingDate(true)}に配達済みです`;
      case CheckoutShippingStatus.Pending:
        return "保留中です";
    }
  }, []);

  return (

RESTの原則に従うなら、変数名をDETAILSではなくてSHOWにする

before

CHECKOUT_DETAILS: (id: string) => `/checkouts/${id}`,

import { StandardLayout } from "~/components/layouts";
import { AuthedRoute } from "~/features/auth";
import { CheckoutDetailsPage } from "~/features/users";

const Page: NextPageWithLayout = () => {
  return (
    <AuthedRoute>
      <CheckoutDetailsPage />
    </AuthedRoute>
  );
};

after

CHECKOUTS_SHOW: (id: string) => `/checkouts/${id}`,

import { StandardLayout } from "~/components/layouts";
import { AuthedRoute } from "~/features/auth";
import { CheckoutShowPage } from "~/features/users";

const Page: NextPageWithLayout = () => {
  return (
    <AuthedRoute>
      <CheckoutShowPage />
    </AuthedRoute>
  );
};

これ明らかに他のところでも書くなっていう処理はヘルパーで定義する。

ヘルパーで定義することで、変更箇所が一箇所になるので、変更に強くなる。

// frontend/packages/shared/helpers/models/checkout_parcel.ts
import { CheckoutShippingStatus } from "shared/models/checkout";
import { CheckoutParcel, } from "shared/models/checkout_parcel";

export function shippingStatusSentence(parcel: CheckoutParcel) {
  switch (parcel.shippingStatus) {
    case CheckoutShippingStatus.Waiting:
      return `${parcel.formatShippingDateTime()}に配達予定です`;
    case CheckoutShippingStatus.Partially:
      return `${parcel.formatShippingDateTime()}に一部発送済みです`;
    case CheckoutShippingStatus.Completed:
      return `${parcel.formatShippingDateTime()}に配達済みです`;
    case CheckoutShippingStatus.Canceled:
      return "キャンセル済みです。";
    case CheckoutShippingStatus.Pending:
      return "保留中です";
  }
}

あるべきところにあるべきcssを書く。

cssを書くうえで大事なことをまとめる

無駄にcssを書きすぎない
親クラスにcss指定して継承できるなら、そうする。子供に同じcssを何回も書かない
あるべきところにあるべきcssを書く。ここのDOMにこのcssを指定すべきではないなってのがだんだんわかってくる。(いまだにちょっと悩むけど)
cssは、複雑化するとあっという間に崩れかけのジェンガみたいになるので、できる限りシンプルになるように書く

w-6/12 と w-2/5 が並んでいて、分母が変わってしまうと、一体残りがどれくらいあるのかわからなくなるので、こういう時は分母を揃える

無駄にhtmlをネストさせない

純粋にネストが深くなって見づらくなるため。ただ少なくしろって話ではなくて、必要ならちゃんと書く。

必要のないcssは書かない。

space-x-0 の指定は、ただの０なので、あえて明記する必要はない。

w-fitを使うことで、デザインがコンテンツの内容に依存してしまうのでなるべく使わない方が良い。コンテンツが変わっても変化しないデザインが良い。

before

sm:w-fit

after

sm:grid-cols-4

スマホに対応するためにsm:mt-0を書くDOMに書くのはしんどい。flex と gap-n gap-y-n で用いた方がシンプルにかける。

無茶苦茶シンプルになったな。

before

    <div className={classNames(className)}>
      <div className="overflow-hidden bg-white shadow sm:rounded-lg rounded border-gray-200">
        <div className="flex justify-between flex-wrap p-4 sm:p-6">
          <div className="flex space-x-0 flex-wrap sm:space-x-10">
            <div className="w-6/12 sm:w-fit">
              <h3 className="text-base font-medium leading-5">注文日</h3>
              <div className="text-base font-medium leading-5 text-gray-500">{checkout.formatCheckedOutAt(true)}</div>
            </div>
            <div className="w-6/12 sm:w-fit">
              <h3 className="text-base font-medium leading-5">合計</h3>
              <div className="text-base font-medium leading-5 text-gray-500">{formatPrice(checkout.totalPrice)}</div>
            </div>
            <div className="mt-3 w-6/12 sm:mt-0 sm:w-fit">
              <h3 className="text-base font-medium leading-5">お届け先</h3>
              <div className="text-base font-medium leading-5 text-gray-500 relative group">
                <div className="flex flex-col">
                  {checkout.shippingAddress.lastName} {checkout.shippingAddress.firstName}
                  <div>
                    <span className="mr-1">{checkout.shippingAddress.city}</span>
                    <span className="mr-1">{checkout.shippingAddress.town}</span>
                    <span className="mr-1">{checkout.shippingAddress.street}</span>
                    {checkout.shippingAddress.building}
                  </div>
                </div>
              </div>
            </div>
          </div>

after

    <div className={classNames(className)}>
      <div className="grid gap-2 overflow-hidden bg-white shadow sm:rounded-lg rounded border-gray-200 p-4">
        <div className="grid grid-cols-2 gap-4 text-base font-medium leading-5 sm:grid-cols-4">
          <div>
            <h3>注文日</h3>
            <div className="text-gray-500">{checkout.formatCheckedOutAt(true)}</div>
          </div>
          <div>
            <h3>合計</h3>
            <div className="text-gray-500">{formatPrice(checkout.totalPrice)}</div>
          </div>
          <div>
            <h3>お届け先</h3>
            <div className="text-gray-500">
              <div className="grid">
                {checkout.shippingAddress.lastName} {checkout.shippingAddress.firstName}
                <div>
                  <span className="mr-1">{checkout.shippingAddress.city}</span>
                  <span className="mr-1">{checkout.shippingAddress.town}</span>
                  <span className="mr-1">{checkout.shippingAddress.street}</span>
                  {checkout.shippingAddress.building}

Cardはリスト表示に使うコンポーネント。詳細画面はかなり個別のカスタマイズが入るので、あまり共通化するメリットがない画面である

詳細ページの中身をコンポーネント化しても汎用性がないので、普通にページコンポーネントにベタ書きすればよい。

ルーティングのコンポーネントでgetParamを実行すると、なんでここでid取得してんの？ってなって詳細を見に行くような、ハイコンテキストなコードになってしまうので、特に必要性がないなら、ページコンポーネント側に書く。

また、このコンポーネントはidを渡さないと使えないので、使う側がidを渡すか渡さないかに依存している。なので使いづらい。

before

export const getServerSideProps: GetServerSideProps<Props, Params> = async ({
  query,
}) => {
  const id = getParam(query, "id");

  return {
    props: {
      id,
    },
  };
};

export const CheckoutDetailsPage: BFC<Props> = ({ id }) => {
  const { checkout, isLoading } = useCheckout(id);
  const shippingStatusSentence = useCallback((parcel:

after

export const CheckoutDetailsPage: BFC<Props> = ({ id }) => {
  const { checkout, isLoading } = useCheckout(id);
  const shippingStatusSentence = useCallback((parcel:

export const CheckoutDetailsPage: BFC = () => {
  const router = useRouter();
  const id = getParam(router.query, "id");
  const { checkout, isLoading } = useCheckout(id);

命名に応じた引数を設定する。

formatShippingStatus(parcel: CheckoutParcel)だと、「なんでstatusをフォーマットしたいのに、parcelを渡さないといけないの？」ってこの関数の利用者は思う。このような場合、formatCheckoutParcelShippingStatus(parcel: CheckoutParcel)にすると、まだ分かりやすい。

情報量が多いのに、一行にまとめると見づらい。適切に改行を入れる

before

{ onSuccess: ({ data: { checkout } }) => setCheckout(new Checkout(checkout)) },

after(refetchで再取得するようにしている)

  const { isLoading, refetch } = useQuery(
    ["checkout", id],
    () => api.getCheckout(id),
    {
      onSuccess: ({ data: { checkout } }) => {
        setCheckout(new Checkout(checkout));
      },
    },
  );

  const { mutate: cancelMutate } = useMutation((id: string) => api.cancelCheckout(id),
    {
      onSuccess: () => {
        refetch();
      },
    },
  );

  return {

外に出す関数名数にはmutateみたいなワードは付けない

利用者にとって、どうでもいい情報が露出しているので、直感的に使いづらい。あと、抽象度の低いインターフェースになっている。 before

  const { mutate: cancel } = useMutation((id: string) => api.cancelCheckout(id),
    {
      onSuccess: () => {
        refetch();
      },
    },
  );
  return {
    checkout,
    isLoading,
    cancel,
  };
};

after

  const { mutate: cancel } = useMutation((id: string) => api.cancelCheckout(id),
    {
      onSuccess: () => {
        refetch();
      },
    },
  );
  return {
    checkout,
    isLoading,
    cancel,
  };
};

名が体をなす

フロントに限らずめちゃ大事。

custom hookで関数を外部に露出する際には、可能な限り内部の実装に依存しないような一般的な名称で露出した方が良い。

以下の理由が考えられる。

custom hookに関して話しているが、関数の命名にも関係している。

利用する側からしたら、useStateにおけるstateなのか、recoilにおけるstoreなのか、定数なのかどうでもいい。どうでもいい情報が露出することで、hookが直感的に使えなくなる。使いづらくなる。そのため、目的を表す直感的な命名にする
命名が内部の実装に依存しているので、抽象度の低いインターフェースになってしまう。抽象度の低いインターフェースは、変更に弱い

目次

作った機能

Rails

full_nameメソッドをモデルに実装する

アドレスはハードコーディングせずに設定で持った方が良い

サービス内でメール送信処理を行う

対象のリソースを置き換えるなら、putにした方が適切

Class.newで独自エラーが作れる

cancel処理は、単純なステータス変更以外にも様々な関連モデルの変更が予想されるので、サービスを通して処理をする

フロントの都合でDBのスキーマを変更しない。必要ならシリアライザーを使う。

バックエンドのモデルとフロントエンドのモデルは基本的には責務は同じだが、ちょっと責務が違う部分がある。

キャンセル可能かどうかを判定する

Rspec

フロントエンド

エクスポートパスをアルファベット順に記載する

リソースに応じたパス名をつける

都度ラップコンポーネントを用意するのを避けるために、

インデントを無駄に作って、ネストが深くならないようにする

DOM内でswitch文を使うようなトリッキーな記述をしない。

汎用性がない粒度でモデルにメソッドを生やさない

日時のフォーマットは、可能な限り統一したフォーマットにする

全アイコンで共通のclassNameを使う場合、渡す際にclassNameを指定して、classNameの指定がいろんな箇所に散らばらないように一箇所にする。あと、使う側の都合で決まる値は、使う方で指定する。

履歴のカードコンポーネントなら、CheckoutHistoryCardという命名にする

StopPropagationとe.preventDefaultはjsやるならマスターしておいた方が良い

スマホ表示なら、カードの左右のスペースはいらない。表示面積が狭くなるだけだから。

スマホとPCで表示を分けたいなら、tailwindのsmプロパティを使って頑張る

特定のユースケースに限定されたコンポーネントをなるべく作らない

購入した際の金額はproduct.priceを参照するのではなくて、checkout_item.priceを参照した方が良い

エラー表示よりカード表示の方が優先度が高いので、カード表示の方を上に書く

keyに指定するためだけにidを保持しているなら、hrefを使う

コンポーネント特有の表示ならそこに書けば良い

RESTの原則に従うなら、変数名をDETAILSではなくてSHOWにする

これ明らかに他のところでも書くなっていう処理はヘルパーで定義する。

あるべきところにあるべきcssを書く。

w-6/12 と w-2/5 が並んでいて、分母が変わってしまうと、一体残りがどれくらいあるのかわからなくなるので、こういう時は分母を揃える

無駄にhtmlをネストさせない

必要のないcssは書かない。

w-fitを使うことで、デザインがコンテンツの内容に依存してしまうのでなるべく使わない方が良い。コンテンツが変わっても変化しないデザインが良い。

スマホに対応するためにsm:mt-0を書くDOMに書くのはしんどい。flex と gap-n gap-y-n で用いた方がシンプルにかける。

Cardはリスト表示に使うコンポーネント。詳細画面はかなり個別のカスタマイズが入るので、あまり共通化するメリットがない画面である

命名に応じた引数を設定する。

情報量が多いのに、一行にまとめると見づらい。適切に改行を入れる

外に出す関数名数にはmutateみたいなワードは付けない

名が体をなす

custom hookで関数を外部に露出する際には、可能な限り内部の実装に依存しないような一般的な名称で露出した方が良い。

参考記事