Skip to content

refactor(ui): #176 B 案 PR 2 — qr-ticket inline style 撤去 + #225 useMemo/abort 対応 - #272

Merged
fumtas1k merged 7 commits into
developfrom
feature/issue-176-b2-qr-ticket
May 4, 2026
Merged

refactor(ui): #176 B 案 PR 2 — qr-ticket inline style 撤去 + #225 useMemo/abort 対応#272
fumtas1k merged 7 commits into
developfrom
feature/issue-176-b2-qr-ticket

Conversation

@fumtas1k

@fumtas1k fumtas1k commented May 4, 2026

Copy link
Copy Markdown
Owner

概要

#176 B 案(style-src 'unsafe-inline' 削減)の PR 2 / 6: src/components/tools/qr-ticket/ 配下 3 ファイル(合計 42 件の JSX inline style)を @layer components の class + Tailwind utility に置換する。

同梱で #225 (refactor QrTicket) を解消する。

スコープ

inline style 撤去 (3 ファイル / 計 42 件)

File 件数
src/components/tools/qr-ticket/GenerateTab.tsx 27
src/components/tools/qr-ticket/VerifyTab.tsx 12
src/components/tools/qr-ticket/TicketDetail.tsx 3

3 ファイルとも import { bodyEmphasis, caption, colors } from '@/utils/styles' を完全削除。

