Skip to content

refactor: EncodingConverter の手書き debounce 2 系統を useDebouncedTransform に統合 (#394) - #480

Merged
fumtas1k merged 2 commits into
developfrom
refactor/issue-394-encoding-debounce
May 24, 2026
Merged

refactor: EncodingConverter の手書き debounce 2 系統を useDebouncedTransform に統合 (#394)#480
fumtas1k merged 2 commits into
developfrom
refactor/issue-394-encoding-debounce

Conversation

@fumtas1k

Copy link
Copy Markdown
Owner

概要

リファクタ監査 (2026-05-11) finding L-7 への対応。EncodingConverter が持つ手書き debounce pipeline 2 系統(detect / convert)を汎用フックに統合し、debounce plumbing と state reset の散逸を解消する。

Closes #394

設計判断(issue 記載との差分)

issue は「useCodec をそのまま使う」想定だったが、EncodingConverteruseCodec のモデルと乖離している:

  • 入力が Uint8Array | nullactiveBytes、コンポーネントが useMemo で所有)で string でない
  • 出力が string でない(detect→DetectionResult+preview / convert→Uint8Array+preview)
  • 条件付き debounce(file 入力は即時、text 入力のみ 300ms)
  • 2 系統が同一 source を共有

useCodec を bytes / 非 string / 条件付き debounce 対応まで一般化すると 4 消費者(ConfigConverter / JsonXml / JsonCsv / Base64Codec)に波及するため、useCodec は無変更とし、汎用フック useDebouncedTransform<I, R> を新設した。

変更内容

  • src/hooks/useDebouncedTransform.ts(新規): source / transform / emptyResult / deps / { debounceMs, immediate, fallbackError } を受け、source === null で即時クリア、immediate で同期実行(file パス)、それ以外は debounce(再入力で前回キャンセル)、transform throw を error 化。ジェネリックで入出力型に依存しない。
  • src/components/tools/EncodingConverter.tsx:
    • 手書き detect / convert effect(setTimeout/clearTimeout)と runDetect/runConvert の setState 群を削除し、useDebouncedTransform 2 回呼び出しに置換。
    • 変換ロジックを pure function detectFromBytes / convertFromBytes としてモジュールスコープに抽出(既存テストの vi.mock('@/utils/encoding') を維持するため utils 側ではなくコンポーネント側に配置)。
    • error 表示を fileError || detectError || convertError に合流。旧実装で detect/convert effect が単一 error を相互上書きしていた latent な問題も解消。
    • activeBytes(useMemo)と inputMethod 分岐はオーケストレーション役としてコンポーネント側に残置。
  • src/hooks/__tests__/useDebouncedTransform.test.ts(新規): immediate 同期実行 / debounce / 再入力キャンセル / source=null クリア / throw 時 error の各ケース。

検証

  • npm run test: 1358 passed(新規フックテスト + 既存 EncodingConverter.test.tsx の debounce 集約・useMemo 安定・file reject エラー表示・convert debounce の 4 ケース全 green)
  • node_modules/.bin/astro check: 0 errors / 0 warnings
  • npm run test:e2e -- encoding-converter a11y-live-region filename-sanitize: 29 passed
  • pre-commit: Prettier OK / TypeScript OK

補足

  • 内部リファクタのためツール追加/削除なし。README.md / SPEC.md 更新は不要。
  • aria 属性の削除なし。Tailwind primitive scale 直書きの新規追加なし(既存 className 維持)。

🤖 Generated with Claude Code

… に統合 (#394)

detect / convert の手書き useEffect + setTimeout + clearTimeout pipeline 2 系統を
汎用フック useDebouncedTransform に置換。bytes 入力・条件付き debounce(file 即時 /
text 300ms)・非 string 出力に対応する。runDetect / runConvert は副作用を除いた
pure function(detectFromBytes / convertFromBytes)としてモジュールスコープに抽出し、
既存テストの @/utils/encoding mock を維持。error は file I/O / detect / convert を
合流表示し、旧実装の error 相互上書きも解消した。useCodec は無変更。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

🖼️ Visual Regression Test 結果

  • Status: ✅ 全 40 件 pass
  • Workflow run: 26365404324
  • Artifact (diff 画像 / playwright-report): 上記 workflow run の Artifacts セクションから download

diff が 意図的な visual 変更の場合: Update Visual Regression Baseline workflow を本 PR ブランチで workflow_dispatch trigger して baseline を更新。
diff が 意図しない regression の場合: 該当変更を fix。
本 check は required ではないため fail のままでも merge は可能(reviewer 判断)。

Copy link
Copy Markdown
Owner Author

レビュー (多角的観点)

issue #394 の「useCodec をそのまま使う」想定から useDebouncedTransform 新設へ舵を切った判断は妥当で、PR 本文に乖離理由(入力が Uint8Array | null・出力が非 string・条件付き debounce・2 系統が同一 source 共有)が明記されており、useCodec 無変更で 4 消費者への波及を回避した点も適切です。新フックの test-gates 規律(clearTimeout キャンセルの陽性/陰性対照分離、immediate/debounce 双方の throw パス、fallbackError、throw 後 recovery)は模範的で、CI test job も green(1358 passed)を確認しました。detectFromBytes/convertFromBytes をコンポーネント側モジュールスコープに置いて vi.mock('@/utils/encoding') を効かせる(intra-module call だと mock が無効化する)洞察も正確です。

以下、観点別の指摘です(いずれも non-blocking)。

🟢 Low — hook 堅牢性: immediate パスが isPending を正規化しない

useDebouncedTransform.ts の immediate ブランチは setResult/setError のみで setIsPending(false) を呼びません。debounce パスで isPending=true の状態から immediate が true へ切り替わる遷移(前 effect の cleanup は clearTimeout するが isPending は残す)で、isPending が true のまま stuck し得ます。

本コンポーネントでは影響なしです(handleInputMethodChangehandleClear()fileBytes/textInput が null 化 → activeBytes が null → source=null reset 経由で isPending が false 化される。かつ EncodingConverter は isPending を destructure せず UI で消費していない)。ただし汎用フックとして将来の消費者が immediate/debounce を混在させると観測し得るため、immediate ブランチに setIsPending(false) を 1 行足しておくと意味的にも正しく cheap です。

🟢 Low — アーキ: debounce プリミティブが 3 系統に

useCodec / useCodecWithMeta#478 で統合予定)に加え、本 PR で useDebouncedTransform が 3 つ目の debounce / clearTimeout / isPending / throw 処理コピーになりました。useDebouncedTransform が最も汎用的(source 外部所有・非 string・条件付き immediate)なので、むしろこれを唯一の core プリミティブとし、useCodec(= string source を所有する薄い wrapper)を寄せるのが DRY 的な最終形になり得ます。#478 のスコープに「useDebouncedTransform を統合先候補として検討」を追記する形で集約するのを提案します(新規 issue 不要)。今回スコープでは必須ではありません。

🔵 Nit — テスト: error 合流の優先順位が component テスト未カバー

const error = fileError || detectError || convertError の優先順位は新規ロジックですが、現状 fileError パス(#391 のファイル reject テスト)のみ検証され、detect/convert が同時に error を持つ場合の優先順位は単体テストがありません。PR が掲げる「detect/convert effect が単一 error を相互上書きしていた latent な問題の解消」の核心部なので、convert モードで「detect 成功・convert throw → convertError 表示」「detect throw → detectError 優先」を 1 ケース追加すると回帰ガードになります(component test か e2e いずれでも可)。

ℹ️ 情報共有 — 挙動変化: convert モードでの detect error 表面化

旧実装は detect/convert が単一 error を effect 実行順で上書きしていたため、convert モードで detect が throw しても convert が後勝ちで error を空にし得ました。新実装は detectErrorconvertError より優先表示するため、convert モードでも detect の decode エラー等が出るようになります。基本的に改善(より正確)ですが、挙動差として共有まで。

確認できた点(問題なし)

  • セキュリティ: ダウンロードファイル名は sanitizeFilename で許可拡張子ホワイトリストにサニタイズ(baseName 連結後に二重サニタイズも維持)。入力は bytes 処理で XSS 経路なし、preview は React テキスト描画。リファクタで弱体化なし。
  • ロジック: hook の source === null 厳密比較で 0 長バイト列を誤クリアしない。activeBytes(useMemo) と各 deps の整合で stale closure なし(convert の transform は [mode,sourceEnc,targetEnc,withBom,newlineMode] で追跡)。handleClear/handleModeChange の明示 setState 削除は source 制御による自動 reset と等価。
  • 型安全: ジェネリック <I, R> で入出力型に非依存、各消費箇所で型整合。
  • a11y / スタイル: aria 属性削除なし・Tailwind primitive scale 直書きなし、CI visual-regression green(40 件)。

CI: test ✅ / visual-regression ✅ / e2e は実行中。e2e green 確認後に merge 可と考えます。設計・テスト・セキュリティいずれも健全で、挙げた指摘は #1/#3 が軽微な追加推奨、#2#478 への追記提案です。


Generated by Claude Code

PR #480 レビュー対応:
- error = fileError || detectError || convertError の優先順位が未テストだった (Nit)。
  convert モードで detect throw・convert 成功時に detect error が表示される陽性対照を
  追加。merge を convertError 優先に戻すと fail することを実機確認済み。
- useDebouncedTransform の immediate ブランチが isPending を正規化せず、debounce→
  immediate 遷移で true のまま stuck し得た (Low)。setIsPending(false) を追加し、
  遷移時に false 化される陽性対照テストを併設(fix 前実装で fail を確認)。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@fumtas1k

Copy link
Copy Markdown
Owner Author

レビューありがとうございます。観点別に対応しました(commit 98f7a91)。

🟢 Low — immediate パスが isPending を正規化しない → 対応済み

useDebouncedTransform.ts の immediate ブランチに setIsPending(false) を 1 行追加しました。ご指摘どおり本コンポーネントでは無影響(handleClear 経由の source=null reset で false 化、かつ isPending 未消費)ですが、汎用フックとして debounce→immediate 遷移で stuck しないよう意味的に正しくしました。

陽性対照テストを併設: debounce パス(isPending=true)から immediate へ遷移すると isPending が false に正規化されるfix 前実装(setIsPending(false) なし)に当てると isPending が true のまま fail することをローカルで確認済み。

🟢 Low — debounce プリミティブが 3 系統に → #478 にスコープ追記

ご提案どおり新規 issue は作らず、#478 にコメントで追記しました(#478 (comment) )。「最も汎用的な useDebouncedTransform を唯一の core とし useCodec を寄せる」方向を統合先候補として明記。useCodec は input を hook が所有するモデルなので単純 wrapper 化では API 差を吸収しきれない点も注記しました。

🔵 Nit — error 合流の優先順位が未テスト → 対応済み

EncodingConverter.test.tsx に陽性対照を追加: convert モードで detect throw・convert 成功時に detect の error が表示される。これは PR が掲げる「detect/convert の error 相互上書き解消」の核心の回帰ガードです。

test-gates の鉄則に従い、merge を fileError || convertError(detect を落とす=旧 overwrite 相当)に戻すと fail することをローカルで実機確認しました(detect error が表示されず queryByText が null → fail)。修正版では pass。

ℹ️ convert モードでの detect error 表面化 → 認識一致

旧実装の effect 実行順による error 上書きが無くなり、convert モードでも detect の decode エラー等が正しく表面化します。より正確な方向の挙動差という認識で一致しています(上記回帰ガードがこの挙動を固定します)。


検証: npm run test 1360 passed(陽性対照 2 件追加)/ astro check 0 errors / pre-commit Prettier・TypeScript OK。push 済みのため CI で e2e 再実行されます。

Copy link
Copy Markdown
Owner Author

再レビュー (commit 98f7a91): LGTM ✅

前回指摘 3 件すべて、commit 98f7a91 の差分で対応を確認しました。

🟢 Low — immediate パスの isPending 正規化 → 修正確認

useDebouncedTransform.ts の immediate ブランチに setIsPending(false) が追加されています。併設の陽性対照(immediate:false(debounce, isPending=true) → immediate:true へ rerender して isPending が false へ正規化されることを assert)は本物のガードです。修正前実装では初期 debounce で立った isPending=true が残るため当 assert が fail することを確認しました。

🟢 Low — debounce プリミティブ 3 系統 → #478 へ追記確認

新規 issue を作らず #478 にスコープ追記いただき、提案どおりの集約です。「useCodec は input を hook が所有するモデルゆえ単純 wrapper 化では API 差を吸収しきれない」という注記も的確で、統合時の論点が明示されています。

🔵 Nit — error 合流優先順位の未テスト → 陽性対照追加確認

EncodingConverter.test.tsx に「convert モードで detect だけ throw・convert 成功時に detect error が表示される」を追加。detectError が非空なら fileError || detectError || convertError で必ず表示されるため、merge を fileError || convertError(detect 落とし=旧 overwrite 相当)へ戻すと確実に fail する設計で、PR の核心(error 相互上書き解消)の回帰ガードとして機能します。mockImplementationOnce の消費タイミング(mount 時は source=null で未呼び出し → 入力 + debounce 後に 1 回だけ throw)も整合しています。

ℹ️ convert モードでの detect error 表面化 → 認識一致

上記回帰ガードがこの挙動(より正確な方向)を固定する点も含め、認識一致です。


CI: test ✅ / visual-regression ✅(40 件)/ e2e は実行中。

残課題なし。3 件いずれも本物の陽性対照付きで検証されており、e2e green 確認後に merge して問題ない品質です。


Generated by Claude Code

@fumtas1k
fumtas1k merged commit 7a21371 into develop May 24, 2026
3 checks passed
@fumtas1k
fumtas1k deleted the refactor/issue-394-encoding-debounce branch May 24, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P1] refactor(encoding-converter): 手書き debounce 2 系統を useCodec に統合

1 participant