Worked review 2: form validation and discriminated-union state

domain ของรีวิวนี้คือ state ของ form โมเดลเป็น discriminated union — pattern เดียวกับที่บท narrowing ของคอร์สนี้สอนว่าเป็น pattern ที่ถูกต้อง bug ในที่นี้ไม่ได้อยู่ที่การเลือกใช้ discriminated union มันอยู่ที่ความมั่นใจที่ mistake เล็ก ๆ สี่ตัวที่ดูสมเหตุสมผลซ่อนอยู่ข้างในมันได้แค่ไหน

โปรแกรม

Review this before reading further

// @run
interface FormValues { email: string; discountCode?: string; couponValue?: number }

type FormState =
  | { status: "idle" }
  | { status: "editing"; values: FormValues }
  | { status: "submitting"; values: FormValues }
  | { status: "success"; values: FormValues }
  | { status: "error"; values: FormValues; message?: string };

function isValidForm(values: unknown): values is FormValues {
  return true; // BUG 1: type predicate that never actually checks anything
}

function couponSavings(values: FormValues): number {
  return values.couponValue || 5; // BUG 2: || -- default promo of 5, but an explicit 0 (opt out) looks like "not set" too
}

function describeState(state: FormState): string {
  switch (state.status) {
    case "idle":
      return "Fill in the form to continue.";
    case "editing":
      return `Editing, coupon savings: ${couponSavings(state.values)}`;
    case "submitting":
      return "Submitting...";
    case "success":
      return "Thanks! Your form was submitted.";
    // BUG 3: "error" case forgotten -- no default, no never check, no error
  }
  return "Unknown state";
}

function renderErrorBanner(state: FormState): string {
  if (state.status !== "error") return "";
  return `Error: ${state.message!.toUpperCase()}`; // BUG 4: ! on an optional field
}

function submitForm(raw: unknown): FormState {
  if (!isValidForm(raw)) return { status: "idle" };
  return { status: "submitting", values: raw };
}

const editingWithZeroCoupon: FormState = {
  status: "editing",
  values: { email: "[email protected]", couponValue: 0 },
};
console.log(describeState(editingWithZeroCoupon));

const submitted = submitForm({ totallyNotAFormValues: true });
console.log(describeState(submitted));

const errorState: FormState = { status: "error", values: { email: "[email protected]" } };
console.log(describeState(errorState));
console.log(renderErrorBanner(errorState));

Open in TS Playground

กด Check types: tsc --strict รายงาน ไม่มี error เลย กด Run:

Editing, coupon savings: 5
Submitting...
Unknown state
TypeError: Cannot read properties of undefined (reading 'toUpperCase')
    at renderErrorBanner (...)

สี่บรรทัดผิดติดกัน แล้วไม่มีสักบรรทัดที่ดูเหมือน crash จนกว่าจะถึงบรรทัดสุดท้ายที่ crash จริง ๆ

ลองหา bug ก่อนอ่านต่อ

สองในนี้ถูกระบุตรง ๆ ใน spec ของคอร์สนี้สำหรับรีวิวนี้ อีกสองตัวซ่อนอยู่ตรงหน้าเลย ลองบอกให้ครบทั้งสี่ก่อนเปิดเฉลย

Bug 1 — บรรทัด 10, isValidForm return true แบบไม่มีเงื่อนไข

Symptom: signature ของ function (values is FormValues) สัญญาว่าเช็ค shape แล้ว body ไม่เช็คอะไรเลย

Evidence: submitForm({ totallyNotAFormValues: true }) — object ที่ไม่มี field ไหนของ FormValues เลย — ผ่าน isValidForm ไปได้อยู่ดี แล้ว submitForm ก็คืน { status: "submitting", values: raw } แบบร่าเริงโดย values ผิดสมบูรณ์ describeState พิมพ์ "Submitting..." ให้กับ input ขยะ แทนที่จะปฏิเสธมัน

Fix: เช็ค field ที่ predicate อ้างว่าเช็ค:

// @skip-verify partial snippet, illustrates the fix only -- see the full fixed version below
function isValidForm(values: unknown): values is FormValues {
  return typeof values === "object" && values !== null
    && typeof (values as Record<string, unknown>).email === "string";
}

Bug 2 — บรรทัด 15, values.couponValue || 5

Symptom: user ที่กรอก 0 ตรง ๆ (opt out จาก promo) ถูกปฏิบัติเหมือนกับ user ที่ไม่เคยแตะ field coupon เลย

Evidence: editingWithZeroCoupon มี couponValue: 0 — opt-out ตรง ๆ — แล้ว describeState รายงาน "Editing, coupon savings: 5" ไม่ใช่ 0

Fix: ?? แยก “ไม่ได้ตั้งค่า” ออกจาก “ตั้งเป็นค่า falsy” ได้:

// @skip-verify partial snippet, illustrates the fix only -- see the full fixed version below
function couponSavings(values: FormValues): number {
  return values.couponValue ?? 5;
}

Bug 3 — บรรทัด 28, case "error" หายไปจาก switch

