Skip to content

Comments

refactor: add enum type for ipc channel name#47

Merged
Tohrusky merged 5 commits intomainfrom
n-br
Aug 28, 2025
Merged

refactor: add enum type for ipc channel name#47
Tohrusky merged 5 commits intomainfrom
n-br

Conversation

@NangInShell
Copy link
Owner

No description provided.

@Tohrusky Tohrusky marked this pull request as ready for review August 27, 2025 09:53
@Tohrusky
Copy link
Collaborator

/gemini review

@Tohrusky Tohrusky requested a review from Copilot August 27, 2025 09:54
Copy link

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

This PR introduces IPC type definitions and constants to improve type safety in the Electron application. The main focus is replacing hardcoded string literals with centralized enum constants and adding magic string constants for template replacement.

  • Adds centralized IPC channel enums for type-safe communication between main and renderer processes
  • Introduces magic string constants for template replacement in VPY files and commands
  • Updates TypeScript configuration to support new module paths and type definitions

Reviewed Changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tsconfig.node.json Adds @main path mapping and includes preload type definitions
src/shared/constant/magicStr.ts Defines magic string constants for template replacement
src/shared/constant/ipc.ts Defines IPC channel enums for type-safe communication
src/renderer/src/utils/getVpy.ts Replaces hardcoded strings with MagicStr enum constants
src/renderer/src/utils/getTaskConfig.ts Removes unused comments
src/renderer/src/utils/getFFmpeg.ts Replaces hardcoded strings with MagicStr enum constants
src/renderer/src/components/*.vue Updates IPC calls to use typed channel constants and adds explicit return types
src/main/*.ts Updates IPC handlers to use typed channel constants and improves process management
package.json Updates dependencies to newer versions
eslint.config.js Adds TypeScript-specific linting rules
electron.vite.config.ts Adds @main and @shared path aliases
README.md Minor title update
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +18 to +39
function handleFileChange(event): void {
const files = event.target.files
if (files.length > 0) {
handleFiles(files)
}
}

function handleDrop(event) {
function handleDrop(event): void {
event.preventDefault()
const files = event.dataTransfer.files
if (files.length > 0) {
handleFiles(files)
}
}

function handleFiles(files) {
function handleFiles(files): void {
for (let i = 0; i < files.length; i++) {
fileList.value.push(files[i])
}
}

function removeFile(index) {
function removeFile(index): void {
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The event parameter should have a proper type annotation. Consider using 'Event' or a more specific event type like 'InputEvent' instead of implicit 'any'.

Copilot uses AI. Check for mistakes.
}

function handleDrop(event) {
function handleDrop(event): void {
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The event parameter should have a proper type annotation. Consider using 'DragEvent' instead of implicit 'any'.

Suggested change
function handleDrop(event): void {
function handleDrop(event: DragEvent): void {

Copilot uses AI. Check for mistakes.
}

function handleFiles(files) {
function handleFiles(files): void {
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The files parameter should have a proper type annotation. Consider using 'FileList' instead of implicit 'any'.

Suggested change
function handleFiles(files): void {
function handleFiles(files: FileList): void {

Copilot uses AI. Check for mistakes.
}

function removeFile(index) {
function removeFile(index): void {
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The index parameter should have a proper type annotation. Consider using 'number' instead of implicit 'any'.

Suggested change
function removeFile(index): void {
function removeFile(index: number): void {

Copilot uses AI. Check for mistakes.
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

本次 PR 主要通过引入枚举(enums)来替代硬编码的 IPC 通道名称和魔法字符串,极大地提升了代码的可读性和可维护性。这是一个非常好的实践。此外,还更新了一些依赖项,并对代码进行了一些小的重构和类型注解。

我的审查意见主要集中在以下几个方面,旨在进一步提高代码质量:

  1. 错误处理:建议在 catch 块中添加日志,避免静默地忽略潜在错误。
  2. 代码整洁:建议移除用于调试的 console.log 语句。
  3. 类型安全:建议为函数参数添加更明确的类型,以增强代码的健壮性和开发体验。

总体来说,这是一次高质量的重构,使项目代码更加规范和易于维护。

Comment on lines +28 to +36
try {
vspipe.proc.stdout.unpipe(ffmpeg.proc.stdin)
}
catch {}
try {
ffmpeg.proc.stdin.end()
}
catch {}
}

Choose a reason for hiding this comment

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

medium

safeUnpipe 函数中,空的 catch 块会抑制潜在的错误,这可能导致调试困难。即使您预期在某些情况下会发生错误(例如,当流已经关闭时),也最好记录这些错误以供将来参考。

建议添加日志记录,以便在出现意外问题时能够追踪。

    try {
      vspipe.proc.stdout.unpipe(ffmpeg.proc.stdin)
    }
    catch (error) {
      console.warn('Failed to unpipe stream:', (error as Error).message)
    }
    try {
      ffmpeg.proc.stdin.end()
    }
    catch (error) {
      console.warn('Failed to end stdin:', (error as Error).message)
    }
  }

Comment on lines +170 to +171
console.log(`ffmpegPID:${renderProcess.pid}`)
console.log(`vspipePID:${vspipeProc.pid}`)

Choose a reason for hiding this comment

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

medium

这些 console.log 语句看起来是用于调试的。在合并到主分支之前,建议将它们移除或替换为更结构化的日志记录工具(例如,使用带有不同日志级别的记录器),以保持生产环境日志的整洁。

Comment on lines +18 to 23
function handleFileChange(event): void {
const files = event.target.files
if (files.length > 0) {
handleFiles(files)
}
}

Choose a reason for hiding this comment

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

medium

为了提高代码的可读性和可维护性,建议为 event 参数添加明确的类型。虽然 no-explicit-any 规则已关闭,但显式类型可以帮助其他开发者(以及未来的您)更快地理解代码。

function handleFileChange(event: Event): void {
  const input = event.target as HTMLInputElement
  const files = input.files
  if (files && files.length > 0) {
    handleFiles(files)
  }
}

Comment on lines +25 to 31
function handleDrop(event): void {
event.preventDefault()
const files = event.dataTransfer.files
if (files.length > 0) {
handleFiles(files)
}
}

Choose a reason for hiding this comment

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

medium

为了提高代码的可读性和可维护性,建议为 event 参数添加明确的类型。使用 DragEvent 类型可以提供更好的类型安全和自动补全支持。

function handleDrop(event: DragEvent): void {
  event.preventDefault()
  const files = event.dataTransfer?.files
  if (files && files.length > 0) {
    handleFiles(files)
  }
}

Comment on lines +33 to 37
function handleFiles(files): void {
for (let i = 0; i < files.length; i++) {
fileList.value.push(files[i])
}
}

Choose a reason for hiding this comment

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

medium

为了提高代码的可读性和可维护性,建议为 files 参数添加明确的 FileList 类型。

function handleFiles(files: FileList): void {
  for (let i = 0; i < files.length; i++) {
    fileList.value.push(files[i])
  }
}

Comment on lines +39 to 41
function removeFile(index): void {
fileList.value.splice(index, 1) // 根据索引删除文件
}

Choose a reason for hiding this comment

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

medium

为了提高代码的可读性和可维护性,建议为 index 参数添加明确的 number 类型。

function removeFile(index: number): void {
  fileList.value.splice(index, 1) // 根据索引删除文件
}

Comment on lines +66 to 71
function handlePreviewInfo(_event: any, data: any): void {
if (data && data.frames) {
frameCount.value = Number.parseInt(data.frames)
message.success(`检测到 ${data.frames} 帧,尺寸 ${data.width}×${data.height}`)
}
}

Choose a reason for hiding this comment

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

medium

为了提高代码的可读性和可维护性,建议为事件处理函数的参数添加明确的类型。_event 可以是 Electron.IpcRendererEventdata 可以定义一个接口来描述其结构。

function handlePreviewInfo(_event: Electron.IpcRendererEvent, data: { frames: string; width: string; height: string; fps: string }): void {
  if (data && data.frames) {
    frameCount.value = Number.parseInt(data.frames)
    message.success(`检测到 ${data.frames} 帧,尺寸 ${data.width}×${data.height}`)
  }
}

@Tohrusky Tohrusky changed the title IPC类型定义 refactor: 3 different IPC channels Aug 27, 2025
@Tohrusky Tohrusky changed the title refactor: 3 different IPC channels refactor: add enum type for ipc channel name Aug 27, 2025
@Tohrusky Tohrusky merged commit 5c48c2b into NangInShell:main Aug 28, 2025
2 checks passed
@Tohrusky Tohrusky mentioned this pull request Aug 29, 2025
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