fix(config-converter): デバウンス中のダウンロードを抑止 (#149) - #181
Conversation
変換先フォーマット切替後の 300ms デバウンス待ち中にダウンロードボタンを 押すと、ファイル名の拡張子は新しいフォーマット・内容は旧フォーマットとなる 不整合が発生していた。 useCodec に isPending フラグを追加し、deps が変化してからデバウンス完了まで true を返すようにした。ConfigConverter ではこのフラグを DownloadButton の disabled prop に渡すことで、出力が確定するまでダウンロードを抑止する。 Closes #149 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Deploying devtools with
|
| Latest commit: |
08e8820
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4dfcf3be.devtools-d9w.pages.dev |
| Branch Preview URL: | https://fix-issue-149-debounce-downl.devtools-d9w.pages.dev |
fumtas1k
left a comment
fumtas1k
left a comment
There was a problem hiding this comment.
レビュー結果: Comment（Approve 相当 / 軽微な提案のみ）
※ 自分が作成者の PR のため Approve ステータスは付けられず Comment で投稿しています。
#149 のデバウンス中ダウンロード抑止の修正、目的にしっかり沿った最小スコープの良い PR だと思います。useCodec に isPending を生やして DownloadButton の disabled に直結する設計はシンプルで読みやすく、E2E でも「切り替え直後 → デバウンス完了後」の状態遷移をきちんと検証できています。
良い点
- API 拡張が非破壊的: 既存の
useCodec利用側（JsonXml.tsx/JsonCsv.tsx/Base64Codec.tsx）は戻り値の追加プロパティを無視するだけで動作するため、互換性が保たれています。 - 状態遷移の網羅性: 入力クリア時 (
if (!input))・reset()呼び出し時の双方でsetIsPending(false)を漏れなくリセットしており、ボタンが固まる事故を防げています。 finallyで確実に解除:transformが throw してもisPendingが確実にfalseに戻るのは堅実な作り。- JSDoc 追記:
useCodec.tsの hook コメントにisPendingの意味と用途（拡張子と内容の不整合防止）が日本語で明示されており、他ツールへの横展開判断もしやすい。 - E2E の質: 「変換先切替直後 disabled → 完了後 enabled」を 1 ケースで往復検証しており、回帰検知に十分。
改善提案（任意 / 重要度低）
1. JSDoc / PR 説明文と実挙動の僅かな乖離（src/hooks/useCodec.ts:21-22）
JSDoc に「deps が変化してから〜」とありますが、実際は input が変化したとき（＝タイプ中） にも isPending が true になります（useEffect の依存配列に input が含まれるため）。これは「出力が確定するまで disabled」というユーザー視点の挙動として正しい一方、コメントから受ける印象だと「タイプ中は対象外」と読めてしまうかもしれません。一行の補足で十分なので、例えば以下のように直すとより正確です:
* `isPending` は input または deps が変化してからデバウンス完了（出力反映）までの間 true になる。PR 説明の「deps が変化してから」も同様。実装は変更不要、ドキュメント表現のみの提案です。
2. 同根の問題が他ツールにも存在する可能性（スコープ外 / 横展開）
JsonXml.tsx JsonCsv.tsx Base64Codec.tsx も useCodec を使っているため、変換先切替で拡張子が変わるツールがあれば同じバグの素地があります。本 PR はスコープを ConfigConverter に絞っているのは良判断ですが、shared-agent-rules.md 6.4 に従い 横展開要否の確認 issue を 1 本立てておくと先送り防止になります（必要無しと判断したならその旨を PR 内コメントで明記）。
3. E2E のタイミング依存（tests/e2e/config-converter.spec.ts:170-174 付近）
click() 直後に expect(downloadBtn).toBeDisabled() を待つ構成は、デバウンス 300ms ≫ Playwright のポーリング間隔 (~100ms) なので通常は安定しますが、CI 負荷時にデバウンス完了前の disabled 状態を取り逃すリスクが理論上あります。気になるなら、useCodec の debounceMs をテスト用に長めにするか、page.clock で時間制御する手もあります。現状の安定度を見て不要なら据え置きで OK です。
4. ユニットテスト追加の検討（任意）
useCodec 自体に isPending の挙動を検証するユニットテスト（input 入力中・deps 変更後・reset 後・throw 時）が tests/ 配下にあると、フックの仕様として固められて将来の改修に強くなります。E2E でカバーされているので必須ではありません。
規約遵守チェック
- Tailwind カラークラス未使用 ✅
- 日本語コメント / 日本語 PR 本文 ✅
- ベースブランチ
develop✅ - E2E 同時追加 ✅（
shared-agent-rules.md3章） - 型チェック /
npm run test実施記載あり ✅
軽微な提案ばかりで、本 PR は問題なくマージして良い品質と判断します。お疲れさまでした。
Generated by Claude Code
- JSDoc の isPending 説明に input 変化時の挙動を明記 - @testing-library/react + jsdom を devDependencies に追加 - vitest.config.ts のテスト glob を .tsx まで拡張 - src/hooks/__tests__/useCodec.test.ts を新規追加（5 ケース）: input 入力中に isPending が true になる deps 変更後に isPending が true になる デバウンス完了後に isPending が false になる reset() 後に isPending が false になる transform が throw しても finally で isPending が false に戻る Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
useCodec の単体テストで renderHook を使うために追加した devDependency を package-lock.json に同期。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
ご指摘ありがとうございます。4 点について対応結果をご報告します。 1. JSDoc 修正（適用） 2. 横展開調査 3. E2E タイミング依存（据え置き） 4. Generated by Claude Code |
fumtas1k
left a comment
fumtas1k
left a comment
There was a problem hiding this comment.
再レビュー結果: Comment（Approve 相当）
※ 自身が作成者の PR のため Approve ステータスは付与できず Comment で投稿します。
前回（review #4209861086）の指摘 4 点に対する対応を確認しました。いずれも適切に処理されており、追加された対応内容も品質が高いと判断します。最新コミット 08e8820 ベースでの再レビューです。
前回指摘事項への対応評価
| # | 指摘内容 | 対応 | 評価 |
|---|---|---|---|
| 1 | JSDoc の deps 限定表現の補正 |
src/hooks/useCodec.ts:22 で「input または deps が変化してから〜」に修正済み |
✅ 完全対応 |
| 2 | 横展開（他ツールへの波及） | JsonXml/Base64Codec はダウンロード無し、JsonCsv は reset() で拡張子不整合は防止済み・残存リスクは #184 として issue 化 |
✅ 6.4 章準拠で適切 |
| 3 | E2E のタイミング依存 | 現状安定動作のため据え置き（flaky 顕在化時に対処方針） | ✅ 妥当な判断 |
| 4 | useCodec 単体テスト追加 |
src/hooks/__tests__/useCodec.test.ts を新規作成し、5 ケース（input 入力中・デバウンス完了・deps 変更・reset・throw 時の finally）を網羅 |
✅ 想定以上の質 |
特に #2 は shared-agent-rules.md 6.4 章「先送り時は必ず issue 化」を完全に踏んだ対応で、形骸化を避けつつスコープ管理ができています。
追加分の良い点
useCodec.test.tsのテスト設計:vi.useFakeTimers()+act()の組み合わせで時間制御が決定論的。transform が throw しても finally で isPending が false に戻るケース（src/hooks/__tests__/useCodec.test.ts:121-134）まで検証しており、フックの不変条件（pending 状態が滞留しない）を将来の改修からも守れる設計。// @vitest-environment jsdomをファイル先頭に置きグローバル設定を汚染せず、ノード既定環境のままユニットテストの大半を残している点も丁寧。
vitest.config.ts:12を*.test.{ts,tsx}に拡張したのは将来の React コンポーネントテスト追加に備えた良い変更。- devDependency 追加:
@testing-library/react ^16.3.0/jsdom ^26.1.0ともに dev のみ・本番バンドルへ影響なし。package.jsonとpackage-lock.jsonの同期も取れています。 - JSDoc の修正は最小: 実装コードを触らずドキュメント表現のみ更新しており、安全性が高い。
規約遵守チェック（再確認）
- Tailwind カラークラス未使用 ✅
- 日本語コメント / 日本語 PR 本文・返信 ✅
- ベースブランチ
develop✅ - E2E 同時追加 ✅（
shared-agent-rules.md3 章） - 単体テスト追加 ✅
- スコープ外発見時の issue 化（#184）✅（
shared-agent-rules.md6.4 章） - 型チェック /
npm run test(383 件) /npm run test:e2e(13 件) 実施記載あり ✅
追加指摘
特になし。前回の任意提案にも丁寧に対応いただき、横展開課題も #184 で追跡可能になっているため、本 PR は安心してマージできる品質です。お疲れさまでした。
Generated by Claude Code
Generated by Claude Code
レビュー指摘 #3 #4 への対応: - shared-agent-rules.md 6.2: 「`origin/develop` 起点を必ず明示」の理由（PR #154/#181 既知問題）と playbook 1.1 章へのポインタを追加 - playbooks/e2e-validation.md 2.1: ステップ 0 のコマンド列が広すぎたので説明文を表外の引用ブロックへ移動 - playbooks/pr-creation.md 4: PR 作成コマンドの一時ファイルパス例に `$TMPDIR/pr_body.md` を併記（`/tmp/claude/` も併記、permissions allow/ask の理由を明示） レビュー指摘 #1 (SSOT 明示) と #2 (参照チェッカ機械化) は別 issue で対応予定。
* docs(rules): playbooks/ と setup/ 配下にタスク手順書・環境セットアップを切り出す shared-agent-rules.md の肥大化対策として、以下 4 ファイルを新設: - docs/playbooks/pr-creation.md: ブランチ作成→検証→PR→マージの完全手順 - docs/playbooks/e2e-validation.md: E2E 実行手順・push 前チェックリスト・失敗判定 - docs/setup/plugins.md: Claude Code プラグイン install ガイド (Web silent fail / context7 403 / API キー) - docs/setup/gemini-policy.md: Gemini security policy symlink セットアップ 「常時必読の規約」と「タスク開始時に読む手順書」を物理的に分離し、 セッション毎に必要な情報量を減らすのが狙い。 * docs(rules): CLAUDE.md / GEMINI.md / shared-agent-rules.md を新ファイル参照型に圧縮 肥大化していた常時ロード対象ファイルを以下の方針でスリム化: - CLAUDE.md: 80→46 行。プラグイン install トラブル詳細を docs/setup/plugins.md へ移動 - GEMINI.md: 53→39 行。security policy symlink 手順を docs/setup/gemini-policy.md へ移動 - shared-agent-rules.md: 365→258 行。以下を移動: - 旧 3 章（E2E 実行手順）→ docs/playbooks/e2e-validation.md - 旧 6.2 / 6.2a（ブランチ作成詳細）と 3.2 親 push チェックリスト → docs/playbooks/pr-creation.md - 旧 8 章（UI 目視確認）→ docs/ui-conventions.md 3.1 章に統合 - 旧 10 章 → 9 章、旧 11 章 → 10 章、旧 12 章 → 11 章 に章番号を繰り上げ - agent-lessons.md の章番号参照を新番号に追従 各章末尾に「詳細手順 → docs/playbooks/X.md」のポインタを残して双方向リンク化。 * docs(rules): PR #240 レビュー指摘の軽微対応（「なぜ」補足 / 表幅 / TMPDIR 例示） レビュー指摘 #3 #4 への対応: - shared-agent-rules.md 6.2: 「`origin/develop` 起点を必ず明示」の理由（PR #154/#181 既知問題）と playbook 1.1 章へのポインタを追加 - playbooks/e2e-validation.md 2.1: ステップ 0 のコマンド列が広すぎたので説明文を表外の引用ブロックへ移動 - playbooks/pr-creation.md 4: PR 作成コマンドの一時ファイルパス例に `$TMPDIR/pr_body.md` を併記（`/tmp/claude/` も併記、permissions allow/ask の理由を明示） レビュー指摘 #1 (SSOT 明示) と #2 (参照チェッカ機械化) は別 issue で対応予定。
* docs(rules): shared-agent-rules.md に Tailwind v4 variant 警告と subagent 運用補足を昇格 agent-lessons.md からの規約昇格: - 7.1 章 (新設): @layer components 内手書き class は hover:/focus: variant 非対応の警告。 silent regression 事故 (PR #277) と検証手順を明示。 - 6.7 章 (新設): subagent 運用補足。完了報告は項目別ステータス必須 (PR #218 事例) / package.json 変更時 lock 同期確認 (PR #181 事例)。 これで agent-lessons.md の該当 lesson を本ファイルへ集約し、agent-lessons.md は 継続検討中の lesson のみに整理する基盤を作る。 * docs(lessons): agent-lessons.md から完了済 / memory 重複 6 lesson を削除 shared-agent-rules.md 11 章「教訓の運用」に従い、規約昇格済 / 完了済 / Claude memory に集約済の lesson を削除。継続検討中の 6 lesson のみに整理。 削除した 6 lesson: - [2026-05-07] Tailwind v4 @layer components hover variant 非対応 → shared-rules 7.1 章へ昇格 - [2026-05-01] subagent isolation:"worktree" 必須 → memory feedback_worktree_and_isolation に集約 - [2026-05-01] worktree 古い node_modules で E2E timeout → 後続 [062] で廃止確認済 (scripts/agent-worktree-setup.sh 削除) - [2026-05-01] PR 本文同期は親 → memory feedback_subagent_workflow (現 shared-rules 6.6) に集約 - [2026-05-01] worktree 内部 branch 取り違え → memory feedback_worktree_merge_order に集約 - [2026-05-02] subagent 絶対パス → memory feedback_worktree_and_isolation に集約 保持する 6 lesson: - [2026-04-28] QRチケット 160px (将来検討事項あり) - [2026-05-01] devDependency lock 同期 (規約昇格候補、但し現状 subagent 完了確認で十分対応可) - [2026-05-02] React effect/memo 矛盾指示 (プロンプト設計 lesson) - [2026-05-02] subagent 完了報告漏れ (規約昇格候補) - [2026-05-04] memory dir Bash rm 不可 (Claude Code harness bug、回避不能) - [2026-05-04] sandbox profile mirror 仮説 (未確認) * docs(claude): CLAUDE.md の PR 4 点必須を要約に圧縮し playbook を SoT 明示 旧表記の sub-bullet 詳細 (pre-create check 3 つを各行で展開等) は pr-creation.md 3 章と shared-agent-rules.md 6.x 章で完全カバー済のため、CLAUDE.md は要約 4 点と 正本 pointer に圧縮。同内容の二重管理による drift を防止。 * docs(rules): PR #296 review 指摘 3 件を反映 - 7.1 章末尾: markdown content scan 副次発見 (hover:bg-blue-50 等の utility 名リテラルを docs/ 配下から拾って unused utility が build CSS に混入する リスク + src/ コメント内では分割記述する) を追記。 - 6.7 章: 「PR 本文の更新は親で実行」 bullet を追加。gh pr edit --body-file は ask permission で subagent から非対話 deny される (PR #189 事例)。 - 6.3 章: 「main 向けはリリース PR のみ」を明記。release-only branch policy。 PR #296 review (Conditional Approve) で指摘された 4 件のうち #2 / #3 / #4 の対応。#1 (PR description 数字) は別途 gh pr edit で対応。
1 participant
概要
useCodecフックにisPendingフラグを追加し、deps（変換先フォーマットなど）が変化してからデバウンス完了までtrueを返すようにしたConfigConverterでこのフラグをDownloadButtonのdisabledprop に渡し、出力が確定するまでダウンロードを抑止する変更内容
src/hooks/useCodec.ts:isPending: boolean状態を追加。deps 変化直後true、デバウンス完了・入力クリア・reset でfalsesrc/components/tools/ConfigConverter.tsx:isPendingをDownloadButtonのdisabledに接続tests/e2e/config-converter.spec.ts: 変換先切り替え直後に disabled になり、完了後に有効化されることを検証する E2E テストを追加テスト
npm run test— 377 件全件 passastro check— 型エラー 0 件Closes #149
🤖 Generated with Claude Code