Symptom: describeState จัดการ 4 ใน 5 variant ของ FormState; ตัวที่ห้าตกไปที่ fallback string ที่ hardcode ไว้แบบเงียบ ๆ แทนที่จะเป็น compile error

Evidence: describeState(errorState) พิมพ์ "Unknown state" — FormState จริงและ valid สร้าง message ที่ตั้งใจไว้สำหรับสิ่งที่ type system ไม่เคยเห็นมาก่อนเท่านั้น

Fix: branch default พร้อม exhaustiveness check ที่ type เป็น never เปลี่ยน case ที่ลืมในอนาคตให้เป็น compile error แทนที่จะเป็น string ที่ผิด:

// @skip-verify partial snippet, illustrates the fix only -- see the full fixed version below
function describeState(state: FormState): string {
  switch (state.status) {
    // ...
    case "error":
      return `Error state: ${state.message ?? "unknown error"}`;
    default: {
      const exhaustiveCheck: never = state;
      return exhaustiveCheck;
    }
  }
}

Bug 4 — บรรทัด 34, state.message!.toUpperCase()

Symptom: message บน variant "error" type เป็น string | undefined — error state ไม่ได้มี message เสมอไป — และ ! บอก checker ให้ไม่สนใจสิ่งนั้น

Evidence: การ crash จริง errorState ถูกสร้างโดยไม่มี message เลย state.message! เป็น undefined แล้ว .toUpperCase() throw

Fix: fallback ไป default message แทนที่จะ assert ว่ามันมีอยู่:

// @skip-verify partial snippet, illustrates the fix only -- see the full fixed version below
function renderErrorBanner(state: FormState): string {
  if (state.status !== "error") return "";
  return `Error: ${(state.message ?? "unknown error").toUpperCase()}`;
}

Version ที่แก้แล้ว

// @run
interface FormValues { email: string; discountCode?: string; couponValue?: number }

type FormState =
  | { status: "idle" }
  | { status: "editing"; values: FormValues }
  | { status: "submitting"; values: FormValues }
  | { status: "success"; values: FormValues }
  | { status: "error"; values: FormValues; message?: string };

function isValidForm(values: unknown): values is FormValues {
  return typeof values === "object" && values !== null
    && typeof (values as Record<string, unknown>).email === "string";
}

function couponSavings(values: FormValues): number {
  return values.couponValue ?? 5;
}

function describeState(state: FormState): string {
  switch (state.status) {
    case "idle":
      return "Fill in the form to continue.";
    case "editing":
      return `Editing, coupon savings: ${couponSavings(state.values)}`;
    case "submitting":
      return "Submitting...";
    case "success":
      return "Thanks! Your form was submitted.";
    case "error":
      return `Error state: ${state.message ?? "unknown error"}`;
    default: {
      const exhaustiveCheck: never = state;
      return exhaustiveCheck;
    }
  }
}

function renderErrorBanner(state: FormState): string {
  if (state.status !== "error") return "";
  return `Error: ${(state.message ?? "unknown error").toUpperCase()}`;
}

function submitForm(raw: unknown): FormState {
  if (!isValidForm(raw)) return { status: "idle" };
  return { status: "submitting", values: raw };
}

const editingWithZeroCoupon: FormState = {
  status: "editing",
  values: { email: "[email protected]", couponValue: 0 },
};
console.log(describeState(editingWithZeroCoupon));

const submitted = submitForm({ totallyNotAFormValues: true });
console.log(describeState(submitted));

const errorState: FormState = { status: "error", values: { email: "[email protected]" } };
console.log(describeState(errorState));
console.log(renderErrorBanner(errorState));

Real output:

Editing, coupon savings: 0
Fill in the form to continue.
Error state: unknown error
Error: UNKNOWN ERROR

ทุกบรรทัดถูกต้องแล้ว: coupon 0 ที่ตั้งใจส่งมารอด, input ขยะถูกปฏิเสธกลับไปเป็น "idle" อย่างถูกต้อง, และ error state render fallback message จริงแทนที่จะ crash

เครื่องมือจับอะไรได้บ้าง

tsc --strict: ไม่มี error เลย ทั้งสอง version

typed rule ของ typescript-eslint รันกับ version ที่มี bug:

16:29  error  Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`),
              as it is a safer operator                          @typescript-eslint/prefer-nullish-coalescing
20:11  error  Switch is not exhaustive. Cases not matched: "error"  @typescript-eslint/switch-exhaustiveness-check
36:20  error  Forbidden non-null assertion                       @typescript-eslint/no-non-null-assertion

✖ 4 problems (4 errors, 0 warnings)

สามในสี่ตัว ถูกจับโดยตรง — รวมถึง switch ที่ไม่ exhaustive ซึ่ง switch-exhaustiveness-check จับได้ โดยไม่ต้องมี never check ที่เขียนเองเลย ต่างจากการพึ่ง tsc อย่างเดียว bug ตัวเดียวที่ไม่มีเครื่องมือไหนที่นี่จับได้คือ Bug 1: predicate ที่โกหกของ isValidForm signature ของ type predicate ถูกเชื่อเต็มร้อย ไม่มีอะไรตรวจว่า body ของมันพิสูจน์สิ่งที่มันกล่าวอ้างจริงหรือเปล่า