同梱 issue (#225)

観点 対応
観点 1: useTicketKeyPair / useTicketGeneration 戻り値の不安定性 戻り値を useMemo で安定化(useQrCamera と同 pattern)。return shape は完全不変
観点 2: カメラ起動中アンマウント時の verify race useTicketVerificationverify に専用 AbortController を追加。external signal を link、signal.aborted で setState を抑制

inline-style-migration.test.tsMIGRATED_FILES 拡張

PR 2 で migrate した 3 ファイルを追加(13 → 16 件、tracker test 6 spec 増)。

src/styles/global.css 追加 class(PR 2 用 8 件、@layer components 内)

色 token utility:

  • .text-error / .text-error-text / .text-success / .text-primary

コンポーネント scoped:

  • .alert-success / .alert-error (検証結果ボックス)
  • .qr-file-picker-label + [data-enabled='true'] (画像アップロード label)
  • .badge-category (カテゴリバッジ pill)
  • .btn-row-remove + :disabled (チケット行削除)
  • .qr-result-grid (生成結果 auto-fill grid)

設計

詳細は docs/superpowers/specs/2026-05-04-issue-176-b2-qr-ticket-design.md 参照。主要な技術選択:

  • video / canvas / file input の表示制御: JSX inline style={{ display: ... }} を撤去し、HTML hidden 属性 (canvas / video) または sr-only Tailwind utility (file input、a11y 配慮) に置換
  • 動的 file picker label の状態: data-enabled={Boolean(...)} 属性 + CSS attribute selector (.qr-file-picker-label[data-enabled='true']) で表現。視覚状態と input 側の disabled 属性 (a11y) を責務分離
  • 検証結果ボックス: .alert-success / .alert-error の variant class で bg + border-color を切替、内部 typo は text-success / text-error
  • 動的 isOver / disabled state: 三項で className 切替(text-error font-semibold / text-muted 等)。CSSOM mutation は使わない
  • カテゴリバッジの padding 0.1rem 0.5rem: Tailwind utility (py-0.5 = 0.125rem) では VRT pixel 差が出るため、専用 .badge-category class で original 厳密一致

前提 PR / prerequisite

PR 内容 状態
#249 A-1 (script-src strict) merged
#254 VRT 基盤導入 merged
#256 PR 1 — 基礎工事 + ui/* simple 11 merged
#261 PR 1.5 — ResultTable + InputField merged
#268 #258 ClearButton / CopyButton type="button" merged
#270 #269 ToggleGroup / QrReader / Gs1Databar type="button" merged

検証

項目 結果
npm run test (vitest) 701 / 701 pass (43 test files 全 pass、migration tracker 35 spec pass)
npx astro check 0 errors / 0 warnings / 10 hints (既存)
npm run test:e2e 144 spec pass / 1 skip (既存 disabled)
migration test detector 3 ファイル全件で style={{ ヒット 0、.style.X = Y 形式 mutation 0
想定外ファイル変更 なし (git diff origin/develop --name-only で 11 ファイル全て想定通り)
aria-* / role / htmlFor 削除 なし(diff 行は multi-line → single-line 整形のみ、属性は新行に維持)

VRT (visual-regression)

CI Linux runner で 36 件 baseline 比較。required check 外。意図差分があれば PR ブランチで update-visual-baseline.ymlworkflow_dispatch trigger して baseline 更新(PR 1 / 1.5 と同フロー)。

バッチ計画における位置付け

# スコープ 状態
PR 0 VRT 基盤 merged (#254)
PR 1 基礎工事 + ui/* simple 11 merged (#256)
PR 1.5 ui/* complex (ResultTable + InputField) merged (#261)
PR 2 qr-ticket/* (本 PR) 本 PR
PR 3 JwtDecoder + UuidV7Generator 未着手
PR 4 Gs1Databar + EncodingConverter + DummyText 未着手
PR 5 QrReader + ConfigConverter + JanCode + QrCode + 残り tools 未着手
PR 6 flip + cleanup (CSP strict 化、stripMetaStyleSrc() 撤去、src/utils/styles.ts 削除) 未着手

レビュアー向けメモ

  • @layer components への新規 class 追加は YAGNI 厳守 (qr-ticket 固有の利用範囲に限定、再利用見込が出るのは text-error/success/primaryalert-success/error 程度)
  • 鍵 textarea (<textarea readOnly>) は周辺 layout (CopyButton 横並び) との結合を保つため InputField 置換は しない(spec §3.2 採用案)
  • data-enabled 属性は aria-disabled と責務分離。input 側 disabled 属性で a11y 表現は維持
  • useTicketVerification.verify の signal 引数は内部 controller と link するように変更したが、外部 callsite (useQrCamera.onQrDetected) は signal 未指定で呼ぶ既存挙動を維持

関連

  • 起源 issue: #176 (B 案 style-src 削減)
  • 同梱 issue: #225 (refactor QrTicket)
  • prerequisite issue: #258 (#268 で merged) / #269 (#270 で merged)
  • 過去 decisions: [054] (CSP 初導入) / [064] (A-1 採用) / [066] (VRT 採用)
  • spec: docs/superpowers/specs/2026-05-04-issue-176-b2-qr-ticket-design.md
  • plan: docs/superpowers/plans/2026-05-04-issue-176-b2-qr-ticket.md
  • PR 1 spec: docs/superpowers/specs/2026-05-03-issue-176-b1-foundation-and-ui-simple-design.md
  • PR 1.5 spec: docs/superpowers/specs/2026-05-04-issue-176-b1-5-ui-complex-design.md

fumtas1k added 5 commits May 4, 2026 18:10
Phase 0 (親 Opus 直接実行) のコミット:

- spec: docs/superpowers/specs/2026-05-04-issue-176-b2-qr-ticket-design.md
- plan: docs/superpowers/plans/2026-05-04-issue-176-b2-qr-ticket.md
- global.css: PR 2 用の @layer components 追記
  - text-error / text-error-text / text-success / text-primary
  - alert-success / alert-error
  - qr-file-picker-label / qr-file-picker-label[data-enabled='true']
  - badge-category
  - btn-row-remove / btn-row-remove:disabled
  - qr-result-grid

後続 Phase 1 で 3 track 並列 sonnet subagent が:
- Track A: GenerateTab.tsx (27 styles) inline style 撤去
- Track B: VerifyTab.tsx (12 styles) inline style 撤去
- Track C: TicketDetail.tsx (3 styles) + 3 hook (#225) + test 追加

を独立ファイル範囲で並列実装、Phase 2 で親 Opus が統合・検証・PR 作成する。
27 件の JSX inline style を @layer components の class + Tailwind utility に置換。
import { bodyEmphasis, caption, colors } from '@/utils/styles' を完全削除。

主な移行:
- 整理ボタン → .btn-link-plain + .text-link
- 鍵 textarea → caption font-mono w-full px-3 py-2 rounded-lg border border-input bg-surface
- expiry label → body-emphasis text-default block mb-3
- ヘッダ列 / モバイル label → caption text-muted font-semibold (+ leading-none)
- 動的 isOver → 三項で text-error font-semibold / text-muted
- 行削除ボタン → .btn-row-remove + min-w-10 min-h-10 p-3
- QR card → border border-default bg-surface
- QR container → w-40 h-40
- カテゴリバッジ → .badge-category
- 結果 grid → .qr-result-grid (auto-fill minmax(180px, 1fr))

挙動変化なし。a11y 属性 (aria-label / aria-controls / aria-expanded / aria-hidden) はすべて維持。
12 件の JSX inline style を @layer components の class + Tailwind utility に置換。
import { bodyEmphasis, caption, colors } from '@/utils/styles' を完全削除。

主な移行:
- caption + muted の <p> 4 ヶ所 → caption text-muted
- errorText の <p> → caption text-error-text
- video 表示切替 → className 静的 + hidden HTML 属性
- canvas → hidden HTML 属性
- file input → sr-only (a11y 配慮で display:none ではなく)
- file picker label 動的 styling → data-enabled 属性 + .qr-file-picker-label
- hint p → text-xs text-muted mt-1
- 検証結果ボックス → .alert-success / .alert-error + text-success / text-error

挙動変化なし。a11y 属性 (aria-label / aria-hidden / role / aria-live / aria-atomic) はすべて維持。
…同梱)

TicketDetail.tsx:
- 3 件の JSX inline style を caption / text-muted / text-default + tailwind utility に置換
- import { colors, caption } from '@/utils/styles' を完全削除

#225 観点 1 — useTicketKeyPair.ts / useTicketGeneration.ts:
- 戻り値を useMemo で安定化 (useQrCamera と同 pattern)
- return shape は完全不変。GenerateTab に props で渡す object identity を安定化

#225 観点 2 — useTicketVerification.ts:
- verify に専用 AbortController を追加
- unmount 時 cleanup で controllerRef.current?.abort()
- external signal を link、ctrl.signal.aborted を見て setState を抑制
- カメラ起動中アンマウント時の race (verify 完走後の setVerificationResult 発火) を防ぐ

useTicketVerification.test.tsx:
- 「unmount 後 verify が完走しても setState が走らない」陽性対照を 1 件追加
inline-style-migration.test.ts の MIGRATED_FILES array に PR 2 で移行した 3 ファイルを追加:
- src/components/tools/qr-ticket/GenerateTab.tsx
- src/components/tools/qr-ticket/VerifyTab.tsx
- src/components/tools/qr-ticket/TicketDetail.tsx

migration test は 6 spec 追加で 35 spec 全 pass。
PR 6 で MIGRATED_FILES を await glob('src/components/**\/*.tsx') 等の全件カバーに置換予定。
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

🖼️ Visual Regression Test 結果

  • Status: ✅ 全 36 件 pass
  • Workflow run: 25313514370
  • 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 判断)。

@fumtas1k fumtas1k self-assigned this May 4, 2026
GenerateTab.tsx の「既存の秘密鍵をインポート」<button> に `.text-link` を当てていたが、
`.text-link` は下線あり版 (`text-decoration: underline`) のため VRT で想定外の下線差分。

original inline style は `color: colors.link` のみで下線無し。色だけを制御する
`.text-link-color` (global.css line 144、PR 1 既存) に置換。

(spec §3.1 の記述「.text-link」は誤り。実体は色のみの `.text-link-color` が正しい)
@fumtas1k

fumtas1k commented May 4, 2026

Copy link
Copy Markdown
Owner Author

レビュー(並列 3 観点: security / frontend / rules)

3 つの subagent で並列レビューしました。Critical 1 件、Important 数件、規約遵守は概ね問題なしです。


🔴 Critical (修正必須)

useTicketKeyPair.tsuseMemo が機能していない

useTicketKeyPair.tsgenerateKeys / importKey / toggleImportuseCallback でラップされておらず、毎レンダリングで新参照が生成されます。これらは末尾の useMemo の deps 配列に含まれているため、deps が毎回変わって useMemo が再計算 → 戻り値オブジェクトのメモ化が 事実上無効 になります。

const generateKeys = async () => { ... };  // ← useCallback なし
const importKey = async () => { ... };     // ← useCallback なし
const toggleImport = () => setShowImport((v) => !v);  // ← useCallback なし

return useMemo(() => ({ ..., generateKeys, importKey, toggleImport, ... }),
  [..., generateKeys, importKey, toggleImport, ...]);

PR 説明の「観点 1: 戻り値の不安定性 → useMemo で安定化」が達成できておらず、#225 観点 1 の効果がゼロです。同 PR 内の useTicketGeneration.ts は全関数を useCallback でラップしていて非対称になっている点も修正の根拠になります。

推奨: generateKeys / importKey / toggleImportuseCallback でラップ。

  • generateKeys deps: [options](または options?.onPubKeyGenerated
  • importKey deps: [importStr, options]
  • toggleImport deps: []

🟡 Important (議論推奨)

1. useTicketVerification.verify — abort 後に verifying: true が残るパス

const result = await verifyTicket(rawData, pubKey);
if (ctrl.signal.aborted) return;   // ← ここで return すると setVerifying(false) がスキップ
setVerificationResult(result);
setVerifying(false);

verifyTicket() 待機中に abort された場合、setVerifying(false) が呼ばれずに verifying: true が残ります。実用上は unmount 時しか abort しないため state-after-unmount で no-op になりますが、setVerifying(true) 自体も abort された state に対して走り得ます。setVerifying(true) 直前の abort チェック後に async 境界がある時点で、対称な後始末(finallysetVerifying(false) か、abort 時に early setVerifying(false))を入れる方が安全です。

2. useTicketVerification.verify — externalSignal の link 設計

externalSignal.addEventListener('abort', () => ctrl.abort(), { once: true });

{ once: true } で自動解除されるためリーク自体は限定的ですが、外部 signal が長命だとクロージャ経由で ctrl を一時保持します。現 callsite (useQrCamera.onQrDetected) は signal を渡していないため実害なしですが、将来安全側に倒すなら AbortSignal.any([externalSignal, ctrl.signal]) への移行を検討する価値あり(要 polyfill 配慮)。

3. VerifyTab の検証結果ボックス — Tailwind utility と @layer components の優先度

<div className="rounded-lg p-4 border alert-success">

Tailwind の border utility と @layer components.alert-success を併用しており、Tailwind の borderborder-colorcurrentColor 系で出している場合、@layer components 側の border-color が後勝ちにならず期待色にならない可能性があります。実ブラウザで border-color の compute 値を確認してください(VRT が通っていれば実害は小さいですが、VRT は CI 限定の参考扱いです)。

4. useTicketVerification 型定義のドリフト

verify: (rawData: string, signal?: AbortSignal) => Promise<void>;

UseTicketVerificationReturn.verify の引数名は signal のまま、実装シグネチャは externalSignal にリネーム。型は問題ないですが、引数名の意図(外部からの link 用)が型から読み取りづらいので、型側も externalSignal にすると一貫します。

5. #176#225 の同梱 (rules 観点)

PR body で「同 hook ファイル群は同タイミングでないと merge conflict」と説明済みで根拠はあります。ただし #225 だけ先行小 PR 化する余地はあったかもしれません(hook 3 件 + テスト分の認知負荷増)。今回は許容範囲ですが、今後同種が続くなら判断軸を memory に残すと良いです。


🟢 Nit / 確認事項

  • global.css.text-primary 命名--color-primary と将来の Tailwind @theme auto utility が衝突するリスク。spec も rename 可と記載しているので懸念が顕在化したら text-brand 等へ。
  • GenerateTab.tsx:118 の class 名 spec ↔ 実装ドリフト — spec text-link / 実装 text-link-color<button> に下線不要なので実装の選択は妥当。spec 側の追記または rename を。
  • aria- 削除の grep 誤検知gh pr diff 272 | grep -E '^-.*aria-' で 1 件ヒットするが、これは multi-line → single-line 整形に伴う行結合で、属性自体は次行で維持されています(PR body の主張通り)。自動ゲートに ^-.*aria- を組み込む場合はこのフォールスを認識しておくべきです。
  • useTicketGeneration.generate の deps 解説cryptoKeyPairuseCallback deps に含めているのは正しい意図ですが、コメントで明示しておくと将来「不変だから外せる」と誤って削られる事故を防げます。

✅ 確認済み (問題なし)

セキュリティ / a11y / 規約

  • inline style 残留: src/components/tools/qr-ticket/**/*.tsx の追加行に style={{ なし、.style.X = Y mutation なし。
  • CSP 整合性: 追加 class はすべて var(--color-*) トークンと Tailwind utility のみ。style-src 'unsafe-inline' 撤去後も動作する設計。
  • aria-* / role / htmlFor 削除なし: PR body の主張通り、削除は単行整形に伴うもののみで属性は全件維持(aria-hidden / aria-label / aria-expanded / aria-controls / aria-live / role="status" / htmlFor)。
  • <button type="button">: import toggle / 行削除ボタンとも明示済み(#258 / #268 / #269 / #270 方針継続)。
  • file input の a11y: display: nonesr-only への置換は妥当(<label> wrap で keyboard / SR から起動可)。disabled 属性は input 側に維持。
  • data-enableddisabled の責務分離: 視覚状態 / a11y 状態の分離が機能している。
  • dangerouslySetInnerHTML: 新規導入なし。generateQrSvgqrcode ライブラリ内部生成で untrusted input は注入されない。
  • unmount 後の cleanup: useEffect の return で controllerRef.current?.abort() 実行、対応テストも追加済み。
  • useTicketGenerationuseMemo / useCallback: 全関数が useCallback でラップされ deps も妥当。memoization が実効的に機能している。

プロジェクトルール

  • 言語: PR タイトル・本文・全コミット日本語。
  • ベース: develop 一致。
  • スタイリング規約: Tailwind カラークラス新規追加なし。新規 CSS class は var(--color-*) トークン経由で色指定。
  • PR body の必須 4 項目: スコープ / 検証 / 想定外ファイル変更 / aria 削除いずれも記載済み(vitest 701/701 / astro check 0 errors / e2e 144 spec pass 含む)。
  • PR 規模: 実コード +279 / -254、commit 6 件。docs 込み 1226 行だが spec/plan は将来参照価値あり、過大ではない。
  • ATC 要件: docs/superpowers/plans/2026-05-04-issue-176-b2-qr-ticket.md が目的・フェーズ・スコープ外を明示しており tasks/active_context.md 不要ケースに該当。

総評

実装方針(inline style 撤去 / data-* attribute selector / @layer components への scoped class 化)と CSP 段階移行の整合は良好で、規約遵守も丁寧です。Critical の useCallback 漏れだけ修正すれば merge 候補になる品質と判断します。Important 1〜4 は同 PR or follow-up issue いずれでも可ですが、特に 1(abort 時 verifying 取り残し)は同梱した方が #225 観点 2 の完成度が上がります。

並列レビュー: security-engineer / frontend-architect / general-purpose の 3 subagent が独立に走り、親 (Opus) で Critical 指摘を git show <head> で直接検証。

… + 型 + spec)

PR #272 review で発見された問題を 4 件修正。

【Critical】useTicketKeyPair の useMemo が機能していない問題
- generateKeys / importKey / toggleImport が useCallback 未ラップで毎レンダ新参照
- 末尾 useMemo の deps に含まれているため、useMemo が毎回再計算 → memoization 無効
- 3 関数を useCallback でラップ。options?.onPubKeyGenerated は extract して
  callback deps を安定化 (options 自体は呼出側で毎レンダ新規 object literal)
- 同 PR 内の useTicketGeneration (全関数 useCallback ラップ) との非対称を解消

【Important 1】useTicketVerification.verify abort 後 verifying:true 残留
- await verifyTicket() 待機中に abort されると setVerifying(false) がスキップされる
- finally 句に setVerifying(false) を移動し、ctrl.signal.aborted false のときのみ実行
- abort 中は unmount 後 no-op になるため setState 抑制を維持

【Important 4】useTicketVerification 型定義のドリフト解消
- UseTicketVerificationReturn.verify の引数名 signal → externalSignal
- 実装シグネチャと一致させて意図 (外部からの link 用) を型から読めるように

【spec drift】§3.1 .text-link → .text-link-color に訂正
- 直前 commit 7c3bfc4 で実装は修正済 (.text-link は下線あり版で誤適用)
- spec 側にも訂正経緯を明記
@fumtas1k

fumtas1k commented May 4, 2026

Copy link
Copy Markdown
Owner Author

レビューありがとうございます。Critical 1 件 + Important 1 / 4 + spec drift を commit f282fad で対応しました。直前 commit 7c3bfc4 も含めて以下が修正済:

✅ 対応済 (commit f282fad / 7c3bfc4)

🔴 Critical: useTicketKeyPair の useMemo が機能していない

  • generateKeys / importKey / toggleImportuseCallback でラップ
  • options?.onPubKeyGenerated は extract して callback deps を安定化(options 自体は呼出側で毎レンダ新規 object literal なため deps 直接使用は不可)
  • useTicketGeneration との非対称を解消

🟡 Important 1: verify abort 後 verifying: true 残留

  • setVerifying(false)finally 句に移動
  • ctrl.signal.aborted === false のときのみ実行 (abort 中は unmount 後 no-op で setState 抑制)

🟡 Important 4: 型定義の引数名ドリフト

  • UseTicketVerificationReturn.verify の引数名 signalexternalSignal で実装と一致

🟢 Nit: spec ↔ 実装ドリフト (text-link vs text-link-color)

  • 直前 commit 7c3bfc4<button> を下線なし版 .text-link-color に修正済
  • spec §3.1 にも訂正経緯を明記(global.css line 144 既存の .text-link-color を使う)

⏸ Deferred (本 PR 外)

項目 理由
Important 2: AbortSignal.any 現 callsite (useQrCamera.onQrDetected) は signal 未指定で実害なし。polyfill 配慮も必要なため別 issue 候補
Important 3: Tailwind border.alert-successborder-color 優先度 VRT で alert-success / alert-error の表示は CI pass 済。実害があれば fix
Important 5: #176#225 同梱判断 本 PR では同 hook ファイル群への conflict 回避のため許容。「今後同種が続くなら判断軸を memory に残す」は引き続き検討
Nit: .text-primary 命名 --color-primary Tailwind auto utility との衝突は現状未顕在。@theme 切替時 (PR 6 cleanup) に再検討
Nit: aria-* grep 誤検知 既知 (multi-line → single-line 整形の行結合)。自動ゲート組込時は要考慮
Nit: useTicketGeneration.generate の deps コメント 軽微。次回 hook 触れる時に追記

検証

  • vitest (qr-ticket scope): 41 / 41 pass
  • astro check: 0 errors / 0 warnings
  • E2E: CI に委任(CLAUDE.md「post-PR 代行は不要、CI が最終ゲート」)
  • VRT: 直前 commit 7c3bfc4 で text-link 下線差分を解消 → 今 push で再走

CI green 確認後 merge をお願いします。

@fumtas1k

fumtas1k commented May 4, 2026

Copy link
Copy Markdown
Owner Author

Important 2 (AbortSignal.any 化) を follow-up issue として起票しました: #273

直前コメントで「別 issue 候補」と記載しながら起票し忘れていました。失礼しました。

他の deferred 項目の取り扱い(再整理)

項目 性質 対応
Important 2: AbortSignal.any 明示的な issue 候補 #273 として起票
Important 3: Tailwind border × .alert-* 優先度 実害があれば fix(reactive) issue 不要、VRT regression 検出時に対応
Important 5: #176+#225 同梱判断軸 memory に残す(issue ではない) 適切なタイミングで feedback_pr_bundle_judgment.md 等の memory entry 化候補
Nit: .text-primary 命名 PR 6 cleanup スコープ project_b_plan_progress.md の PR 6 チェックリストに追記する案あり、本 PR 外
Nit: aria-* grep 誤検知 documentation only 自動ゲート組込時に再確認、issue 不要
Nit: generate deps コメント 軽微 次回該当 hook 触る時に対応、issue 不要

Important 5 と Nit .text-primary は手元 TODO として保持しておきます。必要であれば後から issue 化も可能です。

@fumtas1k

fumtas1k commented May 4, 2026

Copy link
Copy Markdown
Owner Author

再レビュー (fix commit f282fad)

修正をローカルで git show 検証しました。Critical / 主要 Important は妥当に対応済みで、merge ブロッカーは解消されています。


✅ 修正確認 (Critical)

useTicketKeyPair.tsuseCallback 適用

useTicketKeyPair.ts:54-105

const onPubKeyGenerated = options?.onPubKeyGenerated;
const generateKeys = useCallback(async () => { ... onPubKeyGenerated?.(pubStr); ... }, [onPubKeyGenerated]);
const importKey    = useCallback(async () => { ... }, [importStr, onPubKeyGenerated]);
const toggleImport = useCallback(() => setShowImport((v) => !v), []);

callsite は QrTicket.tsx:25-29setVerifyPubKeyStr (React useState setter) を渡しており、これは React が identity 安定を保証する。options object 自体は毎レンダ新規だが、extract した onPubKeyGenerated 変数は同じ setter 参照に解決されるため deps が安定し、末尾 useMemo の memoization が実効的に機能します。#225 観点 1 達成。

軽微な懸念(任意): 将来 callsite が inline arrow (onPubKeyGenerated: () => {...}) を渡すと安定性が崩れる契約上の前提があります。JSDoc に「caller は callback を stable に保つこと」と一文足すと事故防止になりますが、blocking ではありません。


✅ 修正確認 (Important)

1. useTicketVerification.verifytry/finally

useTicketVerification.ts:55-79

全パスを以下で確認:

パス 旧挙動 新挙動
JSON parse / importPublicKey 失敗 (非 abort) inline setVerifying(false) finallysetVerifying(false)
verifyTicket 完走 (非 abort) inline setVerifying(false) finallysetVerifying(false)
任意 await 中に abort setVerifying(false) skip = stuck true finally!aborted ガードで意図的に skip(state-after-unmount 防止) ✅

abort された場合に verifying: true が残る挙動は変わらないが、コメント明記により「現 callsite (unmount 起因 abort のみ) では no-op」という前提が明示され、構造的にも finally でユニフォームに後始末されるようになりました。妥当な fix です。

4. 型ドリフト解消

UseTicketVerificationReturn.verify の引数名が externalSignal に揃いました。✅


✅ 修正確認 (Nit)

spec ↔ 実装のクラス名ドリフト

docs/superpowers/specs/2026-05-04-issue-176-b2-qr-ticket-design.md:237,255text-linktext-link-color への訂正と、選定根拠(.text-link は下線込みの <a> 用、本ボタンは色のみ必要)が追記されました。spec が一次情報として正しい状態に。✅


🟡 残置(任意 / blocking ではない)

初回レビューの以下は本 fix では未対応ですが、いずれも blocking でないことを再確認しました:


✅ 確認済み(追加)

  • callsite 整合: QrTicket.tsx:28-29useTicketKeyPair({ onPubKeyGenerated: setVerifyPubKeyStr }) の構造は変わらず、interface 互換維持。
  • 型 export 整合: UseTicketVerificationReturn.verify シグネチャと実装が一致。
  • テスト: useTicketVerification.test.tsx の unmount 後 abort 陽性対照テストが既存(mockReturnValueOnce(pendingVerify) で完了タイミング制御)— state-after-unmount が無音であることを陽性確認している。

総評

Critical / Important #1, #4 / Nit (spec) すべて妥当に対応済みで、再 review からの blocker はありません。残置の Important #2/#3/#5 はいずれも別 PR / follow-up issue で扱える性質のもので、本 PR の merge を保留する理由にはなりません。

LGTM。

親 (Opus) で git fetch origin pull/272/headgit diff 7c3bfc4..f282fad 直接検証。並列 subagent 再起動は不要と判断(修正範囲が limited で、初回レビュー指摘との 1:1 対応確認のみ必要だったため)。

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.

1 participant