Conversation
- Add MediaServer_CleanupCachesForFile IPC channel in shared types - Implement cleanupCachesForFile method in MediaServerService with asset hash calculation - Add TranscodeCacheCleanupResult type for cleanup operation results - Register IPC handler in main process to expose cleanup functionality - Expose cleanupCachesForFile API to renderer through preload bridge - Integrate cache cleanup into HomePage video deletion flow Changes: - IpcChannel: Add MediaServer_CleanupCachesForFile channel - MediaServerService: Add cleanupCachesForFile, calculateAssetHash, removeDirectoryIfExists methods - ipc.ts: Register handler for cleanup cache IPC channel - preload: Expose cleanupCachesForFile to renderer API - HomePage: Lookup video file path and trigger cache cleanup after deletion This enhancement ensures HLS segments and audio cache directories are automatically removed when users delete videos from the library, preventing unnecessary disk space usage and maintaining cache hygiene.
- remove 'hls-player-missing' error type from ExtendedErrorType - clean up related UI components and styled components - remove HLS-specific status display from VideoErrorRecovery modal
Walkthrough此变更新增媒体服务器针对单个文件的转码缓存清理能力:扩展 IpcChannel 枚举与主进程 IPC 处理器,MediaServerService 实现清理逻辑与结果类型,并在渲染端删除视频后异步触发清理。另包含若干 UI/逻辑微调(下载流程的超时管理、日志移除、错误类型与兼容性检查更新、体积小的状态处理修正)。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 用户
participant Renderer as 渲染进程<br/>HomePage
participant Preload as 预加载 API<br/>window.api.mediaServer
participant Main as 主进程<br/>ipcMain
participant Service as MediaServerService
User->>Renderer: 删除视频
Renderer->>Renderer: 解析 videoId -> fileId -> filePath(容错)
alt 已解析到 filePath
Renderer->>Preload: cleanupCachesForFile(filePath)
Preload->>Main: IpcChannel.MediaServer_CleanupCachesForFile(filePath)
Main->>Service: cleanupCachesForFile(filePath)
Service->>Service: 计算 assetHash / 定位 HLS/音频缓存目录
Service->>Service: 逐目录尝试删除并收集结果
Service-->>Main: TranscodeCacheCleanupResult
Main-->>Preload: 结果
Preload-->>Renderer: 结果(日志记录)
else 未解析到 filePath
Renderer->>Renderer: 跳过清理(不阻断删除)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/renderer/src/pages/settings/FFprobeSection.tsx (3)
64-68: 为完成态延时引入 ref 以便统一清理新增一个 ref 存储完成态的超时器,避免悬挂:
const isCompletionHandledRef = useRef(false) + const completionTimeoutRef = useRef<NodeJS.Timeout | null>(null) const [isValidatingPath, setIsValidatingPath] = useState(false)
137-142: 在 effect 清理阶段统一清理完成态超时器补充对 completionTimeoutRef 的清理,避免组件卸载后回调执行:
return () => { if (progressInterval) { clearInterval(progressInterval) } + if (completionTimeoutRef.current) { + clearTimeout(completionTimeoutRef.current) + completionTimeoutRef.current = null + } }
205-213: 选择文件对话框返回类型不匹配,导致路径不会更新main 中 IpcChannel.App_Select 返回的是选中路径字符串(或 null),此处按 { filePaths } 结构读取会失效。请改为直接使用字符串。
- const result = await window.api.select({ + const selectedPath = await window.api.select({ title: t('settings.plugins.ffprobe.path.browse_title'), properties: ['openFile'], filters: [{ name: 'FFprobe 可执行文件', extensions: ['exe', 'app', '*'] }] }) - if (result && result.filePaths && result.filePaths.length > 0) { - setFFprobePath(result.filePaths[0]) - } + if (selectedPath) { + setFFprobePath(selectedPath) + }src/main/services/UvBootstrapperService.ts (1)
433-437: 并发防护补强:同时检查 controller 映射,避免竞态重复下载当前仅通过 downloadProgress.has(key) 判断,建议同时检查 downloadController 以缩小竞态窗口。
- if (this.downloadProgress.has(key)) { + if (this.downloadProgress.has(key) || this.downloadController.has(key)) { logger.warn('uv 正在下载中', { platform, arch }) return false }src/renderer/src/pages/settings/FFmpegSection.tsx (2)
605-607: 统一样式 token:将 gap: 8px 改为 SPACING.XS与本项目样式指南一致,便于主题统一。
- gap: 8px; + gap: ${SPACING.XS}px;根据编码规范
245-253: 选择文件对话框返回类型不匹配,无法更新 FFmpeg 路径main 的 App_Select 返回 string(或 null),此处按 { filePaths } 结构读取会失败。改为直接使用字符串。
- const result = await window.api.select({ + const selectedPath = await window.api.select({ title: t('settings.plugins.ffmpeg.path.browse_title'), properties: ['openFile'], filters: [{ name: 'FFmpeg 可执行文件', extensions: ['exe', 'app', '*'] }] }) - if (result && result.filePaths && result.filePaths.length > 0) { - setFFmpegPath(result.filePaths[0]) - } + if (selectedPath) { + setFFmpegPath(selectedPath) + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
packages/shared/IpcChannel.ts(1 hunks)src/main/ipc.ts(1 hunks)src/main/services/MediaServerService.ts(3 hunks)src/main/services/UvBootstrapperService.ts(3 hunks)src/preload/index.ts(1 hunks)src/renderer/src/pages/home/HomePage.tsx(3 hunks)src/renderer/src/pages/player/components/PlayerSelector.tsx(1 hunks)src/renderer/src/pages/player/components/TranscodeLoadingIndicator.tsx(0 hunks)src/renderer/src/pages/player/components/VideoErrorRecovery.tsx(0 hunks)src/renderer/src/pages/player/components/VolumeIndicator.tsx(2 hunks)src/renderer/src/pages/settings/FFmpegSection.tsx(4 hunks)src/renderer/src/pages/settings/FFprobeSection.tsx(3 hunks)src/renderer/src/services/CodecCompatibilityChecker.ts(2 hunks)
💤 Files with no reviewable changes (2)
- src/renderer/src/pages/player/components/VideoErrorRecovery.tsx
- src/renderer/src/pages/player/components/TranscodeLoadingIndicator.tsx
🧰 Additional context used
📓 Path-based instructions (4)
src/renderer/src/**/*.{ts,tsx,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
优先使用 CSS 变量,避免硬编码样式值(颜色等)
Files:
src/renderer/src/pages/player/components/PlayerSelector.tsxsrc/renderer/src/pages/settings/FFmpegSection.tsxsrc/renderer/src/services/CodecCompatibilityChecker.tssrc/renderer/src/pages/player/components/VolumeIndicator.tsxsrc/renderer/src/pages/home/HomePage.tsxsrc/renderer/src/pages/settings/FFprobeSection.tsx
src/renderer/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
尺寸与时长等不要硬编码,优先使用 useTheme() 的 token 或集中样式变量(如 motionDurationMid、borderRadiusSM/MD)
Files:
src/renderer/src/pages/player/components/PlayerSelector.tsxsrc/renderer/src/pages/settings/FFmpegSection.tsxsrc/renderer/src/services/CodecCompatibilityChecker.tssrc/renderer/src/pages/player/components/VolumeIndicator.tsxsrc/renderer/src/pages/home/HomePage.tsxsrc/renderer/src/pages/settings/FFprobeSection.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: 定制 antd 组件样式优先使用 styled-components 包装 styled(Component),避免全局 classNames
项目的图标统一使用 lucide-react,不使用 emoji 作为图标
组件/Hook 顶层必须通过 useStore(selector) 使用 Zustand,禁止在 useMemo/useEffect 内部调用 store Hook
避免使用返回对象的 Zustand 选择器(如 useStore(s => ({ a: s.a, b: s.b })));应使用单字段选择器或配合 shallow 比较器
遵循 React「副作用与状态更新」规范:渲染纯函数、Effect 三分法、幂等更新、稳定引用、严格清理、禁止写回自身依赖、Provider 值 memo、外部状态 selector 稳定等
统一使用 loggerService 记录日志而不是 console
logger 使用示例中第二个参数必须为对象字面量(如 logger.error('msg', { error }))
任何组件或页面都不要写入 media 元素的 currentTime,播放器控制由编排器统一负责
在 styled-components 中:主题相关属性使用 AntD CSS 变量(如 var(--ant-color-bg-elevated));
在 styled-components 中:设计系统常量(尺寸、动画、层级、字体、毛玻璃等)使用 JS 常量(如 SPACING、BORDER_RADIUS、Z_INDEX、FONT_SIZES 等)
Files:
src/renderer/src/pages/player/components/PlayerSelector.tsxsrc/preload/index.tssrc/renderer/src/pages/settings/FFmpegSection.tsxsrc/main/ipc.tspackages/shared/IpcChannel.tssrc/main/services/UvBootstrapperService.tssrc/renderer/src/services/CodecCompatibilityChecker.tssrc/renderer/src/pages/player/components/VolumeIndicator.tsxsrc/renderer/src/pages/home/HomePage.tsxsrc/renderer/src/pages/settings/FFprobeSection.tsxsrc/main/services/MediaServerService.ts
**/player/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Player 页面:统一在组件顶层使用 Zustand selector,禁止在 useMemo/useEffect 内调用 store Hook;useSubtitleEngine 通过参数传入 subtitles 等防御处理
Files:
src/renderer/src/pages/player/components/PlayerSelector.tsxsrc/renderer/src/pages/player/components/VolumeIndicator.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-17T14:59:36.985Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T14:59:36.985Z
Learning: Applies to **/player/**/*.{ts,tsx} : Player 页面:统一在组件顶层使用 Zustand selector,禁止在 useMemo/useEffect 内调用 store Hook;useSubtitleEngine 通过参数传入 subtitles 等防御处理
Applied to files:
src/renderer/src/pages/player/components/PlayerSelector.tsxsrc/renderer/src/pages/player/components/VolumeIndicator.tsx
🧬 Code graph analysis (4)
src/renderer/src/pages/settings/FFmpegSection.tsx (1)
src/renderer/src/infrastructure/styles/theme.ts (2)
SPACING(43-58)FONT_SIZES(25-40)
src/main/ipc.ts (1)
src/main/services/MediaServerService.ts (1)
mediaServerService(722-722)
src/renderer/src/pages/settings/FFprobeSection.tsx (1)
src/renderer/src/infrastructure/styles/theme.ts (1)
SPACING(43-58)
src/main/services/MediaServerService.ts (1)
src/main/utils/index.ts (1)
getDataPath(11-17)
🪛 GitHub Actions: Test
src/renderer/src/pages/settings/FFprobeSection.tsx
[error] 94-94: TypeScript error TS7030: Not all code paths return a value.
🔇 Additional comments (21)
src/renderer/src/pages/player/components/VolumeIndicator.tsx (1)
2-2: 改进:避免初次挂载时显示指示器。使用
useRef跟踪初次渲染状态,并在useEffect中跳过首次执行,是防止组件挂载时不必要地显示音量指示器的标准做法。这样可以确保指示器仅在用户主动调整音量或静音状态时显示,提升了用户体验。实现方式符合 React Hooks 最佳实践,且遵循了编码规范(在组件顶层使用 Zustand selector)。
Also applies to: 16-16, 20-24
src/renderer/src/services/CodecCompatibilityChecker.ts (4)
379-380: 代码改进良好!使用可选链和
trim()方法处理编解码器信息,能够正确处理可能为undefined或包含空白字符的情况,增强了代码的健壮性。
390-393: 验证无音轨视频的处理逻辑。当前逻辑将缺失的编解码器视为不支持。对于有些合法的无音轨视频(如静音视频),
audioCodec会是undefined,导致audioSupported为false,进而触发转码(第 396 行:needsTranscode = !videoSupported || !audioSupported)。请确认这是否符合预期行为,或者需要区分"编解码器缺失"和"合法的无音轨"两种情况。
400-409: 错误信息更详细和精确!新增了对缺失编解码器的明确检测,为
incompatibilityReasons提供了更细粒度的错误信息(video-codec-missing、audio-codec-missing),有助于调试和用户理解问题根源。注意:此处的逻辑与第 390-393 行的条件检查一致,如果需要调整无音轨视频的处理,这两处需要同步修改。
257-266: 确认ExtendedErrorType移除安全
已在所有.ts/.tsx文件中搜索,无任何hls-player-missing引用,可安全移除该错误类型。src/renderer/src/pages/settings/FFprobeSection.tsx (2)
420-422: 间距使用主题 token,已对齐规范,赞gap 改为使用 SPACING token,符合样式规范。
547-548: 路径输入容器间距 token 化,赞gap 改为 SPACING.XS,和其他页面保持一致。
src/main/services/UvBootstrapperService.ts (2)
576-581: finally 统一清理临时目录,流程更健壮将临时目录清理集中到 finally,成功/失败/取消都能回收资源,赞。
416-431: 已存在二进制与本机架构复用优化合理优先复用已下载或本机已安装的 uv,减少无谓下载,👍。
若希望进一步降低“近同时触发”的重复下载概率,可考虑在 downloadUv 内在调用 performDownload 前就设置占位进度(或锁)。
packages/shared/IpcChannel.ts (1)
129-131: IPC 枚举新增项命名与命名空间一致,赞与现有 MediaServer_* 约定一致。
src/renderer/src/pages/settings/FFmpegSection.tsx (3)
153-166: 完成态定时器用 ref 管理与清理到位,赞避免悬挂回调与内存泄漏,逻辑正确。
177-181: 在 effect 清理阶段关闭完成态超时器,妥当防止卸载后执行回调。
480-482: 状态文本样式改为主题 token,👍gap 与字体大小均来自 theme,风格统一。
src/preload/index.ts (1)
335-338: 新增 cleanupCachesForFile API 已完全验证IpcChannel 枚举、preload 暴露、ipcMain.handle 注册及 MediaServerService.cleanupCachesForFile 方法定义均正确。
src/main/ipc.ts (1)
654-656: 新增 IPC 处理器实现合理
已确认TranscodeCacheCleanupResult类型声明存在,请手动确认MediaServerService.cleanupCachesForFile方法在MediaServerService类中已正确实现并返回该类型。src/renderer/src/pages/home/HomePage.tsx (3)
3-3: LGTM!导入 FileManager 是实现新的缓存清理功能所必需的。
168-181: LGTM!文件路径查找逻辑正确且错误处理得当:
- 使用 try-catch 确保查找失败不会阻止视频删除
- 正确使用空安全操作符(
?.和??)- 日志记录符合编码规范
193-207: LGTM!缓存清理触发逻辑实现正确:
- 使用
void关键字标识异步即发即弃操作,不阻塞用户体验- 错误处理完善,清理失败不会影响删除操作
- 日志记录符合编码规范
src/main/services/MediaServerService.ts (3)
1-4: LGTM!新增的导入语句支持缓存清理功能所需的文件操作和哈希计算。
61-73: LGTM!接口设计合理,提供了详细的清理结果信息,支持部分成功的场景(一个目录删除成功,另一个失败)。
573-605: LGTM!哈希计算实现正确且高效:
- 使用异步的
stat()和open()方法- 正确使用
try-finally确保文件句柄关闭- 针对大文件优化,仅读取头尾样本(各 8MB)
- 哈希计算包含路径、大小、修改时间和内容样本,确保唯一性
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/main/services/MediaServerService.ts (1)
608-620: 异步实现一致性已改好,但“removed=true”可能与“原本不存在”不区分你已移除同步 existsSync 并用 rm(force) 简化实现,风格一致,👍。若仍需区分“原本不存在”,可采纳此前建议的变体(删除前用异步 stat 预判,或删除后不区分)——取决于上层语义需求。
基于上一条评论的脚本,确认上层是否依赖区分;若需要,我可给出最小改动版异步实现以返回 existed/removed 的组合状态。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/main/services/MediaServerService.ts(3 hunks)src/main/services/UvBootstrapperService.ts(3 hunks)src/renderer/src/pages/player/components/PlayerSelector.tsx(1 hunks)src/renderer/src/pages/settings/FFprobeSection.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: 定制 antd 组件样式优先使用 styled-components 包装 styled(Component),避免全局 classNames
项目的图标统一使用 lucide-react,不使用 emoji 作为图标
组件/Hook 顶层必须通过 useStore(selector) 使用 Zustand,禁止在 useMemo/useEffect 内部调用 store Hook
避免使用返回对象的 Zustand 选择器(如 useStore(s => ({ a: s.a, b: s.b })));应使用单字段选择器或配合 shallow 比较器
遵循 React「副作用与状态更新」规范:渲染纯函数、Effect 三分法、幂等更新、稳定引用、严格清理、禁止写回自身依赖、Provider 值 memo、外部状态 selector 稳定等
统一使用 loggerService 记录日志而不是 console
logger 使用示例中第二个参数必须为对象字面量(如 logger.error('msg', { error }))
任何组件或页面都不要写入 media 元素的 currentTime,播放器控制由编排器统一负责
在 styled-components 中:主题相关属性使用 AntD CSS 变量(如 var(--ant-color-bg-elevated));
在 styled-components 中:设计系统常量(尺寸、动画、层级、字体、毛玻璃等)使用 JS 常量(如 SPACING、BORDER_RADIUS、Z_INDEX、FONT_SIZES 等)
Files:
src/main/services/MediaServerService.tssrc/renderer/src/pages/player/components/PlayerSelector.tsxsrc/main/services/UvBootstrapperService.tssrc/renderer/src/pages/settings/FFprobeSection.tsx
src/renderer/src/**/*.{ts,tsx,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
优先使用 CSS 变量,避免硬编码样式值(颜色等)
Files:
src/renderer/src/pages/player/components/PlayerSelector.tsxsrc/renderer/src/pages/settings/FFprobeSection.tsx
src/renderer/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
尺寸与时长等不要硬编码,优先使用 useTheme() 的 token 或集中样式变量(如 motionDurationMid、borderRadiusSM/MD)
Files:
src/renderer/src/pages/player/components/PlayerSelector.tsxsrc/renderer/src/pages/settings/FFprobeSection.tsx
**/player/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Player 页面:统一在组件顶层使用 Zustand selector,禁止在 useMemo/useEffect 内调用 store Hook;useSubtitleEngine 通过参数传入 subtitles 等防御处理
Files:
src/renderer/src/pages/player/components/PlayerSelector.tsx
🧠 Learnings (3)
📚 Learning: 2025-09-17T14:59:36.985Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T14:59:36.985Z
Learning: Applies to **/player/**/*.{ts,tsx} : Player 页面:统一在组件顶层使用 Zustand selector,禁止在 useMemo/useEffect 内调用 store Hook;useSubtitleEngine 通过参数传入 subtitles 等防御处理
Applied to files:
src/renderer/src/pages/player/components/PlayerSelector.tsx
📚 Learning: 2025-09-17T14:59:36.985Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T14:59:36.985Z
Learning: Applies to **/*.{ts,tsx} : 遵循 React「副作用与状态更新」规范:渲染纯函数、Effect 三分法、幂等更新、稳定引用、严格清理、禁止写回自身依赖、Provider 值 memo、外部状态 selector 稳定等
Applied to files:
src/renderer/src/pages/player/components/PlayerSelector.tsx
📚 Learning: 2025-09-17T14:59:36.985Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T14:59:36.985Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx} : 尺寸与时长等不要硬编码,优先使用 useTheme() 的 token 或集中样式变量(如 motionDurationMid、borderRadiusSM/MD)
Applied to files:
src/renderer/src/pages/settings/FFprobeSection.tsx
🧬 Code graph analysis (2)
src/main/services/MediaServerService.ts (1)
src/main/utils/index.ts (1)
getDataPath(11-17)
src/renderer/src/pages/settings/FFprobeSection.tsx (1)
src/renderer/src/infrastructure/styles/theme.ts (2)
SPACING(43-58)FONT_SIZES(25-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (windows-latest, 20)
- GitHub Check: test (macos-latest, 20)
- GitHub Check: test (ubuntu-latest, 20)
🔇 Additional comments (8)
src/renderer/src/pages/settings/FFprobeSection.tsx (5)
67-67: LGTM!ref 声明正确,类型定义清晰,用于管理成功状态的延迟超时。
121-131: 正确修复了超时泄漏问题!此修改完全解决了过往评审中提到的 TS7030 问题:
- ✅ 不再在异步回调中返回清理函数
- ✅ 使用 ref 持有超时 ID
- ✅ 设置新超时前先清除已存在的超时
- ✅ 超时完成后将 ref 置为 null
- ✅ 在外层 effect 清理中统一处理 ref
实现逻辑清晰且符合 React 最佳实践。
140-143: LGTM!cleanup 逻辑正确且完整,配合超时设置形成了完整的生命周期管理,有效防止内存泄漏。
426-427: 正确使用主题常量!完全符合编码规范要求:
- ✅ gap 使用
SPACING.XS替代硬编码- ✅ font-size 使用
FONT_SIZES.SM替代硬编码 14px此修改解决了过往评审中标记的样式硬编码问题,提升了样式的可维护性和主题化能力。
根据编码规范
553-553: LGTM!使用
SPACING.XS常量替代硬编码值,与文件其他样式修改保持一致,符合编码规范。根据编码规范
src/main/services/UvBootstrapperService.ts (1)
478-481: 目录初始化前置与 finally 统一清理是正确的
- 提前计算并创建 platformDir/targetDir/tempDir,避免作用域外变量/清理遗漏。
- 将清理集中在 finally,保证成功/失败都能清理临时目录。
LGTM。
Also applies to: 580-581
src/main/services/MediaServerService.ts (2)
551-556: 按文件清理流程与结果聚合设计合理
- 资产哈希、HLS/音频路径计算与逐路径清理、结构化结果返回均清晰。
- 日志第二参数均为对象字面量,符合规范。
LGTM。
Also applies to: 558-572
60-73: 不需要调整 removed 语义Renderer 端只是将
TranscodeCacheCleanupResult全量记录到日志,未基于removed字段区分“已清理”/“不存在”,当前不影响用户展示。后续若真有差异化需求,可再引入existed字段或调整返回值。
| // 检查目标平台的缓存二进制 | ||
| const downloadedPath = this.getDownloadedUvPath(platform, arch) | ||
| if (downloadedPath) { | ||
| logger.info('uv 已存在,跳过下载', { platform, arch, path: downloadedPath }) | ||
| return true | ||
| } | ||
|
|
||
| // 仅当请求的平台与当前进程一致时,才额外使用缓存的系统检测结果 | ||
| if (platform === process.platform && arch === process.arch) { | ||
| const installation = await this.checkUvInstallation() | ||
| if (installation.exists && installation.isDownloaded) { | ||
| logger.info('uv 已存在,跳过下载', { platform, arch, path: installation.path }) | ||
| return true | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
下载早退出逻辑总体 OK,但第二段“缓存检测”有冗余
前置的 getDownloadedUvPath 已覆盖“已下载二进制存在”的判断;随后的 checkUvInstallation 分支在仅限 isDownloaded=true 的条件下再次做相同判断,实际不会新增命中场景,反而增加一次额外 I/O/子进程探测的可能。
建议移除 423-431 的二次检测,保持单一早返回路径,降低复杂度与开销。
🤖 Prompt for AI Agents
In src/main/services/UvBootstrapperService.ts around lines 416 to 431, the
second cache check using checkUvInstallation (lines ~423-431) is redundant
because getDownloadedUvPath already covers the "downloaded binary exists" case
and the extra branch only repeats that check while adding I/O/subprocess
overhead; remove the entire conditional block that checks platform ===
process.platform && arch === process.arch and its inner checkUvInstallation
logic so the function relies on the single early return from
getDownloadedUvPath.
| // 检查是否正在下载 | ||
| if (this.downloadProgress.has(key)) { | ||
| if (this.downloadProgress.has(key) || this.downloadController.has(key)) { | ||
| logger.warn('uv 正在下载中', { platform, arch }) | ||
| return false | ||
| } |
There was a problem hiding this comment.
存在竞态:并发调用可能绕过“正在下载”检测,导致重复下载与文件竞争
当前“是否在下载”的检查发生在设置 downloadController/progress 之前(控制器在 performDownload 内创建)。两个近同时的 downloadUv 调用可能同时通过 432-436 的检查,随后分别进入 performDownload 启动两次下载,冲突地写入同一 tempDir/finalPath。
建议在启动下载前“占位”控制器,或使用 per-key 去重 Promise。最小改动如下:在 downloadUv 中预先创建并登记 AbortController,并将其传入 performDownload;performDownload 若收到外部 controller 则复用,不再新建,从而消除竞态窗口。
可按此 diff 修改:
@@
public async downloadUv(
platform = process.platform as Platform,
arch = process.arch as Arch,
onProgress?: (progress: DownloadProgress) => void
): Promise<boolean> {
const key = `${platform}-${arch}`
@@
- // 检查是否正在下载
+ // 检查是否正在下载
if (this.downloadProgress.has(key) || this.downloadController.has(key)) {
logger.warn('uv 正在下载中', { platform, arch })
return false
}
@@
logger.info('开始下载 uv', {
platform,
arch,
version: version.version,
url: version.url
})
- return await this.performDownload(platform, arch, version, onProgress)
+ // 预留控制器,避免竞态窗口
+ const reservation = new AbortController()
+ this.downloadController.set(key, reservation)
+ try {
+ return await this.performDownload(platform, arch, version, onProgress, reservation)
+ } catch (e) {
+ // 交由 performDownload finally 释放
+ throw e
+ }
}
@@
- private async performDownload(
+ private async performDownload(
platform: Platform,
arch: Arch,
version: UvVersion,
- onProgress?: (progress: DownloadProgress) => void
+ onProgress?: (progress: DownloadProgress) => void,
+ controller?: AbortController
): Promise<boolean> {
const key = `${platform}-${arch}`
- const controller = new AbortController()
- this.downloadController.set(key, controller)
+ const ctrl = controller ?? new AbortController()
+ // 若 downloadUv 未预占位(向后兼容),此处登记
+ if (!this.downloadController.has(key)) {
+ this.downloadController.set(key, ctrl)
+ }
@@
- await this.downloadFile(
+ await this.downloadFile(
version.url,
downloadPath,
(percent, downloaded, total, speed) => {
progress.percent = percent
progress.downloaded = downloaded
progress.total = total
progress.speed = speed
progress.remainingTime = speed > 0 ? (total - downloaded) / speed : 0
onProgress?.(progress)
},
- controller.signal
+ ctrl.signal
)
@@
} finally {
this.downloadProgress.delete(key)
this.downloadController.delete(key)
this.cleanupTempDir(tempDir)
}Also applies to: 457-466, 489-501, 576-581
| useEffect(() => { | ||
| if (hlsMode && hlsSrc) { | ||
| if (playerType !== 'hls') { | ||
| logger.info('检测到 HLS 模式激活,切换到 HLS 播放器', { hlsSrc }) | ||
| setPlayerType('hls') | ||
| } | ||
| logger.info('检测到 HLS 模式激活,切换到 HLS 播放器', { hlsSrc }) | ||
| setPlayerType('hls') | ||
| } else if (!hlsMode) { | ||
| if (playerType !== 'native') { | ||
| logger.info('HLS 模式关闭,切换回原生播放器') | ||
| setPlayerType('native') | ||
| } | ||
| logger.info('HLS 模式关闭,切换回原生播放器') | ||
| setPlayerType('native') | ||
| } | ||
| }, [hlsMode, hlsSrc, playerType]) | ||
| }, [hlsMode, hlsSrc]) |
There was a problem hiding this comment.
需要处理 HLS 模式激活但 hlsSrc 未就绪的边缘情况。
当 hlsMode 为 true 但 hlsSrc 尚未设置时,Effect 的两个分支都不会执行:
- 第 45 行要求
hlsMode && hlsSrc都为真 - 第 48 行要求
!hlsMode为真
这会导致 playerType 保持之前的状态,可能造成状态与渲染不一致:
- 若
playerType为'hls',渲染逻辑(第 55 行)检查playerType === 'hls' && hlsSrc会失败,回退到原生播放器 - 此时状态显示 HLS 模式,但实际渲染的是原生播放器
建议添加明确的边缘情况处理:
useEffect(() => {
if (hlsMode && hlsSrc) {
logger.info('检测到 HLS 模式激活,切换到 HLS 播放器', { hlsSrc })
setPlayerType('hls')
- } else if (!hlsMode) {
+ } else {
+ // hlsMode 为 false,或 hlsMode 为 true 但 hlsSrc 未就绪时,都回退到原生播放器
logger.info('HLS 模式关闭,切换回原生播放器')
setPlayerType('native')
}
}, [hlsMode, hlsSrc])这样可以确保在 HLS 源未就绪时明确回退到原生播放器,避免状态不一致。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (hlsMode && hlsSrc) { | |
| if (playerType !== 'hls') { | |
| logger.info('检测到 HLS 模式激活,切换到 HLS 播放器', { hlsSrc }) | |
| setPlayerType('hls') | |
| } | |
| logger.info('检测到 HLS 模式激活,切换到 HLS 播放器', { hlsSrc }) | |
| setPlayerType('hls') | |
| } else if (!hlsMode) { | |
| if (playerType !== 'native') { | |
| logger.info('HLS 模式关闭,切换回原生播放器') | |
| setPlayerType('native') | |
| } | |
| logger.info('HLS 模式关闭,切换回原生播放器') | |
| setPlayerType('native') | |
| } | |
| }, [hlsMode, hlsSrc, playerType]) | |
| }, [hlsMode, hlsSrc]) | |
| useEffect(() => { | |
| if (hlsMode && hlsSrc) { | |
| logger.info('检测到 HLS 模式激活,切换到 HLS 播放器', { hlsSrc }) | |
| setPlayerType('hls') | |
| } else { | |
| // hlsMode 为 false,或 hlsMode 为 true 但 hlsSrc 未就绪时,都回退到原生播放器 | |
| logger.info('HLS 模式关闭,切换回原生播放器') | |
| setPlayerType('native') | |
| } | |
| }, [hlsMode, hlsSrc]) |
🤖 Prompt for AI Agents
In src/renderer/src/pages/player/components/PlayerSelector.tsx around lines 44
to 52, the effect doesn't handle the edge case where hlsMode is true but hlsSrc
is not yet available, leaving playerType stale and causing render mismatch;
update the effect so that when hlsMode is true and hlsSrc is falsy you
explicitly setPlayerType('native') (and log a clear message) to ensure the UI
falls back to the native player until hlsSrc becomes ready, while keeping the
existing branches for hlsMode && hlsSrc and !hlsMode.
| logger.info('检测到 HLS 模式激活,切换到 HLS 播放器', { hlsSrc }) | ||
| setPlayerType('hls') |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
可选优化:避免冗余日志。
移除守护条件后,即使 playerType 未发生变化,日志也会记录切换操作。虽然 React 的 setState 会跳过值相同的更新(使用 Object.is 比较),但日志记录仍会执行,可能产生冗余信息。
如果希望避免冗余日志,可以保留守护条件但仅用于包裹日志调用:
useEffect(() => {
if (hlsMode && hlsSrc) {
- logger.info('检测到 HLS 模式激活,切换到 HLS 播放器', { hlsSrc })
+ if (playerType !== 'hls') {
+ logger.info('检测到 HLS 模式激活,切换到 HLS 播放器', { hlsSrc })
+ }
setPlayerType('hls')
} else {
- logger.info('HLS 模式关闭,切换回原生播放器')
+ if (playerType !== 'native') {
+ logger.info('HLS 模式关闭,切换回原生播放器')
+ }
setPlayerType('native')
}
}, [hlsMode, hlsSrc])注意:此优化需要将 playerType 重新加入依赖数组。
Also applies to: 49-50
🤖 Prompt for AI Agents
In src/renderer/src/pages/player/components/PlayerSelector.tsx around lines
46-47 (and similarly for lines 49-50), logging is executed unconditionally which
can produce redundant entries even when playerType won't actually change; wrap
the logger.info calls with a guard that checks if playerType !== 'hls' (or !==
'flv' for the other case) before logging and then call setPlayerType, and
restore playerType to the useEffect dependency array so the guard reflects the
current state.
- trim video and audio codec strings to avoid whitespace issues - ensure compatibility checks handle undefined codec values - add missing codec reasons to incompatibility list - improve robustness of codec detection logic
- Move the creation of target and temp directories outside of the try block for better scope management. - Ensure the cleanup of temporary directories in the finally block to prevent leftover files after download completion or failure. - Improve resource management by guaranteeing temp files are removed regardless of download success.
- Add useRef to track initial mount state - Prevent volume indicator from showing on component's first render - Ensure indicator only appears on subsequent volume changes This change enhances user experience by avoiding unnecessary indicator display during initial component load.
…h checks - Add logic to check for cached binary paths before initiating downloads - Ensure system checks are only performed when platform and architecture match the current process - Improve logging to reflect when UV binaries are already present and downloads are skipped - Refactor download logic to optimize performance and reduce unnecessary downloads This update optimizes the UV download process by leveraging cached paths and system checks, ensuring efficient resource utilization and improved performance.
- Add cleanup for success timeout to prevent potential memory leaks - Ensure proper handling of download completion state transitions This fix addresses the issue of unhandled timeouts after FFprobe download success, ensuring resources are properly managed.
- Replace hardcoded gap values with theme-based spacing constants - Ensure consistent styling across components using SPACING.XS This change improves maintainability and consistency in the styling of the FFprobeSection component by utilizing predefined theme constants.
- Add a ref to handle completion timeout for download success state - Clear existing timeout before setting a new one to prevent overlapping - Ensure timeout is cleared on component unmount to avoid memory leaks This update ensures that the download completion state is managed properly, preventing potential issues with overlapping timeouts and ensuring proper cleanup.
…display - Eliminate logger usage for state changes in TranscodeLoadingIndicator - Simplify component by removing unnecessary debug logs This change reduces the complexity of the component by removing logging that was not essential for the functionality of the loading indicator.
…g function - Ensure proper cleanup by adding an empty return statement in the download progress polling function. - This change prevents potential issues with function execution flow and enhances code readability.
- Add successTimeoutRef to handle and clear timeout for success state transition - Ensure proper cleanup of success timeout on component unmount - Prevent multiple timeout instances by clearing existing timeout before setting a new one These changes improve the reliability of the download success state transition by ensuring timeouts are properly managed and cleaned up.
- Update font size in StatusContainer to use FONT_SIZES.SM from theme constants for consistency across the application. This change ensures uniformity in font sizing, improving the visual coherence of the UI.
…le existence check - Remove synchronous fs.existsSync checks and replace them with asynchronous stat calls to improve non-blocking behavior. - Simplify logic in removeDirectoryIfExists by directly attempting to remove directories without prior existence check. - Enhance error handling for file existence validation in cleanupCachesForFile method.
…download controllers - Add condition to check for existing download controllers to avoid initiating multiple downloads for the same platform and architecture. - Improve logging to warn when a download is already in progress.
- Add specific error handling for ENOENT code in cleanupCachesForFile method - Ensure other errors are propagated correctly
d57d7de to
06842a9
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/main/services/UvBootstrapperService.ts (2)
416-430: ** 第二段缓存检测仍然冗余**第一次
getDownloadedUvPath调用(417 行)已经检查了下载的二进制文件是否存在。423-430 行的checkUvInstallation调用会再次执行相同的检查(因为checkUvInstallation内部首先调用getDownloadedUvPath),这导致了重复的文件系统 I/O 操作。建议移除 423-430 行的条件块,保持单一早返回路径。
应用此差异移除冗余检测:
- // 仅当请求的平台与当前进程一致时,才额外使用缓存的系统检测结果 - if (platform === process.platform && arch === process.arch) { - const installation = await this.checkUvInstallation() - if (installation.exists && installation.isDownloaded) { - logger.info('uv 已存在,跳过下载', { platform, arch, path: installation.path }) - return true - } - } - // 检查是否正在下载
432-436: ** 并发竞态窗口仍未修复**当前的"正在下载"检查(433 行)发生在控制器创建之前(465 行在
performDownload内创建)。两个并发的downloadUv调用可能同时通过此检查,导致重复下载并竞争写入同一文件。建议在
downloadUv中预先创建并注册AbortController,然后传入performDownload,以消除竞态窗口。可参考先前审查中提供的详细修改方案。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/main/services/MediaServerService.ts(3 hunks)src/main/services/UvBootstrapperService.ts(3 hunks)src/renderer/src/pages/player/components/TranscodeLoadingIndicator.tsx(0 hunks)src/renderer/src/pages/player/components/VolumeIndicator.tsx(2 hunks)src/renderer/src/pages/settings/FFmpegSection.tsx(4 hunks)src/renderer/src/pages/settings/FFprobeSection.tsx(5 hunks)src/renderer/src/services/CodecCompatibilityChecker.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/renderer/src/pages/player/components/TranscodeLoadingIndicator.tsx
🧰 Additional context used
📓 Path-based instructions (4)
src/renderer/src/**/*.{ts,tsx,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
优先使用 CSS 变量,避免硬编码样式值(颜色等)
Files:
src/renderer/src/pages/player/components/VolumeIndicator.tsxsrc/renderer/src/pages/settings/FFmpegSection.tsxsrc/renderer/src/pages/settings/FFprobeSection.tsxsrc/renderer/src/services/CodecCompatibilityChecker.ts
src/renderer/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
尺寸与时长等不要硬编码,优先使用 useTheme() 的 token 或集中样式变量(如 motionDurationMid、borderRadiusSM/MD)
Files:
src/renderer/src/pages/player/components/VolumeIndicator.tsxsrc/renderer/src/pages/settings/FFmpegSection.tsxsrc/renderer/src/pages/settings/FFprobeSection.tsxsrc/renderer/src/services/CodecCompatibilityChecker.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: 定制 antd 组件样式优先使用 styled-components 包装 styled(Component),避免全局 classNames
项目的图标统一使用 lucide-react,不使用 emoji 作为图标
组件/Hook 顶层必须通过 useStore(selector) 使用 Zustand,禁止在 useMemo/useEffect 内部调用 store Hook
避免使用返回对象的 Zustand 选择器(如 useStore(s => ({ a: s.a, b: s.b })));应使用单字段选择器或配合 shallow 比较器
遵循 React「副作用与状态更新」规范:渲染纯函数、Effect 三分法、幂等更新、稳定引用、严格清理、禁止写回自身依赖、Provider 值 memo、外部状态 selector 稳定等
统一使用 loggerService 记录日志而不是 console
logger 使用示例中第二个参数必须为对象字面量(如 logger.error('msg', { error }))
任何组件或页面都不要写入 media 元素的 currentTime,播放器控制由编排器统一负责
在 styled-components 中:主题相关属性使用 AntD CSS 变量(如 var(--ant-color-bg-elevated));
在 styled-components 中:设计系统常量(尺寸、动画、层级、字体、毛玻璃等)使用 JS 常量(如 SPACING、BORDER_RADIUS、Z_INDEX、FONT_SIZES 等)
Files:
src/renderer/src/pages/player/components/VolumeIndicator.tsxsrc/renderer/src/pages/settings/FFmpegSection.tsxsrc/renderer/src/pages/settings/FFprobeSection.tsxsrc/main/services/UvBootstrapperService.tssrc/main/services/MediaServerService.tssrc/renderer/src/services/CodecCompatibilityChecker.ts
**/player/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Player 页面:统一在组件顶层使用 Zustand selector,禁止在 useMemo/useEffect 内调用 store Hook;useSubtitleEngine 通过参数传入 subtitles 等防御处理
Files:
src/renderer/src/pages/player/components/VolumeIndicator.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-17T14:59:36.985Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T14:59:36.985Z
Learning: Applies to **/player/**/*.{ts,tsx} : Player 页面:统一在组件顶层使用 Zustand selector,禁止在 useMemo/useEffect 内调用 store Hook;useSubtitleEngine 通过参数传入 subtitles 等防御处理
Applied to files:
src/renderer/src/pages/player/components/VolumeIndicator.tsx
📚 Learning: 2025-09-17T14:59:36.985Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-17T14:59:36.985Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx} : 尺寸与时长等不要硬编码,优先使用 useTheme() 的 token 或集中样式变量(如 motionDurationMid、borderRadiusSM/MD)
Applied to files:
src/renderer/src/pages/settings/FFprobeSection.tsx
🧬 Code graph analysis (3)
src/renderer/src/pages/settings/FFmpegSection.tsx (1)
src/renderer/src/infrastructure/styles/theme.ts (2)
SPACING(43-58)FONT_SIZES(25-40)
src/renderer/src/pages/settings/FFprobeSection.tsx (1)
src/renderer/src/infrastructure/styles/theme.ts (2)
SPACING(43-58)FONT_SIZES(25-40)
src/main/services/MediaServerService.ts (1)
src/main/utils/index.ts (1)
getDataPath(11-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (windows-latest, 20)
- GitHub Check: test (ubuntu-latest, 20)
- GitHub Check: test (macos-latest, 20)
🔇 Additional comments (19)
src/renderer/src/services/CodecCompatibilityChecker.ts (4)
257-266: AI 摘要与代码不一致。AI 摘要声称
ExtendedErrorType移除了'hls-player-missing'类型,但在提供的代码差异中未看到任何移除操作的标记。当前代码显示的类型定义包含'hls-playback-error'而非'hls-player-missing'。
379-380: 代码改进合理。添加
.trim()处理是个好的改进:
- 确保去除编解码器名称前后的空白字符
- 空白字符串会被 trim 为空字符串(falsy 值),后续逻辑能正确处理
- 与后续的 truthy 检查(lines 390, 393)配合良好
390-390: 防御性编程改进得当。使用三元运算符显式检查
videoCodec和audioCodec是否为 truthy 值后再调用检查方法,避免了向checkVideoCodecSupport/checkAudioCodecSupport传递空字符串或 undefined,提升了代码的健壮性。Also applies to: 393-393
400-409: 逻辑更清晰且正确。新的不兼容原因构建逻辑改进显著:
- 明确区分"编解码器缺失"(missing)和"编解码器不支持"(unsupported)两种情况
- 使用
if-else if结构,避免同时添加 missing 和 unsupported 消息- 与 lines 390, 393 的 truthy 检查保持一致
这种改进让错误信息更加精确,便于用户和开发者理解问题根源。
src/main/services/UvBootstrapperService.ts (2)
478-481: LGTM - 变量声明提前合理将
platformDir、targetDir和tempDir的声明移至try块之前,使它们在作用域内对后续的ensureDir调用和finally块中的清理逻辑都可见,这是合理的重构。
580-580: LGTM - finally 块中集中清理临时目录将
cleanupTempDir移至finally块确保无论成功还是失败都会执行清理,这是良好的资源管理实践。src/main/services/MediaServerService.ts (5)
1-3: LGTM - 导入了必要的异步文件操作和加密模块从
node:crypto和node:fs/promises导入了实现缓存清理功能所需的模块,符合 Node.js 最佳实践。
60-72: LGTM - 清理结果类型定义清晰
TranscodeCacheCleanupResult接口结构合理,包含assetHash以及 HLS 和音频缓存各自的清理状态,便于调用方了解清理详情。
533-576: LGTM - 缓存清理逻辑实现正确方法实现了完整的缓存清理流程:
- 使用异步
stat检查文件存在性,并正确区分ENOENT与其他错误(539-547 行)- 计算资产哈希值(549 行)
- 定位 HLS 和音频缓存目录(552-553 行)
- 执行删除并返回详细结果(562-575 行)
已解决先前审查中提出的同步文件操作问题。
578-610: LGTM - 资产哈希计算实现稳健哈希计算逻辑考虑周全:
- 结合路径、大小和修改时间(583-585 行)
- 对大文件采样头尾部分,避免读取整个文件(592-603 行)
- 在
finally块中确保文件句柄关闭(605-607 行)实现高效且资源安全。
612-624: LGTM - 目录删除逻辑简洁有效使用
rm的force: true选项自然处理了"目录不存在"的情况,无需额外检查。错误处理和日志记录完善。src/renderer/src/pages/player/components/VolumeIndicator.tsx (2)
2-2: LGTM - 导入必要的 Hook添加
useRef用于跟踪初始挂载状态。
16-24: LGTM - 跳过初次渲染的音量指示器显示使用
useRef跟踪初始挂载状态,在首次渲染时跳过音量指示器的显示,避免组件挂载时不必要的视觉反馈。这是处理初始渲染副作用的常见模式。代码已遵循编码规范:Zustand 选择器在组件顶层调用(12-13 行),而非在
useEffect内部。基于编码规范
src/renderer/src/pages/settings/FFmpegSection.tsx (3)
76-76: LGTM!新增的
completionTimeoutRef用于追踪完成状态的延迟处理,实现清晰且符合 React Hook 最佳实践。
177-180: LGTM!在 effect 清理函数中正确清除超时并重置 ref,防止组件卸载后执行过期的处理器,遵循 React 最佳实践。
480-481: 优秀的重构!将硬编码的样式值替换为主题常量(
SPACING.XS、FONT_SIZES.SM),完全符合编码规范要求,提升了代码的可维护性和一致性。依据编码规范:
- "尺寸与时长等不要硬编码,优先使用 useTheme() 的 token 或集中样式变量"
- "在 styled-components 中:设计系统常量(尺寸、动画、层级、字体、毛玻璃等)使用 JS 常量"
src/renderer/src/pages/settings/FFprobeSection.tsx (3)
426-428: 样式已改用主题 token,符合规范使用 SPACING 与 FONT_SIZES 统一样式来源,赞。
553-553: 间距改为主题 token,LGTM与项目样式规范一致。
140-144: 清理时使用 window.clearTimeout 保持一致性函数式无误,建议与 setTimeout 一致采用 window 命名空间。
- if (successTimeoutRef.current) { - clearTimeout(successTimeoutRef.current) - successTimeoutRef.current = null - } + if (successTimeoutRef.current) { + window.clearTimeout(successTimeoutRef.current) + successTimeoutRef.current = null + }Likely an incorrect or invalid review comment.
| if (completionTimeoutRef.current) { | ||
| clearTimeout(completionTimeoutRef.current) | ||
| } | ||
| completionTimeoutRef.current = setTimeout(() => { | ||
| setIsDownloading(false) | ||
| setShowSuccessState(false) | ||
| setFFmpegStatus(currentStatus) | ||
| // 更新 FFmpeg 路径为下载后的路径 | ||
| setFFmpegPath(currentStatus.path) | ||
| // 自动开始预热 | ||
| handleWarmup() | ||
| completionTimeoutRef.current = null | ||
| }, 2000) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
完成状态超时处理逻辑正确。
实现细节:
- 在设置新超时前清除已存在的超时,防止重复执行
- 2 秒延迟用于展示成功状态,然后恢复正常状态并触发预热
- 执行后正确重置 ref 为 null
这个实现模式与 FFprobeSection 保持一致,逻辑安全可靠。
可选优化: 考虑将 2000ms 超时时长提取为命名常量,提升代码可维护性:
+// 文件顶部添加常量
+const SUCCESS_DISPLAY_DURATION = 2000 // 成功状态显示时长
// ...在超时设置处使用
- completionTimeoutRef.current = setTimeout(() => {
+ completionTimeoutRef.current = setTimeout(() => {
setIsDownloading(false)
setShowSuccessState(false)
// ...
- }, 2000)
+ }, SUCCESS_DISPLAY_DURATION)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/renderer/src/pages/settings/FFmpegSection.tsx around lines 153 to 165,
the 2000ms hard-coded timeout should be replaced with a named constant for
clarity and maintainability; define a descriptive constant (e.g.,
FFPEG_SUCCESS_DISPLAY_MS or FFmpegCompletionDelay) near the top of the file or
module, use that constant in setTimeout instead of 2000, and ensure any related
sections (like FFprobeSection) use the same constant or a similarly named one if
shared behavior is intended.
| const [showSuccessState, setShowSuccessState] = useState(false) | ||
| const isCancellingRef = useRef(false) | ||
| const isCompletionHandledRef = useRef(false) | ||
| const successTimeoutRef = useRef<number | null>(null) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
定时器句柄类型建议使用 ReturnType,更稳妥
当前使用 number 可以工作,但为兼容 TS lib 配置合并,推荐用 ReturnType,避免 Node/DOM 混用边界问题。
- const successTimeoutRef = useRef<number | null>(null)
+ const successTimeoutRef = useRef<ReturnType<typeof window.setTimeout> | null>(null)另外,建议将同文件中的进度轮询句柄也统一到 window 定义(在 effect 内),避免 NodeJS.Timeout 混入渲染进程类型:
// 建议在该 effect 内:
let progressInterval: ReturnType<typeof window.setInterval> | null = null
progressInterval = window.setInterval(async () => { /* ... */ }, 2000)
// 清理:
if (progressInterval) {
window.clearInterval(progressInterval)
}基于规范:尺寸/时长/环境依赖统一与稳定类型。As per coding guidelines
🤖 Prompt for AI Agents
In src/renderer/src/pages/settings/FFprobeSection.tsx around line 67, the
successTimeoutRef is typed as number | null which can mix Node/DOM timeout
types; change its type to ReturnType<typeof window.setTimeout> | null, ensure
you clear it with window.clearTimeout. Also update the progress polling interval
inside the effect to declare a local let progressInterval: ReturnType<typeof
window.setInterval> | null = null, assign via window.setInterval and clear with
window.clearInterval in the cleanup; adjust any related null checks and usages
accordingly.
| if (successTimeoutRef.current) { | ||
| clearTimeout(successTimeoutRef.current) | ||
| } | ||
| successTimeoutRef.current = window.setTimeout(() => { | ||
| setIsDownloading(false) | ||
| setShowSuccessState(false) | ||
| setFFprobeStatus(currentStatus) | ||
| // 更新 FFprobe 路径为下载后的路径 | ||
| setFFprobePath(currentStatus.path) | ||
| successTimeoutRef.current = null | ||
| }, 2000) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
✅ 修正到位;去除硬编码 2000ms,并统一使用 window.clearTimeout
- 已正确避免在回调中返回清理函数,方向正确。
- 建议将 2000ms 抽为集中常量或主题 token,便于统一管理与调参;同时使用 window.clearTimeout 与 window.setTimeout 保持一致性。
- if (successTimeoutRef.current) {
- clearTimeout(successTimeoutRef.current)
- }
- successTimeoutRef.current = window.setTimeout(() => {
+ if (successTimeoutRef.current) {
+ window.clearTimeout(successTimeoutRef.current)
+ }
+ successTimeoutRef.current = window.setTimeout(() => {
setIsDownloading(false)
setShowSuccessState(false)
setFFprobeStatus(currentStatus)
// 更新 FFprobe 路径为下载后的路径
setFFprobePath(currentStatus.path)
successTimeoutRef.current = null
- }, 2000)
+ }, SUCCESS_FEEDBACK_DELAY_MS)在文件顶部(或迁移到主题常量)新增:
// 临时集中常量(如主题已提供 FEEDBACK_DURATION.SUCCESS,请优先使用主题 token)
const SUCCESS_FEEDBACK_DELAY_MS = 2000根据编码规范(时长不硬编码,优先主题/集中变量)。As per coding guidelines
🤖 Prompt for AI Agents
In src/renderer/src/pages/settings/FFprobeSection.tsx around lines 121 to 131,
replace the hard-coded 2000ms delay and mixed timeout/clear calls by introducing
a centralized constant (add at the top of the file or use an existing theme
token like FEEDBACK_DURATION.SUCCESS) such as SUCCESS_FEEDBACK_DELAY_MS and use
that constant instead of 2000; also ensure you call window.clearTimeout when
clearing successTimeoutRef.current (keeping window.setTimeout for assignment) so
both set/clear use the same window namespace and the delay is no longer
hard-coded.
Summary by CodeRabbit