refactor: run file watcher in child process#4202
Conversation
Aaaaash
changed the title
|
是否需要考虑增加 Watcher 心跳 和 kill + 重启逻辑？ |
要的，我加一下 |
|
/next |
|
🎉 PR Next publish successful! 3.6.1-next-1733810441.0 |
|
/next |
|
🎉 PR Next publish successful! 3.6.1-next-1733818657.0 |
|
/next |
|
🎉 PR Next publish successful! 3.6.1-next-1733880873.0 |
Aaaaash
changed the base branch from
refactor/support-watch-by-unrecursive-config
to
main
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options. Walkthrough该拉取请求主要涉及对多个文件的修改，重点是增强构建过程和文件监视功能。 Changes
Possibly related PRs
Suggested labels
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Outside diff range and nitpick comments (18)
packages/core-common/src/types/reporter.ts (1)
109-127: 实现看起来不错，建议添加错误处理代码实现简洁明了，符合接口规范。不过建议考虑以下几点改进：
- 在构造函数中验证 emitter 参数不为空
- 在 fire 事件时添加错误处理
建议按如下方式改进代码：
export class CommonProcessReporter implements IReporter { - constructor(private emitter: Emitter<ReporterProcessMessage>) {} + constructor(private emitter: Emitter<ReporterProcessMessage>) { + if (!emitter) { + throw new Error('Emitter is required'); + } + } performance(name: string, data: PerformanceData): void { + try { this.emitter.fire({ reportType: REPORT_TYPE.PERFORMANCE, name, data, }); + } catch (error) { + console.error('Failed to emit performance report:', error); + } } point(name: string, data: PointData): void { + try { this.emitter.fire({ reportType: REPORT_TYPE.POINT, name, data, }); + } catch (error) { + console.error('Failed to emit point report:', error); + } } }packages/extension/src/hosted/ext.process-base.ts (1)
Line range hint
115-127: 建议改进进程消息的错误处理当前的进程消息处理逻辑可能在进程不存在或发送失败时出现问题。
建议添加错误处理:
reporterEmitter.event((reportMessage: ReporterProcessMessage) => { - if (process && process.send) { + if (!process?.send) { + logger.warn('Process.send is not available for reporting'); + return; + } + try { process.send({ type: ProcessMessageType.REPORTER, data: reportMessage, }); + } catch (error) { + logger.error('Failed to send reporter message:', error); + } - } });packages/file-service/src/node/hosted/watch-process-log.ts (2)
22-44: 建议为方法参数添加类型注解当前多个方法（如
error、warn、log、debug等）的参数缺少类型注解。为了提高代码的可读性和类型安全性，建议为这些方法的参数添加明确的类型注解，例如...args: any[]。
50-52: 为setOptions方法的参数添加类型注解
setOptions(options)方法的参数options缺少类型注解。建议为options参数指定明确的类型，以增强代码的类型安全性和可读性。packages/file-service/src/node/hosted/watcher.process.ts (2)
50-50: 建议使用可选链（Optional Chaining）简化代码在第 50 行和第 55 行，可以使用可选链运算符
?.来简化对logger的检查，提升代码的可读性。修改如下：
- return (logger && logger.error.bind(logger)) || console.error.bind(console); + return (logger?.error.bind(logger)) || console.error.bind(console);- return (logger && logger.warn.bind(logger)) || console.warn.bind(console); + return (logger?.warn.bind(logger)) || console.warn.bind(console);Also applies to: 55-55
🧰 Tools
🪛 Biome (1.9.4)
[error] 50-50: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
93-93: 拼写错误：应将 'Watcehr-Host' 修改为 'Watcher-Host'第 93 行的日志信息中，'Watcehr-Host' 可能是拼写错误，建议更正为 'Watcher-Host'。
packages/file-service/src/node/watcher-process-manager.ts (1)
81-81: 简化异步函数中的返回值在异步函数中，可以直接返回 Promise，而无需使用
await。建议如下修改以简化返回语句：
- return await normalizedIpcHandlerPathAsync(name, true, this.appConfig.extHostIPCSockPath); + return normalizedIpcHandlerPathAsync(name, true, this.appConfig.extHostIPCSockPath);packages/core-common/src/preferences/file-watch.ts (1)
8-16: 建议优化 flattenExcludes 函数实现当前实现虽然正确，但可以使用更简洁的函数式方法重构。
建议使用以下实现：
export function flattenExcludes(fileExcludes: { [key: string]: boolean }) { - const excludes: string[] = []; - for (const key of Object.keys(fileExcludes)) { - if (fileExcludes[key]) { - excludes.push(key); - } - } - return excludes; + return Object.entries(fileExcludes) + .filter(([_, value]) => value) + .map(([key]) => key); }packages/core-common/src/types/file-watch.ts (1)
21-25: 建议完善方法文档当前的文档注释比较简单，建议补充以下信息：
- 参数 excludes 的格式要求和示例
- 更新失败时的错误处理说明
- 方法调用的时机建议
建议的文档格式：
/** * Update watcher file excludes + * @description 动态更新文件监视器的排除规则 * @param excludes 排除规则数组，支持 glob 模式，如 ["**/*.git", "node_modules/**"] + * @throws 当更新失败时可能抛出的错误 + * @example + * await updateWatcherFileExcludes(["**/*.tmp", "dist/**"]); */packages/core-node/src/types.ts (1)
93-96: 建议完善 watcherHost 配置项的文档说明当前的注释仅说明了这是"Watcher Node 进程入口文件"，建议补充以下信息：
- 配置示例
- 默认值说明
- 环境变量覆盖说明
建议更新注释为：
/** - * Watcher Node 进程入口文件 + * Watcher Node 进程入口文件 + * @default packages/file-service/lib/node/hosted/watcher.process.js + * @example + * watcherHost: '/custom/path/to/watcher.js' + * @env WATCHER_HOST_ENTRY - 可通过环境变量覆盖默认配置 */packages/file-service/src/browser/file-service-provider-client.ts (1)
109-117: 建议改进错误处理机制当初始化过程发生错误时，建议：
- 在日志中包含更多错误上下文信息
- 考虑是否需要将错误传播给调用者
async initialize(clientId: string) { if (this.fileServiceProvider?.initialize) { try { await this.fileServiceProvider?.initialize(clientId); } catch (err) { - getDebugLogger('fileService.fsProvider').error('initialize error', err); + const logger = getDebugLogger('fileService.fsProvider'); + logger.error(`初始化文件服务提供者失败 [clientId=${clientId}]`, err); + throw new Error(`文件服务初始化失败: ${err.message}`); } } }packages/core-browser/src/react-providers/config-provider.tsx (1)
315-319: 建议：添加更详细的配置说明新增的
unRecursiveDirectories配置项需要更详细的文档说明，包括：
- 配置项的具体用途
- 示例值
- 与其他相关配置的关系
建议添加如下注释：
/** - * Unrecursive directories + * 指定不需要递归监视的目录列表 + * 例如：['/node_modules', '/dist'] + * 注意：此配置与 files.watcherExclude 配合使用 */ unRecursiveDirectories?: string[];packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (2)
219-219: 优化 ignore 配置的合并逻辑当前的数组合并方式可能导致重复项。
建议使用 Set 去重：
-ignore: this.excludes.concat(rawOptions?.excludes ?? []), +ignore: [...new Set([...this.excludes, ...(rawOptions?.excludes ?? [])])],
Line range hint
223-228: 建议增强错误处理和重试机制当前的错误处理和重试机制可以更加健壮。
建议增加以下改进：
catch (e) { - this.logger.error('watcher subscribe failed ', e, ' try times ', times); + this.logger.error( + `Watcher subscribe failed (attempt ${times + 1}/${maxRetries}):`, + { + error: e, + path: realPath, + backend: FileSystemWatcherServer.PARCEL_WATCHER_BACKEND + } + ); + if (times === maxRetries - 1) { + this.logger.error('All retry attempts exhausted, watcher will not be started'); + } await new Promise((resolve) => { setTimeout(resolve, retryDelay); }); }packages/file-service/src/node/file-service.ts (1)
98-106: 建议优化条件判断的代码结构当前的条件判断可以使用可选链操作符来简化:
- recursive: options && options.recursive !== undefined ? options.recursive : this.options.recursive, + recursive: options?.recursive ?? this.options.recursive,这样可以:
- 提高代码可读性
- 减少重复的条件判断
- 使用现代 JavaScript 特性
🧰 Tools
🪛 Biome (1.9.4)
[error] 105-105: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/file-service/__tests__/node/file-node-watcher.test.ts (1)
11-11: 建议改进测试可靠性固定的休眠时间可能导致测试不稳定。建议实现更可靠的等待机制。
建议采用以下方案之一：
- 实现重试机制：
-const sleepTime = 1000; +const MAX_RETRIES = 5; +const RETRY_INTERVAL = 500; + +async function waitForFileChange(condition) { + for (let i = 0; i < MAX_RETRIES; i++) { + if (condition()) return true; + await sleep(RETRY_INTERVAL); + } + return false; +}
- 使用文件系统事件回调而不是固定延时：
+async function waitForFSEvent(emitter) { + return new Promise(resolve => { + emitter.once('change', resolve); + }); +}packages/file-service/__tests__/node/file-service-watcher.test.ts (2)
Line range hint
13-13: 建议添加跳过 macOS 测试的原因说明为了提高代码可维护性，建议添加注释说明在 macOS 上跳过测试的具体原因。
+(isMacintosh ? describe.skip /* 在 macOS 上跳过原因：<在此说明原因> */ : describe)('ParceWatcher Test', () => { -isMacintosh ? describe.skip : describe)('ParceWatcher Test', () => {
Line range hint
22-22: 建议改进类型处理方式使用
@ts-ignore可能掩盖潜在的类型问题。建议通过适当的类型定义来解决这个问题。- // @ts-ignore - injector.mock(FileSystemWatcherServer, 'isEnableNSFW', () => false); + type MockableFileSystemWatcherServer = { + isEnableNSFW: () => boolean; + }; + injector.mock<MockableFileSystemWatcherServer>(FileSystemWatcherServer, 'isEnableNSFW', () => false);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (34)
package.json(1 hunks)packages/core-browser/src/core-preferences.ts(2 hunks)packages/core-browser/src/react-providers/config-provider.tsx(1 hunks)packages/core-common/src/log.ts(4 hunks)packages/core-common/src/preferences/file-watch.ts(1 hunks)packages/core-common/src/types/file-watch.ts(1 hunks)packages/core-common/src/types/file.ts(1 hunks)packages/core-common/src/types/reporter.ts(2 hunks)packages/core-node/src/types.ts(1 hunks)packages/extension/src/hosted/ext.process-base.ts(2 hunks)packages/extension/src/hosted/extension-reporter.ts(0 hunks)packages/file-service/__tests__/browser/file-service-client.test.ts(1 hunks)packages/file-service/__tests__/node/file-node-watcher.test.ts(1 hunks)packages/file-service/__tests__/node/file-service-watcher.test.ts(1 hunks)packages/file-service/__tests__/node/index.test.ts(1 hunks)packages/file-service/package.json(1 hunks)packages/file-service/src/browser/file-service-client.ts(5 hunks)packages/file-service/src/browser/file-service-contribution.ts(1 hunks)packages/file-service/src/browser/file-service-provider-client.ts(1 hunks)packages/file-service/src/common/file-service-client.ts(1 hunks)packages/file-service/src/common/files.ts(1 hunks)packages/file-service/src/common/watcher.ts(2 hunks)packages/file-service/src/node/disk-file-system.provider.ts(7 hunks)packages/file-service/src/node/file-service.ts(2 hunks)packages/file-service/src/node/hosted/recursive/file-service-watcher.ts(6 hunks)packages/file-service/src/node/hosted/shared/file-type.ts(1 hunks)packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts(3 hunks)packages/file-service/src/node/hosted/watch-process-log.ts(1 hunks)packages/file-service/src/node/hosted/watcher.host.service.ts(1 hunks)packages/file-service/src/node/hosted/watcher.process.ts(1 hunks)packages/file-service/src/node/index.ts(2 hunks)packages/file-service/src/node/watcher-process-manager.ts(1 hunks)packages/workspace/src/browser/workspace-service.ts(2 hunks)tools/dev-tool/src/server.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/extension/src/hosted/extension-reporter.ts
✅ Files skipped from review due to trivial changes (2)
- packages/file-service/tests/browser/file-service-client.test.ts
- packages/file-service/tests/node/index.test.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/file-service/src/node/file-service.ts
[error] 105-105: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/file-service/src/node/hosted/watcher.process.ts
[error] 50-50: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 55-55: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (21)
packages/extension/src/hosted/ext.process-base.ts (1)
8-8: 替换报告器实现的改动看起来没问题
新的 CommonProcessReporter 替换原有的 ExtensionReporter 是个合理的改动。
Also applies to: 96-96
packages/file-service/src/common/watcher.ts (1)
58-70: 新添加的常量和接口定义清晰
新添加的常量 KT_WATCHER_PROCESS_SOCK_KEY、WATCHER_INIT_DATA_KEY 以及接口 IWatcherHostService、IWatcherProcessManager 定义清晰，命名规范，符合编码标准。
packages/file-service/src/browser/file-service-client.ts (1)
357-359: 验证非递归目录的匹配逻辑
在 watchFileChanges 方法中，isUnRecursive 的计算可能会受到路径格式的影响，建议确认 unRecursiveDirectories 中的路径格式与 uri.path 一致，以确保非递归监视目录正确匹配。
请检查 unRecursiveDirectories 和 uri.path 的格式，确保路径匹配逻辑正确无误。
packages/core-common/src/preferences/file-watch.ts (1)
1-6: 文件排除模式配置合理且完整
默认的文件排除模式涵盖了常见的大型目录（如 git 对象、node_modules），这有助于提高文件监视性能并减少不必要的系统资源消耗。
packages/file-service/package.json (1)
13-14: 构建脚本可能存在冗余
build 和 build:watcher-host 脚本使用相同的 TypeScript 配置文件，这可能导致重复编译。建议：
- 明确说明两个构建脚本的区别
- 考虑使用不同的 TypeScript 配置文件，以便针对 watcher-host 进行优化
运行以下脚本检查 TypeScript 配置文件的内容：
✅ Verification successful
构建脚本的区别在于编译选项
经过验证，两个构建脚本的主要区别在于：
build使用--build选项，这会启用项目引用的增量编译build:watcher-host使用-p选项，这是简单的项目编译模式
这两个脚本的设计是合理的，因为：
build用于完整构建，支持增量编译和项目引用build:watcher-host用于观察者模式下的快速编译
不需要使用不同的配置文件，因为它们的区别在于编译模式而不是配置内容。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 TypeScript 配置文件的具体设置
cat ../../configs/ts/references/tsconfig.file-service.json
Length of output: 147
Script:
#!/bin/bash
# 查找所有 tsconfig 文件
fd tsconfig
# 查看 package.json 中的构建脚本
cat packages/file-service/package.json
Length of output: 4292
Script:
#!/bin/bash
# 检查 tsconfig.file-service.json 的内容
cat configs/ts/references/tsconfig.file-service.json
# 检查基础配置文件
cat configs/ts/tsconfig.base.json
Length of output: 1252
packages/file-service/src/node/index.ts (1)
17-17: 导入和提供者配置正确实现
新增的 WatcherProcessManager 相关导入和提供者配置符合依赖注入模式，为文件监视器的子进程实现提供了必要的基础设施。
Also applies to: 36-36
packages/file-service/src/node/hosted/shared/file-type.ts (1)
13-13: 验证导入路径更改的正确性
导入路径的更改可能是由于文件结构重组。需要确保新路径 '../../../common' 正确指向目标模块。
✅ Verification successful
Let me gather more specific information about the imports and file structure.
导入路径需要调整
根据代码分析，导入的符号 EditorFileType、getFileTypeByExt 和 isErrnoException 都定义在 packages/file-service/src/common 目录下。而当前文件 file-type.ts 位于 packages/file-service/src/node/hosted/shared/ 目录中，因此从该位置访问 common 目录需要使用 ../../../common 作为相对路径。
导入路径的更改是正确的，与文件的实际位置相符。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 验证新的导入路径是否有效
fd -t f "common.ts" -x echo "Found common module at: {}"
Length of output: 962
Script:
#!/bin/bash
# 查找文件服务相关的 common 模块
fd -t f "common.ts" packages/file-service
# 检查导入的具体符号在哪些文件中定义
rg "export.*EditorFileType|export.*getFileTypeByExt|export.*isErrnoException" -A 2
# 查看文件的实际位置以验证相对路径
fd "file-type.ts" packages/file-service
Length of output: 1025
packages/core-common/src/types/file-watch.ts (1)
21-25: 接口扩展设计合理
新增的 updateWatcherFileExcludes 方法使文件监视器的排除规则可以动态更新，这是一个很好的改进。将其设置为可选方法也确保了向后兼容性。
tools/dev-tool/src/server.ts (1)
91-92: 验证 watcherHost 路径配置的安全性和可用性
建议在使用环境变量指定的路径之前进行验证，以确保:
- 文件确实存在
- 路径不包含潜在的安全风险
✅ Verification successful
Let me gather more information about the file service package and watcher implementation.
Let me check one final thing about the watcher process implementation.
watcherHost 配置验证结果
根据代码分析，该配置是安全的：
- 默认路径指向
packages/file-service/src/node/hosted/watcher.process.ts文件，该文件确实存在 - 文件监视器实现完整，包含了必要的安全检查和错误处理
- 通过环境变量
WATCHER_HOST_ENTRY提供的灵活性是合理的，因为：- 文件监视器服务有完整的进程管理机制
- 使用了安全的路径拼接方法
- 有适当的错误处理机制
建议：
- 在生产环境中谨慎使用
WATCHER_HOST_ENTRY环境变量，最好保持使用默认配置
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 验证默认的 watcher.process.js 文件是否存在
fd -t f "watcher.process.js$"
# 检查是否有其他地方引用了这个文件
rg "watcher\.process\.js"
Length of output: 219
Script:
#!/bin/bash
# Check if the file-service package exists and its structure
fd -t d "file-service$"
fd -t f -p "packages/file-service" "package.json"
# Look for watcher related implementations
rg -g "!*.d.ts" "class.*Watcher" -A 5
rg -g "!*.d.ts" "createWatcher" -A 5
Length of output: 7162
Script:
#!/bin/bash
# Check the watcher process implementation
fd -t f "watcher.process.js$" -x ls -l {}
fd -t f "watcher.process.ts$"
# Check if there are any direct imports of the watcher process
rg "require.*watcher\.process"
rg "import.*watcher\.process"
Length of output: 199
package.json (1)
19-19: 构建脚本更新正确
新增的 watcher-host 构建脚本及其在 build:all 中的集成位置合理。
Also applies to: 23-23
packages/core-common/src/log.ts (3)
25-26: 日志命名空间扩展合理
为 Watcher 进程添加专门的日志命名空间有助于区分和追踪问题。
174-174: 类型定义清晰
IWatcherProcessLogger 类型别名的定义简洁明确，继承自 ICoreLogger 接口是合适的选择。
299-299: 格式统一性建议
destroy 方法的空格添加保持了代码格式的一致性。
Also applies to: 341-341
packages/core-common/src/types/file.ts (1)
160-160:
注意：这是一个破坏性更改
接口 FileSystemProvider 的 watch 方法签名已更新，增加了可选的 recursive 参数。这个更改可能需要所有实现此接口的类进行相应更新。
执行以下脚本来验证所有实现类是否都已更新：
✅ Verification successful
Let me analyze the implementation and generate additional verification scripts to check the interface usage.
watch 方法签名的变更是合理的
经过验证，这个更改实际上是使接口定义与其他实现保持一致。在 VSCode 和 SUMI Worker 的类型定义中，watch 方法的参数已经包含了 recursive 选项，这次更改是将核心接口与现有实现对齐。
packages/types/vscode/typings/vscode.d.ts中的定义已包含recursive选项packages/types/sumi-worker/sumi-worker.d.ts中的定义也包含相同选项- 唯一的实现类
RemoteFileSystemProvider已经正确处理了这个选项
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：查找所有实现 FileSystemProvider 接口的类
# 搜索实现了 watch 方法的类
ast-grep --pattern 'class $_ implements FileSystemProvider {
$$$
watch($_, $_) {
$$$
}
$$$
}'
Length of output: 11526
Script:
#!/bin/bash
# Search for the interface definition and its usages
rg -A 5 "interface FileSystemProvider"
Length of output: 2081
Script:
#!/bin/bash
# 检查 watch 方法的具体定义
rg -A 3 "watch\(.*\).*{?" packages/types/vscode/typings/vscode.d.ts packages/types/sumi-worker/sumi-worker.d.ts packages/extension/src/common/vscode/file-system.ts packages/core-common/src/types/file.ts
Length of output: 316
Script:
#!/bin/bash
# 检查 watch 方法的具体定义
rg -A 3 "watch\(" packages/types/vscode/typings/vscode.d.ts packages/types/sumi-worker/sumi-worker.d.ts packages/extension/src/common/vscode/file-system.ts packages/core-common/src/types/file.ts
Length of output: 1942
packages/core-browser/src/core-preferences.ts (1)
11-11: 优化：文件监视排除模式已集中管理
将 filesWatcherExclude 的默认值移至 defaultFilesWatcherExcludes 常量是一个很好的重构：
- 提高了代码的可维护性
- 确保了排除模式的一致性
- 便于统一更新和管理
Also applies to: 21-21
packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (1)
Line range hint 65-71: 构造函数注入方式的改进
构造函数的改动提升了依赖注入的清晰度，直接注入 ILogService 而不是通过 LogServiceManager 是更好的实践。
packages/file-service/src/common/files.ts (1)
423-423: 新增的初始化方法增强了接口的扩展性
initialize 方法的添加使得 IDiskFileProvider 接口更加灵活,允许基于客户端 ID 进行初始化配置。这是一个很好的扩展点。
packages/file-service/src/node/file-service.ts (1)
48-48: 导入路径变更反映了代码结构优化
从 ./shared/file-type 改为 ./hosted/shared/file-type 的变更表明进行了代码组织结构的优化。
packages/workspace/src/browser/workspace-service.ts (2)
29-29: 引入工具函数优化了代码结构
从 @opensumi/ide-core-common/lib/preferences/file-watch 引入 flattenExcludes 函数有助于代码复用和维护。
180-184: 代码重构提升了可维护性
使用 flattenExcludes 工具函数替代原有的手动数组构建逻辑:
- 减少了重复代码
- 提高了代码可维护性
- 统一了排除规则的处理方式
packages/file-service/__tests__/node/file-service-watcher.test.ts (1)
9-9: 导入路径更改符合预期
此更改与文件监视器重构到托管环境的架构变更保持一致。
|
/next |
|
🎉 PR Next publish successful! 3.6.2-next-1733889718.0 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
packages/file-service/src/node/disk-file-system.provider.ts (1)
91-106: 建议优化文件变更事件的处理逻辑当前的文件变更事件处理逻辑可以进行以下优化：
- 建议将事件过滤逻辑抽取为单独的方法
- 考虑添加防抖处理，避免短时间内触发过多事件
- 添加错误处理机制
建议重构为:
+ private filterIgnoredChanges(changes: FileChangeEvent) { + return changes.filter((c) => !this.ignoreNextChangesEvent.has(c.uri)); + } + private handleFileChanges(changes: FileChangeEvent) { + try { + const filteredChanges = this.filterIgnoredChanges(changes); + if (filteredChanges.length > 0) { + this.fileChangeEmitter.fire(filteredChanges); + if (Array.isArray(this.rpcClient)) { + this.rpcClient.forEach((client) => { + client.onDidFilesChanged({ changes: filteredChanges }); + }); + } + } + } catch (error) { + this.logger.error('Error handling file changes:', error); + } + } constructor(@Optional() recursive = true) { // ... this.watcherProcessManager.setClient({ onDidFilesChanged: debounce((events: DidFilesChangedParams) => { - if (events.changes.length > 0) { - const changes = events.changes.filter((c) => !this.ignoreNextChangesEvent.has(c.uri)); - this.fileChangeEmitter.fire(changes); - if (Array.isArray(this.rpcClient)) { - this.rpcClient.forEach((client) => { - client.onDidFilesChanged({ - changes, - }); - }); - } - } + this.handleFileChanges(events.changes); }, 300) }); }packages/file-service/src/node/hosted/watcher.host.service.ts (1)
37-70: 建议添加重试机制
initWatcherServer方法在初始化失败时缺少重试机制，可能影响服务可靠性。建议添加重试逻辑:
initWatcherServer(excludes?: string[], force = false) { + const maxRetries = 3; + const retryDelay = 1000; + let retryCount = 0; + + const initWithRetry = async () => { try { this.recursiveFileSystemWatcher = new FileSystemWatcherServer(excludes, this.logger); this.unrecursiveFileSystemWatcher = new UnRecursiveFileSystemWatcher(this.logger); + return true; } catch (error) { + this.logger.error('Failed to init watcher server:', error); + if (retryCount < maxRetries) { + retryCount++; + await new Promise(resolve => setTimeout(resolve, retryDelay)); + return initWithRetry(); + } + return false; } + };packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (3)
Line range hint
229-233: 客户端设置逻辑需要优化
setClient方法的条件判断逻辑可能导致意外行为。当this.client存在且this.toDispose.disposed为 true 时直接返回，这可能导致新客户端设置失败。建议修改为:
setClient(client: FileSystemWatcherClient | undefined) { - if (this.client && this.toDispose.disposed) { + if (this.toDispose.disposed) { + this.logger.warn('Cannot set client: watcher is disposed'); return; } this.client = client; }
33-33: 建议添加服务状态追踪构造函数可以添加状态追踪，以便更好地监控服务健康状况。
建议添加以下属性和初始化:
+ private status: 'initializing' | 'running' | 'disposed' = 'initializing'; constructor(private readonly logger: ILogService) { + this.status = 'running'; + this.logger.log('UnRecursiveFileSystemWatcher initialized'); }
Line range hint
86-126: 建议优化文件变更处理逻辑文件变更处理中的延时处理和状态检查逻辑可以进一步优化，以提高可靠性。
建议考虑以下几点优化：
- 使用更可靠的文件状态检测机制
- 实现变更事件的批处理
- 添加错误重试机制
- 考虑使用 Node.js 的
worker_threads处理耗时操作示例实现:
protected async handleFileChange(changePath: string, type: string): Promise<void> { const batchSize = 100; const changes: Array<{path: string, type: string}> = []; return new Promise((resolve, reject) => { // 实现批处理逻辑 }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
packages/file-service/src/node/disk-file-system.provider.ts(7 hunks)packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts(3 hunks)packages/file-service/src/node/hosted/watcher.host.service.ts(1 hunks)packages/file-service/src/node/watcher-process-manager.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/file-service/src/node/watcher-process-manager.ts
🔇 Additional comments (2)
packages/file-service/src/node/disk-file-system.provider.ts (2)
77-79: 使用依赖注入引入 watcherProcessManager
通过 @Autowired 装饰器注入 IWatcherProcessManager 实例，这是一个好的做法，符合依赖注入原则。
136-138:
dispose 方法需要等待异步操作完成
dispose 方法中的 watcherProcessManager.dispose() 可能是异步操作，需要使用 await 等待其完成。
- async dispose() {
- this.watcherServerDisposeCollection?.dispose();
- this.watcherProcessManager.dispose();
- }
+ async dispose() {
+ try {
+ this.watcherServerDisposeCollection?.dispose();
+ await this.watcherProcessManager.dispose();
+ } catch (error) {
+ this.logger.error('Error disposing file system provider:', error);
+ throw error;
+ }
+ }Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/extension/__tests__/node/extension.service.client.test.ts(2 hunks)packages/file-service/__tests__/node/file-node-watcher.test.ts(3 hunks)packages/file-service/__tests__/node/index.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/file-service/tests/node/file-node-watcher.test.ts
- packages/file-service/tests/node/index.test.ts
Aaaaash
force-pushed
the
refactor/run-file-watcher-in-child-process
branch
from
84410c0 to
e2bf659
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (2)
229-232: 建议增强 setClient 方法的错误处理当前实现可以进一步改进：
- 建议添加日志记录，以便追踪客户端状态变化
- 考虑在客户端已释放时抛出适当的错误
- 添加客户端状态的防御性检查
setClient(client: FileSystemWatcherClient | undefined) { if (this.client && this.toDispose.disposed) { + this.logger.warn('Attempting to set client while disposed'); return; } + this.logger.verbose(`Setting new file watcher client: ${client ? 'connected' : 'disconnected'}`); this.client = client; }
12-12: 建议实现文件监视器心跳机制根据 PR 评论的建议，应该添加以下功能：
- 实现监视器心跳检测机制
- 添加进程终止和重启逻辑
- 增加监视器状态的健康检查
这些改进将提高文件监视器的可靠性和稳定性。
需要我帮助实现这些功能吗？我可以提供详细的实现方案。
packages/file-service/src/common/watcher.ts (2)
59-60: 建议为常量添加详细的文档注释这些常量用于进程间通信，建议添加详细的 JSDoc 文档说明其用途和格式。
+/** + * 用于在父子进程间传递 socket 连接信息的键名 + */ export const KT_WATCHER_PROCESS_SOCK_KEY = 'kt-watcher-process-sock'; + +/** + * 用于在父子进程间传递初始化数据的键名 + */ export const WATCHER_INIT_DATA_KEY = 'kt-watcher-init-data';
62-69: 建议为接口方法添加参数验证
IWatcherHostService接口的方法应该添加参数验证，以确保传入的参数符合预期。export interface IWatcherHostService { - $watch(uri: UriComponents, options?: { excludes?: string[]; recursive?: boolean }): Promise<number>; + $watch(uri: UriComponents, options?: { + excludes?: string[]; + recursive?: boolean; + /** + * 监视器超时时间(ms) + * @default 30000 + */ + timeout?: number; + }): Promise<number>; $unwatch(watchId: number): Promise<void>; $setWatcherFileExcludes(excludes: string[]): Promise<void>; $dispose(): Promise<void>; initWatcherServer(): void; }packages/file-service/src/browser/file-service-provider-client.ts (1)
109-117: 建议改进错误处理和日志记录初始化方法的错误处理可以更详细，并考虑添加重试机制。
async initialize(clientId: string) { if (this.fileServiceProvider?.initialize) { + const logger = getDebugLogger('fileService.fsProvider'); + const maxRetries = 3; + let retryCount = 0; + + while (retryCount < maxRetries) { try { await this.fileServiceProvider?.initialize(clientId); + return; } catch (err) { - getDebugLogger('fileService.fsProvider').error('initialize error', err); + retryCount++; + logger.error( + `初始化失败 (尝试 ${retryCount}/${maxRetries}): ${err.message}`, + err + ); + if (retryCount < maxRetries) { + await new Promise(resolve => setTimeout(resolve, 1000 * retryCount)); + } } + } + throw new Error(`初始化失败，已重试 ${maxRetries} 次`); } }packages/file-service/src/node/hosted/watcher.process.ts (1)
124-136: 建议优化未处理的 Promise rejection 处理逻辑当前实现使用固定的 1 秒延迟检查未处理的 Promise，建议使用更灵活的方式。
-const unhandledPromises: Promise<any>[] = []; +interface UnhandledPromise { + promise: Promise<any>; + timestamp: number; + reason: any; +} +const unhandledPromises: UnhandledPromise[] = []; +const PROMISE_CHECK_INTERVAL = 5000; // 5 秒检查一次 +const PROMISE_TIMEOUT = 30000; // 30 秒超时 + +setInterval(() => { + const now = Date.now(); + unhandledPromises.forEach((entry, index) => { + if (now - entry.timestamp > PROMISE_TIMEOUT) { + unhandledPromises.splice(index, 1); + onUnexpectedError( + new Error(`Promise 超时未处理: ${entry.reason}`) + ); + } + }); +}, PROMISE_CHECK_INTERVAL); process.on('unhandledRejection', (reason, promise) => { - unhandledPromises.push(promise); + unhandledPromises.push({ + promise, + timestamp: Date.now(), + reason + }); - setTimeout(() => { - const idx = unhandledPromises.indexOf(promise); - if (idx >= 0) { - promise.catch((e) => { - unhandledPromises.splice(idx, 1); - onUnexpectedError(e); - }); - } - }, 1000); });packages/file-service/src/node/file-service.ts (1)
98-106: 建议优化可选链使用在
watchFileChanges方法中，可以使用可选链来简化条件判断。建议按照以下方式修改:
async watchFileChanges(uri: string, options?: { excludes: string[]; recursive?: boolean }): Promise<number> { const id = this.watcherId++; const _uri = this.getUri(uri); const provider = await this.getProvider(_uri.scheme); const schemaWatchIdList = this.watcherWithSchemaMap.get(_uri.scheme) || []; const watcherId = await provider.watch(_uri.codeUri, { - excludes: (options && options.excludes) || [], + excludes: options?.excludes ?? [], - recursive: options && options.recursive !== undefined ? options.recursive : this.options.recursive, + recursive: options?.recursive ?? this.options.recursive, });🧰 Tools
🪛 Biome (1.9.4)
[error] 105-105: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
package.json(1 hunks)packages/core-browser/src/core-preferences.ts(2 hunks)packages/core-browser/src/react-providers/config-provider.tsx(1 hunks)packages/core-common/src/log.ts(4 hunks)packages/core-common/src/preferences/file-watch.ts(1 hunks)packages/core-common/src/types/file-watch.ts(1 hunks)packages/core-common/src/types/file.ts(1 hunks)packages/core-common/src/types/reporter.ts(2 hunks)packages/core-node/src/types.ts(1 hunks)packages/extension/__tests__/node/extension.service.client.test.ts(2 hunks)packages/extension/src/hosted/ext.process-base.ts(2 hunks)packages/extension/src/hosted/extension-reporter.ts(0 hunks)packages/file-service/__tests__/browser/file-service-client.test.ts(1 hunks)packages/file-service/__tests__/node/file-node-watcher.test.ts(3 hunks)packages/file-service/__tests__/node/file-service-watcher.test.ts(1 hunks)packages/file-service/__tests__/node/index.test.ts(2 hunks)packages/file-service/package.json(1 hunks)packages/file-service/src/browser/file-service-client.ts(5 hunks)packages/file-service/src/browser/file-service-contribution.ts(1 hunks)packages/file-service/src/browser/file-service-provider-client.ts(1 hunks)packages/file-service/src/common/file-service-client.ts(1 hunks)packages/file-service/src/common/files.ts(1 hunks)packages/file-service/src/common/watcher.ts(2 hunks)packages/file-service/src/node/disk-file-system.provider.ts(7 hunks)packages/file-service/src/node/file-service.ts(2 hunks)packages/file-service/src/node/hosted/recursive/file-service-watcher.ts(6 hunks)packages/file-service/src/node/hosted/shared/file-type.ts(1 hunks)packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts(3 hunks)packages/file-service/src/node/hosted/watch-process-log.ts(1 hunks)packages/file-service/src/node/hosted/watcher.host.service.ts(1 hunks)packages/file-service/src/node/hosted/watcher.process.ts(1 hunks)packages/file-service/src/node/index.ts(2 hunks)packages/file-service/src/node/watcher-process-manager.ts(1 hunks)packages/workspace/src/browser/workspace-service.ts(2 hunks)tools/dev-tool/src/server.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/extension/src/hosted/extension-reporter.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- packages/file-service/src/common/files.ts
- packages/core-common/src/types/file-watch.ts
- packages/file-service/src/browser/file-service-contribution.ts
- packages/file-service/package.json
- packages/file-service/src/node/index.ts
- packages/core-node/src/types.ts
- packages/core-browser/src/react-providers/config-provider.tsx
- packages/file-service/tests/browser/file-service-client.test.ts
- packages/core-browser/src/core-preferences.ts
- packages/core-common/src/log.ts
- packages/core-common/src/types/file.ts
- tools/dev-tool/src/server.ts
- packages/workspace/src/browser/workspace-service.ts
- packages/core-common/src/types/reporter.ts
- packages/extension/tests/node/extension.service.client.test.ts
- packages/extension/src/hosted/ext.process-base.ts
- packages/core-common/src/preferences/file-watch.ts
- packages/file-service/src/node/hosted/watch-process-log.ts
- package.json
- packages/file-service/tests/node/file-service-watcher.test.ts
- packages/file-service/src/node/hosted/shared/file-type.ts
- packages/file-service/src/node/watcher-process-manager.ts
- packages/file-service/src/common/file-service-client.ts
- packages/file-service/tests/node/file-node-watcher.test.ts
👮 Files not reviewed due to content moderation or server errors (3)
- packages/file-service/src/node/disk-file-system.provider.ts
- packages/file-service/src/browser/file-service-client.ts
- packages/file-service/tests/node/index.test.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/file-service/src/node/hosted/watcher.process.ts
[error] 50-50: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 55-55: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/file-service/src/node/file-service.ts
[error] 105-105: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (7)
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (2)
4-7: 需要添加 @Injectable 装饰器
类缺少 @Injectable 装饰器，这可能会影响依赖注入系统的正常工作。建议添加装饰器以确保类能够正确注册到 DI 容器中。
+import { Injectable } from '@opensumi/di';
+@Injectable()
export class UnRecursiveFileSystemWatcher implements IFileSystemWatcherServer {33-33: 验证日志服务在子进程中的可用性
由于文件监视器现在运行在子进程中，需要确保 ILogService 在子进程上下文中正常工作。建议：
- 验证日志服务是否能够跨进程正常传输日志
- 考虑添加进程间通信的错误处理机制
- 确保日志不会在进程间传输时丢失
packages/file-service/src/node/hosted/watcher.host.service.ts (3)
117-126: $unwatch 方法中需要完善资源清理
在 $unwatch 方法中，当找到匹配的 watcherId 并执行 disposable.dispose() 后，需要确保从 watcherCollection 中删除对应的条目。
133-138:
进程退出处理需要优化
在 $dispose 方法中直接调用 process.exit(0) 可能会导致资源未能正确清理。建议实现更优雅的退出机制。
建议按照以下方式修改:
async $dispose(): Promise<void> {
this.logger.log('dispose watcher host service');
this.unrecursiveFileSystemWatcher?.dispose();
this.recursiveFileSystemWatcher?.dispose();
- process.exit(0);
+ await Promise.all([
+ ...Array.from(this.watcherCollection.values()).map(watcher =>
+ watcher.disposable.dispose()
+ )
+ ]);
+ process.emit('SIGTERM');
}Likely invalid or redundant comment.
86-90:
doWatch 方法的错误处理需要加强
当 watcherServer 不存在时直接返回 -1，缺少错误日志记录和错误处理机制。
建议按照以下方式修改:
private async doWatch(uri: Uri, options?: { excludes?: string[]; recursive?: boolean }): Promise<number> {
const watcherServer = this.getWatcherServer(options?.recursive);
if (!watcherServer) {
+ this.logger.error(`获取 watcher server 失败: ${uri.toString()}`);
+ throw new Error('Watcher server 未初始化');
- return -1;
}Likely invalid or redundant comment.
packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (2)
293-296: 客户端设置逻辑存在潜在问题
当前的检查逻辑可能导致已释放的实例仍然接收到新的客户端。
183-184: 🛠️ Refactor suggestion
updateWatcherFileExcludes 实现需要增强
当前实现仅更新了 excludes 属性，但没有触发现有 watcher 的更新。
建议按照以下方式修改:
async updateWatcherFileExcludes(excludes: string[]): Promise<void> {
this.excludes = excludes;
+ // 更新所有现有 watcher 的排除模式
+ for (const [watcherId, _] of this.WATCHER_HANDLERS) {
+ const options = this.watcherOptions.get(watcherId);
+ if (options) {
+ options.excludesPattern = excludes.map((pattern) => parseGlob(pattern));
+ options.excludes = excludes;
+ }
+ }
}Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/file-service/src/common/watcher.ts (1)
62-69: 建议添加接口方法的文档注释
IWatcherHostService接口的设计很好，但建议为每个方法添加 JSDoc 文档注释，特别是：
initWatcherServer方法的用途和时机$watch方法的返回值含义options参数中excludes的格式要求示例文档：
export interface IWatcherHostService { + /** + * 监听指定 URI 的文件变化 + * @param uri 要监听的文件或目录的 URI + * @param options 监听选项 + * @returns 监听器 ID + */ $watch(uri: UriComponents, options?: { excludes?: string[]; recursive?: boolean }): Promise<number>;packages/file-service/src/node/hosted/watcher.process.ts (2)
17-17: 建议改进 logger 的类型定义将 logger 声明为
any类型可能会导致类型安全问题。建议定义一个具体的 logger 接口类型。-const logger: any = console; +interface ILogger { + error: (...args: any[]) => void; + warn: (...args: any[]) => void; + info: (...args: any[]) => void; + log: (...args: any[]) => void; +} +const logger: ILogger = console;
48-51: 优化错误日志处理器的空值检查使用可选链操作符可以使代码更简洁和安全。
- return (logger && logger.error.bind(logger)) || console.error.bind(console); + return logger?.error.bind(logger) ?? console.error.bind(console);🧰 Tools
🪛 Biome (1.9.4)
[error] 50-50: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/file-service/src/common/watcher.ts(2 hunks)packages/file-service/src/node/hosted/watcher.process.ts(1 hunks)packages/file-service/src/node/watcher-process-manager.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/file-service/src/node/watcher-process-manager.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/file-service/src/node/hosted/watcher.process.ts
[error] 50-50: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 55-55: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
packages/file-service/src/common/watcher.ts (3)
1-3: 导入和常量定义看起来不错！
新增的导入和常量定义清晰地表明了进程间通信的意图，并且常量命名符合项目规范。
Also applies to: 59-60
71-73: 代理标识符定义合理
WatcherServiceProxy 和 WatcherProcessManagerProxy 的定义符合 RPC 通信的需求。
75-87:
需要实现心跳检测机制
根据之前的评审意见，建议在 IWatcherProcessManager 接口中添加心跳检测相关的方法，以提高子进程的可靠性：
export interface IWatcherProcessManager {
whenReady: Promise<void>;
+
+ /**
+ * 心跳检测间隔（毫秒）
+ * @default 30000
+ */
+ readonly heartbeatInterval?: number;
+
+ /**
+ * 发送心跳信号
+ */
+ sendHeartbeat(): Promise<void>;
+
+ /**
+ * 监听心跳超时事件
+ */
+ onHeartbeatTimeout: Event<void>;
+
+ /**
+ * 重启进程
+ */
+ restart(): Promise<void>;
createProcess(clientId: string): Promise<number | undefined>;
// ... 其他现有方法
}Likely invalid or redundant comment.
packages/file-service/src/node/hosted/watcher.process.ts (1)
19-42: 建议添加进程退出处理和资源清理
需要添加进程退出时的资源清理逻辑，包括关闭 socket 连接和释放监听器等资源。
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
packages/keymaps/__tests__/browser/keymaps.service.test.ts (3)
7-7: 新增的依赖导入需要更新测试文档这些新增的依赖项表明文件监视功能已集成到按键映射服务中。建议在测试文件的顶部添加注释，说明这些依赖的用途。
建议添加如下注释：
+/** + * 按键映射服务测试 + * 依赖说明： + * - WSChannelHandler: 用于处理文件监视的WebSocket通信 + * - WatcherProcessManagerToken: 用于管理文件监视进程 + */Also applies to: 23-24, 29-29
107-121: 文件监视相关的模拟实现可以优化当前的模拟实现过于简单，可能无法完全测试文件监视功能对按键映射的影响。
建议改进模拟实现：
{ token: WSChannelHandler, useValue: { clientId: 'test_client_id', + // 添加更多必要的模拟方法 + sendMessage: jest.fn(), + onMessage: jest.fn(), }, }, { token: WatcherProcessManagerToken, useValue: { - setClient: () => void 0, - watch: (() => 1) as any, - unWatch: () => void 0, - createProcess: () => void 0, + setClient: jest.fn(), + watch: jest.fn().mockReturnValue(1), + unWatch: jest.fn(), + createProcess: jest.fn(), + // 添加用于验证调用的间谍函数 + dispose: jest.fn(), }, },
Line range hint
1-285: 建议添加文件监视相关的测试用例现有测试用例未覆盖新增的文件监视功能。
建议添加以下测试场景：
- 文件监视启动和停止
- 文件变更触发按键映射重新加载
- 监视进程异常处理
需要我帮您生成这些测试用例的代码吗？
packages/file-tree-next/__tests__/browser/file-tree.test.ts (1)
212-219: 建议增强 WatcherProcessManagerToken 的模拟实现当前的模拟实现过于简单，建议添加以下功能：
- 监听器状态追踪
- 错误场景模拟
- 进程生命周期事件模拟
injector.addProviders({ token: WatcherProcessManagerToken, useValue: { + private watcherStatus: Map<string, boolean> = new Map(), setClient: () => void 0, - watch: (() => 1) as any, + watch: ((path: string) => { + this.watcherStatus.set(path, true); + return 1; + }) as any, - unWatch: () => void 0, + unWatch: (path: string) => { + this.watcherStatus.delete(path); + }, + isWatching: (path: string) => this.watcherStatus.has(path), + simulateError: (path: string) => { + // 用于测试错误处理 + } }, });packages/preferences/__tests__/browser/preference-service.test.ts (1)
204-209: WebSocket通道处理器的模拟配置可以优化当前的模拟实现仅包含clientId，建议考虑添加其他可能需要的方法存根，以防止测试中出现未定义方法的错误。
建议扩展模拟实现：
{ token: WSChannelHandler, useValue: { clientId: 'test_client_id', + // 添加常用方法的存根 + dispose: () => void 0, + onMessage: () => void 0, }, }packages/extension-storage/__tests__/browser/storage.test.ts (1)
127-142: 避免代码重复这段模拟实现与之前的实现完全相同。建议将这些共同的模拟对象提取到一个共享的测试工具文件中。
建议创建一个共享的测试工具文件，例如
test-utils.ts：// test-utils.ts export const mockWatcherProcessManager = { setClient: () => void 0, watch: (() => 1) as any, unWatch: () => void 0, createProcess: () => void 0, setWatcherFileExcludes: () => void 0, heartbeat: () => true, restartProcess: () => void 0, };packages/storage/__tests__/browser/index.test.ts (1)
Line range hint
446-460: 建议统一管理模拟对象这里的模拟实现与其他测试文件类似，建议统一管理这些模拟对象。
建议使用之前提到的共享测试工具文件。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/debug/__tests__/browser/launch-preferences.test.ts(3 hunks)packages/extension-storage/__tests__/browser/storage.test.ts(3 hunks)packages/extension/__tests__/browser/main.thread.workspace.test.ts(2 hunks)packages/file-service/__tests__/browser/file-service-client.test.ts(2 hunks)packages/file-tree-next/__tests__/browser/file-tree.test.ts(5 hunks)packages/keymaps/__tests__/browser/keymaps.service.test.ts(3 hunks)packages/preferences/__tests__/browser/preference-service.test.ts(3 hunks)packages/storage/__tests__/browser/index.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/file-service/tests/browser/file-service-client.test.ts
🔇 Additional comments (6)
packages/file-tree-next/__tests__/browser/file-tree.test.ts (1)
Line range hint 327-331: LGTM! 面板收起测试逻辑完善
测试用例完整地覆盖了面板收起的功能：
- 正确模拟了命令执行
- 验证了状态变化
- 包含了回调验证
packages/extension/__tests__/browser/main.thread.workspace.test.ts (1)
87-87: 导入声明看起来不错！
导入语句清晰明确，符合代码规范。
packages/preferences/__tests__/browser/preference-service.test.ts (1)
7-7: 导入语句看起来没有问题！
新增的导入语句与后续的依赖注入配置相匹配，结构清晰。
Also applies to: 30-31, 36-36
packages/extension-storage/__tests__/browser/storage.test.ts (1)
6-6: 导入声明看起来不错！
新增的导入声明清晰地表明了对WebSocket通道和文件监视器的依赖。
Also applies to: 10-11, 16-16
packages/storage/__tests__/browser/index.test.ts (1)
7-7: 导入声明符合一致性要求
新增的导入声明与其他测试文件保持一致，这是个好的实践。
Also applies to: 13-13
packages/debug/__tests__/browser/launch-preferences.test.ts (1)
6-6: 导入声明组织合理
新增的导入声明按照功能模块清晰地组织，便于理解依赖关系。
Also applies to: 26-27
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4202 +/- ##
==========================================
- Coverage 53.93% 53.72% -0.21%
==========================================
Files 1618 1622 +4
Lines 98300 98555 +255
Branches 20123 20139 +16
==========================================
- Hits 53014 52948 -66
- Misses 37624 37903 +279
- Partials 7662 7704 +42
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
/next |
|
🎉 PR Next publish successful! 3.6.4-next-1734515084.0 |
5 participants
Types
Background or solution
Changelog
Summary by CodeRabbit
Summary by CodeRabbit
watcher.process进程用于实现文件监听unRecursiveDirectories配置，用于指定部分目录使用非递归的文件监听实现WatcherHostServiceImpl类，管理递归和非递归文件系统监听WatcherProcessLogger类，管理文件监听进程的日志记录updateWatcherFileExcludes方法，支持动态更新文件排除列表WatcherProcessManagerImpl类，负责管理文件变更的监视进程initialize方法，支持在文件服务客户端和磁盘文件提供者中进行初始化flattenExcludes方法提升可读性watchFileChanges方法，增加递归监听的灵活性FileService和DiskFsProviderClient的初始化逻辑，增强功能性