Skip to content

refactor: 去重 + 修小漏洞集合 (Bundle A)#59

Closed
wnzzer wants to merge 2 commits into
mainfrom
refactor/quick-wins-cleanup
Closed

refactor: 去重 + 修小漏洞集合 (Bundle A)#59
wnzzer wants to merge 2 commits into
mainfrom
refactor/quick-wins-cleanup

Conversation

@wnzzer
Copy link
Copy Markdown
Owner

@wnzzer wnzzer commented May 13, 2026

Summary

整体 review 的 Bundle A(P1/P2 Quick Wins)。9 个低风险改动合一 PR,每条都独立验证过。

去重

  • isDark × 4 → useTheme()App.vue / components/gaming/PlayerCard.vue / components/record/MatchDetailModal.vue / components/record/RecordCard.vue 各自有一份相同的 computed(() => theme.name === 'Dark'|'dark'),但 composables/useTheme.ts 已经提供,且已被 useTheme.spec.ts 覆盖。删除各自副本,统一调用。
  • augmentRarityClass 去重RecordCard.vue 私有的版本与 utils/augment.ts 的通用版(已被 MatchDetailModal.vue 使用)功能等价,统一到 utils。新签名 (rarity, prefix) 由调用方传入查到的 rarity 和 class 前缀,更通用。

Bug fix

  • useSessionSync setTimeout 泄漏:retry 路径里的 setTimeoutonUnmounted 时未清理,组件卸载后定时器仍可能写已销毁的响应式状态。引入 trackedSetTimeout + pendingTimers Set,onUnmounted 统一 clearTimeout

小清理

  • Automation.vue:addPickData:删除调试遗漏的 console.log('addPickData', value)
  • MatchHistory.vue:两个 onMounted 合并;模块级 let name = ''computed(() => route.query.name as string),避免路由变更不触发重渲染时拿到过期值;championOption 导入路径从 deprecated 的 '../type' 改成 canonical @renderer/types/domain/champion
  • token.rs:auth_resolver:内联 Regex::new(...) 抽到 static AUTH_REGEX: LazyLock<Regex>。LCU 认证失败重试场景下避免每次重复编译。
  • pinia/setting.ts:initTheme 补注释:theme 这一项以裸字符串形式持久化(不是 { value: '...' } 包装),所以必须走原生 invoke 而不能用 services/ipc.tsgetConfigByIpc helper(后者会试图解 .value,对 scalar 配置会拿到 undefined)。一开始这块曾尝试改用 getConfigByIpc 并误称为"bug fix",实际上原版逻辑是正确的,已 revert,仅保留注释说明踩坑路径,避免后续 contributor 重复同一动作。

不在范围

  • getConfigByIpc<T> helper 本身的设计问题(假设后端永远返回 { value: T } 包装,但实际后端从未包装)——所有调用点都会拿到 undefined。需要全仓 grep 确认调用方并整体改造或下线 helper,单开 chore PR 推进。
  • 其他 review 发现的 P1(automation start_task 竞态 / listener 泄漏 / std::sync::Mutex in async / HTTP retry helper 抽取 / AppError 引入 / 工具链升级等)独立 PR 推进,本轮不涉及。

Test plan

  • npm run check:prettier ✓,eslint --fix ✓,vue-tsc 仅历史 @vue/test-utils 错误(PR 不引入新错误)
  • npm run test:210 tests pass,2 个 suite 因 @vue/test-utils 未装而加载失败(与本 PR 无关)
  • CI Windows-latest 全绿(含 cargo fmt --check + cargo clippy -Dwarnings;本机 macOS 跑不动 winapi)
  • npm run tauri dev 手测:
    • 切换深/浅色主题,所有页面(战绩、Gaming、Settings/About)颜色正确切换
    • 重启应用后主题持久化生效(dark / light 都验一次)
    • 战绩翻页(next/prev)名称参数正确
    • 进入/离开 Gaming 页面,DevTools Network 无残留 setTimeout 触发的 IPC
    • 启动 LCU + 反复重启,确认认证仍正常(Regex LazyLock 改动)

