kei615ykhm / logic-loom-nextjs14

LogicLoom is a memo app for engineers created as personal developers. There are no release plans. We will proceed with development while learning practical usage of Next.js, TypeScript, TailwindCSS, Vercel, and Supabase.
0 stars 0 forks source link

WIP: 型安全性を高めるためのzod導入とリファクタリング進捗報告とご相談 #18

Open kei615ykhm opened 1 month ago

kei615ykhm commented 1 month ago

このプルリクエストはマージを目的としていないため、セッション終了後にクローズする予定です。

概要

作業途中のため、以下の項目が主な内容になります。

現在、Issue: #16 に基づき、PR: #14 以降の内容から zod を導入し、バリデーションの実装と型安全性向上を目的としたリファクタリングを進めています。

なお、#17 にてESLint、Prettier、tsconfigルール設定等の環境調整を行った後に作成したブランチでの作業になります。

作業状況

  1. src/schemas ディレクトリの作成

    • src/schemas/memoSchema.ts: メモのスキーマを定義
  2. src/types ディレクトリの更新

    • src/types/index.ts: zod スキーマから型を生成
  3. src/utils ディレクトリの作成

    • src/utils/zodParser.ts: 汎用的な zod パース関数を実装
  4. src/hooks/useMemoManager.ts の更新

    • zod スキーマを使用してバリデーションを実装
    • 新規メモ作成時にバリデーションを適用
    • ローカルストレージへの保存機能を追加
  5. src/types ディレクトリの削除

    • 型定義が src/schemas/memoSchema.ts と重複するため
  6. src/schemas/memoSchema.ts で定義したメモスキーマの改善(作業中)

    • commit: 176f003 〜 dd58504 にて作業

※作業途中で得た知見を基に、一部の予定のタスクを変更している箇所もあります。

ご相談内容

src/hooks/useMemoManager.ts にて zod のバリデーションを実装した際に学んだ内容を基に、src/schemas/memoSchema.ts に定義しているメモスキーマにバリデーションを追加する作業を進めています。

import { z } from 'zod';

export const memoSchema = z.object({
  id: z.string(),
  content: z.string(),
  createdAt: z.string(),
});

export type Memo = z.infer<typeof memoSchema>;

現時点で、createdAtのバリデーション強化、contentの制約追加、idの一意性保証を行いました。しかし、エラーメッセージの詳細化について理解が不十分です。現在、バリデーションエラーメッセージが具体的でないという問題を認識していますが、それぞれのエラーメッセージにどのような内容を設定すべきかがわかりません。どのようなエラーメッセージを設定すべきか、ご助言いただければ幸いです。

niaka3dayo commented 1 month ago
import { z } from 'zod';

export const memoSchema = z.object({
  id: z.string(),
  content: z.string(),
  createdAt: z.string(),
});

export type Memo = z.infer<typeof memoSchema>;

現時点で、createdAtのバリデーション強化、contentの制約追加、idの一意性保証を行いました。しかし、エラーメッセージの詳細化について理解が不十分です。現在、バリデーションエラーメッセージが具体的でないという問題を認識していますが、それぞれのエラーメッセージにどのような内容を設定すべきかがわかりません。どのようなエラーメッセージを設定すべきか、ご助言いただければ幸いです。

memoSchemaおよびMemoのデータベースでの要件が定まっていないため、現時点でそれぞれの値がどのような形をとっているのが正しいのかというのが見えておらず、そもそも「どういう状況が正しいのか」という要件が存在しないのではないでしょうか? なぜなら、このMemoという型は、本来はデータベースのテーブルの1レコードあたりの形と制限に依存するからです。

なので、たとえば、以下のようにデータベースのスキーマをPrismaで定義する予定だという仮定にしてみます。

// prisma/schema.prisma
model Memo {
  id        String   @id @default(uuid())
  content   String   @db.Text
  createdAt DateTime @default(now())
}

すると、データベースからFetchしたとき、かえってくるべきデータは以下の要件となります。

id:

content:

createdAt:

この要件を満たすためのエラーメッセージ入りのmemoSchemaは以下の通りです。