- isDark x4: App / PlayerCard / MatchDetailModal / RecordCard 改用 useTheme()
  composable,删除各自本地的 isDark computed 与不再用的 settingsStore 引用。
- RecordCard: augmentRarityClass 改调 utils/augment.ts 的通用版(与
  MatchDetailModal 对齐),删除本地副本。
- pinia/setting.ts: initTheme/toggleTheme 走 services/ipc.ts 的
  get/putConfigByIpc 封装。顺手修复 initTheme 之前用裸 invoke<string>
  接收 ConfigValue<string> 包装的隐藏 bug——之前 savedTheme 永远是
  { value: '...' } 对象,与字符串比较恒为 false,导致 light 模式启动后
  仍被回退为 dark。
- Automation.vue: 删调试遗漏的 console.log('addPickData', value)。
- MatchHistory.vue: 合并两个 onMounted;模块级 `let name = ''` 改为
  computed(() => route.query.name),消除路由变更不重渲染时的过期值风险。
- useSessionSync: setTimeout 改走 trackedSetTimeout,onUnmounted 中
  clearTimeout 所有未触发的 timer,避免组件卸载后写已销毁的响应式状态。
- token.rs: auth_resolver 的 Regex 抽到 static LazyLock,避免每次
  LCU 认证失败重试时重复编译正则。
Copilot AI review requested due to automatic review settings May 13, 2026 11:11
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

该 PR 将多个“低风险 Quick Wins”合并为 Bundle A:统一主题暗黑判断逻辑、抽取重复工具函数,并修复若干隐藏问题(配置读取、定时器清理、Regex 复用等),以降低重复代码与潜在运行时问题。

Changes:

  • useTheme() 去重多处 isDark 计算逻辑,并在相关组件内改用 composable。
  • 修复/改进:useSessionSync 的 retry setTimeout 在卸载时清理;Rust LCU token 解析 Regex 使用 LazyLock 静态编译一次。
  • 小清理与行为修正:移除遗留 console.log;MatchHistory 使用 computed 读取路由参数避免过期值;RecordCard 统一使用 utils/augment.tsaugmentRarityClass

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lol-record-analysis-tauri/src/views/settings/Automation.vue 移除遗留调试日志输出
lol-record-analysis-tauri/src/pinia/setting.ts 主题配置读写改用 IPC service helper(存在读写格式一致性风险)
lol-record-analysis-tauri/src/composables/useSessionSync.ts 为 retry 定时器增加跟踪与 onUnmounted 清理,避免卸载后写状态
lol-record-analysis-tauri/src/components/record/RecordCard.vue 去重 isDark;augment 稀有度 class 逻辑改用 utils 公共实现
lol-record-analysis-tauri/src/components/record/MatchHistory.vue 合并 onMounted;使用 computed name 避免路由变更后取旧值(仍引用 deprecated 类型)
lol-record-analysis-tauri/src/components/record/MatchDetailModal.vue 去重 isDark,改用 useTheme()
lol-record-analysis-tauri/src/components/gaming/PlayerCard.vue 去重 isDark,改用 useTheme()
lol-record-analysis-tauri/src/App.vue 去重 isDark,改用 useTheme()
lol-record-analysis-tauri/src-tauri/src/lcu/util/token.rs Regex 从函数内联改为 LazyLock 静态编译,减少重复开销
Comments suppressed due to low confidence (2)

lol-record-analysis-tauri/src/pinia/setting.ts:26

  • Theme persistence format is inconsistent with getConfigByIpc's unwrapping behavior: these writes store a raw string, but initTheme() now reads via getConfigByIpc (which expects { value: T }). Consider writing the same wrapped format used elsewhere (e.g., { value: 'light' }) or adjusting the IPC helper/read logic so strings/booleans/numbers stored directly still round-trip correctly.
  async function toggleTheme() {
    if (theme.value.name == 'dark') {
      theme.value = lightTheme
      await putConfigByIpc('theme', 'light')
    } else {
      theme.value = darkTheme
      await putConfigByIpc('theme', 'dark')
    }

lol-record-analysis-tauri/src/components/record/MatchHistory.vue:93

  • This file imports championOption from ../type, which is explicitly marked @deprecated (it re-exports from @renderer/types/domain/champion). Since this hunk already touches imports, switch to importing championOption directly from @renderer/types/domain/champion to avoid propagating deprecated types.
import { renderSingleSelectTag, renderLabel, filterChampionFunc } from '../composition'
import { modeOptions, initModeOptions } from './composition'
import { invoke } from '@tauri-apps/api/core'
import { championOption } from '../type'
import type { Game, MatchHistory } from './match'

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 10 to 13
async function initTheme() {
try {
const savedTheme = await invoke<string>('get_config', { key: 'theme' })
const savedTheme = await getConfigByIpc<string>('theme')
theme.value = savedTheme === 'light' ? lightTheme : darkTheme
回应 Copilot review (#59 review 4280964805):

1. pinia/setting.ts: 我之前那个"顺手修隐藏 bug"恰好搞反了。事实上:
   - Rust 的 config::Value 用 #[serde(untagged)],Value::String("dark")
     在 wire 上就是裸字符串 "dark",**不会**被包成 { value: "dark" }。
   - theme 这一项历史上一直裸值持久化,老的 invoke<string> 读法是对的。
   - 现存其他 putConfigByIpc 调用(useRules / Automation.vue)是手动包
     { value: ... } 写、getConfigByIpc 自动解 .value 读——这是另一套约定,
     theme 不属于这套。
   - 我换 helper 后写仍裸值、读试图解 .value → undefined → light 主题
     用户重启永远回退 dark。比改之前更糟。
   还原到原始 invoke 直调,附注释说明 theme 走 raw 路径的原因。

2. MatchHistory.vue: championOption 改从源 @renderer/types/domain/champion
   直接 import,不再走 ../type 的 @deprecated re-export,并改成 import type。
@wnzzer
Copy link
Copy Markdown
Owner Author

wnzzer commented May 13, 2026

Confirmed — Copilot 这条 review 完全正确,我之前 PR body 里那个"顺手修 initTheme 隐藏 bug"是基于错误的理解。

实际 wire 格式:Rust 端 command/config.rsget_config 返回 Result<config::Value, String>,而 config::Value#[serde(untagged)] —— Value::String("dark") 在 JSON 上就是裸字符串 `"dark"`,不会被包成 `{value: "dark"}`。

真实的代码状态

  • theme 一直裸值持久化(YAML 里就是 `theme: dark`),老的 `invoke('get_config', { key: 'theme' })` 是对的
  • 现存其他配置(useRules.ts, Automation.vue)走另一套约定:put 时手动包{ value: ... },get 由 getConfigByIpc 自动解 .value
  • theme 不属于第二套约定

我换 helper 后,写入仍裸值(没主动 wrap)、读出来 getConfigByIpc<string> 试图解 .valueundefined → 已配置 light 主题的用户重启永远回退 dark。比改之前更糟

修复(a6239c2):

  1. pinia/setting.ts 撤回到原始 invoke 直接调用,附注释说明 theme 为何走 raw 路径
  2. 一并解决了你的第二条建议:MatchHistory.vuechampionOption 改从源 @renderer/types/domain/champion 直接 import type,不再走 ../type@deprecated re-export

后续如果想统一这套配置约定,正确的修法应该是让 getConfigByIpc 容错两种返回形态(scalar / wrapped),单独 PR 推。

@wnzzer
Copy link
Copy Markdown
Owner Author

wnzzer commented May 16, 2026

Superseded by #60 — rebased the work onto current main (post-#58 / #61) and bundled with the Bundle B toolchain/CI/config work in a single PR. Both 'refactor: 去重 + 修小漏洞集合' and 'chore: 工具链升级 + CI 硬门禁 + 配置卫生' commits live in #60 now.

@wnzzer wnzzer closed this May 16, 2026
@wnzzer wnzzer deleted the refactor/quick-wins-cleanup branch May 30, 2026 08:48
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.

2 participants