export const memoSchema = z.object({
  id: z
    .string({ message: 'IDは文字列である必要があります。' })
    .uuid({ message: 'IDは有効なUUID形式でなければなりません。' }),

  content: z
    .string( { messsage: '内容は文字列で定義する必要があります。'} )
    .min(1, { message: '内容は最低1文字必要です。' })
    .max(1000, { message: '内容は1000文字以内でなければなりません。' }),

  createdAt: z
    .string( {messsage: '日付は有効な日付形式文字列で定義する必要があります。'} )
    .refine((val) => !isNaN(Date.parse(val)), { message: '作成日時は有効な日付形式でなければなりません。' }),
});

また、データを取得したものが正しいかを判定するのに使用するzodSchemaとフォームがデータを作成する際にバリデーションを行うためのスキーマは、型も目的も違うので別途定義する必要があります。

フォームから新しいメモを作成する際には、idやcreatedAtはクライアント側で生成する必要がないため、これらのフィールドを除外したスキーマを定義します。

import { z } from 'zod';

/** メモ作成用のスキーマ */
export const createMemoSchema = z.object({
  content: z
    .string({ message: '内容は文字列で定義する必要があります。' })
    .min(1, { message: '内容は最低1文字必要です。' })
    .max(1000, { message: '内容は1000文字以内でなければなりません。' }),
});

/** メモ作成時の型 */
export type CreateMemoInput = z.infer<typeof createMemoSchema>;

こういう相談内容はあとから見返せしやすいように、PRで行うより本来はIssueで残したほうがいいかもしれないです。

kei615ykhm commented 1 month ago

@niaka3dayo ご対応ありがとうございます。 確かに、PRよりもissueとして対応する方が適切でしたね。 本件について再度確認したい点がありますので、後ほどissueを作成し、改めてアサインさせていただきます。

また、別件でお願いしていた件についてですが、私の書き方が悪く、うまく伝わっていなかったようで申し訳ありません。

改めてになりますが、各ファイル内のコードの型安全性についても確認をお願いしたいと考えておりました。 相談内容へのご返答で「データを取得する際に使用するzodSchemaと、フォームでデータを作成する際のバリデーション用スキーマは、型や目的が異なるため、別途定義する必要がある」とのことでしたが、これが現在のコードの問題点でもあるという理解で間違いないでしょうか?

kei615ykhm commented 1 month ago

@niaka3dayo 返答と修正が遅れてしまい申し訳ございません。全体的に知識不足と言語化できていない部分があったため、IDUUIDの周辺知識を再学習しながら作業しています。

zennスクラップ

修正が済み次第、返信させていただきますので、よろしくお願い致します。

kei615ykhm commented 1 month ago

Issue: #20 タスクとして、このプルリクエストに適したマイルストーンを設定し直しました。

kei615ykhm commented 1 month ago

@niaka3dayo 修正が遅れてしまい申し訳ありません。

ここまでにご指摘、アドバイスいただいた項目の修正は完了しましたので、各項目にてご確認をお願いいたします。

ドメインモデルの概念や目的については概ね把握しました。しかし、その使い方と設定理論については今後の学習課題です。 また、作業する中で名前の衝突と拡張性についても課題を感じました。そのため、先日教えて頂いたドメイン駆動設計とpackage by featureを組み合わせた設計パターンを採用し、以下のように作ってみたいと考えています。

src/
├── domains/
│   └── memo/
│       ├── memoParser.ts         // メモのパース処理
│       ├── memoValidator.ts      // ランタイムバリデーション
│       ├── memoTypeGuard.ts      // 型ガード
│       ├── memoZodSchema.ts      // Zodスキーマ定義
│       ├── memoDataRepository.ts // リポジトリ層
│       └── memoApplicationService.ts // アプリケーションサービス
└── features/
    └── memo/
        └── useMemoStateManager.ts // メモの状態管理

より詳細な情報

Issue: memoUtils.tsの責任範囲分割とファイル移行計画の提案

こちらの移行作業は、今回のプルリクエストの主旨と異なる内容なので、一旦実験用ブランチにて作業してみます。後ほど、別件としてコードレビュー依頼をさせてください。