feat: support polling watcher #4299
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4299 +/- ##
==========================================
+ Coverage 54.18% 54.22% +0.04%
==========================================
Files 1638 1638
Lines 100084 100147 +63
Branches 21731 21736 +5
==========================================
+ Hits 54233 54308 +75
+ Misses 38095 38090 -5
+ Partials 7756 7749 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Aaaaash
force-pushed
the
feat/add-polling-watcher
branch
from
41aa192 to
4f05f8e
Compare
|
/next |
|
🎉 PR Next publish successful! 3.6.5-next-1736143358.0 |
|
/next |
|
🎉 PR Next publish successful! 3.6.5-next-1736220658.0 |
|
/next |
|
🎉 PR Next publish successful! 3.6.5-next-1736222491.0 |
|
/next |
|
🎉 PR Next publish successful! 3.6.5-next-1736232455.0 |
|
/next |
|
🎉 PR Next publish successful! 3.6.5-next-1736238377.0 |
|
/publish |
|
/next |
|
🎉 PR Next publish successful! 3.6.5-next-1736242999.0 |
|
/next |
|
🎉 PR Next publish successful! 3.6.5-next-1736298613.0 |
Aaaaash
changed the title
Aaaaash
force-pushed
the
feat/add-polling-watcher
branch
from
cc1b101 to
5feff59
Compare
|
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 [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. 概述演练这个拉取请求引入了文件监控和配置管理的重大改进。主要变更包括重命名 变更
可能相关的 PR
建议标签
建议审阅者
✨ Finishing Touches
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: 3
🧹 Nitpick comments (7)
packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (1)
313-349: 建议将轮询间隔设为可配置当前的
DEFAULT_POLLING_INTERVAL被设为100毫秒。为了提高灵活性，建议将轮询间隔设为可配置参数，使其能够根据不同的使用场景进行调整，以优化性能。packages/core-common/src/types/file-watch.ts (1)
28-50: 新增了更灵活的文件监听接口
IWatcher接口提供了基于 URI 的文件监听管理,相比原有接口更加灵活。建议考虑以下几点:
- 文档注释使用英文可以提高国际化友好度
- 考虑添加对监听状态的查询方法
packages/file-service/src/browser/file-service-provider-client.ts (1)
Line range hint
110-117: 建议完善错误处理和日志记录initialize 方法的实现逻辑正确，但建议：
- 在 catch 块中添加更详细的错误类型处理
- 考虑在成功初始化后添加日志记录
async initialize(clientId: string, backend?: RecursiveWatcherBackend) { if (this.fileServiceProvider?.initialize) { try { await this.fileServiceProvider?.initialize(clientId, backend); + getDebugLogger('fileService.fsProvider').log('File service provider initialized successfully'); } catch (err) { - getDebugLogger('fileService.fsProvider').error('initialize error', err); + getDebugLogger('fileService.fsProvider').error('Failed to initialize file service provider', { + error: err, + clientId, + backend + }); + throw err; } } }packages/file-service/src/node/watcher-process-manager.ts (1)
166-169: 建议添加 pollingWatch 选项的文档说明watch 方法新增了 pollingWatch 选项，但缺少相关文档说明。建议添加 JSDoc 注释说明该选项的用途和影响。
+/** + * 监视指定 URI 的文件变化 + * @param uri 要监视的 URI + * @param options 监视选项 + * @param options.excludes 排除的文件模式 + * @param options.recursive 是否递归监视子目录 + * @param options.pollingWatch 是否使用轮询方式监视（在某些文件系统中可能需要） + * @returns 监视器 ID + */ async watch( uri: UriComponents, options?: { excludes?: string[]; recursive?: boolean; pollingWatch: boolean }, ): Promise<number>packages/file-tree-next/src/browser/services/file-tree-api.service.ts (1)
50-50: 建议在发射事件前进行空值检查在发射事件时，建议先检查 URI 是否存在，以避免潜在的空值错误。
- this.onDidResolveChildrenEmitter.fire(file.uri.toString()); + if (file.uri) { + this.onDidResolveChildrenEmitter.fire(file.uri.toString()); + }packages/core-common/src/types/file.ts (1)
160-163: 文件监视选项得到增强
watch方法的选项接口得到了增强：
- 新增了
pollingWatch选项，使文件监视实现更加灵活- 保持了与现有选项的兼容性
建议添加以下内容：
- 为
pollingWatch选项添加 JSDoc 注释，说明其用途和默认值watch( uri: Uri, - options: { excludes?: string[]; recursive?: boolean; pollingWatch?: boolean }, + options: { + /** 排除的文件模式 */ + excludes?: string[]; + /** 是否递归监视子目录 */ + recursive?: boolean; + /** 是否使用轮询方式监视文件变化。默认值：false */ + pollingWatch?: boolean; + }, ): number | Promise<number>;packages/core-browser/src/react-providers/config-provider.tsx (1)
323-333: 新增配置项的文档完善新增的配置项具有清晰的文档说明：
pollingWatcherDirectories: 用于配置需要使用轮询方式监视的目录recursiveWatcherBackend: 用于配置递归监视的后端类型，默认使用nsfw建议补充以下内容：
- 为
pollingWatcherDirectories添加示例值- 说明
recursiveWatcherBackend可选的值/** * Polling watcher directories + * @example ['/path/to/dir1', '/path/to/dir2'] */ pollingWatcherDirectories?: string[]; /** * Recursive watcher backend type * + * Available values: + * - 'nsfw': Native file system watcher (default) + * - 'parcel': Parcel file watcher * * Default value is `nsfw` */ recursiveWatcherBackend?: RecursiveWatcherBackend;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
packages/core-browser/src/react-providers/config-provider.tsx(2 hunks)packages/core-common/src/types/file-watch.ts(3 hunks)packages/core-common/src/types/file.ts(1 hunks)packages/file-service/__tests__/browser/file-service-client.test.ts(2 hunks)packages/file-service/__tests__/node/file-service-watcher.test.ts(3 hunks)packages/file-service/src/browser/file-service-client.ts(3 hunks)packages/file-service/src/browser/file-service-provider-client.ts(2 hunks)packages/file-service/src/common/files.ts(2 hunks)packages/file-service/src/common/watcher.ts(2 hunks)packages/file-service/src/node/disk-file-system.provider.ts(3 hunks)packages/file-service/src/node/hosted/recursive/file-service-watcher.ts(10 hunks)packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts(6 hunks)packages/file-service/src/node/hosted/watcher.host.service.ts(4 hunks)packages/file-service/src/node/hosted/watcher.process.ts(1 hunks)packages/file-service/src/node/watcher-process-manager.ts(4 hunks)packages/file-tree-next/src/browser/services/file-tree-api.service.ts(3 hunks)packages/file-tree-next/src/common/index.ts(2 hunks)packages/startup/entry/web/render-app.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
- GitHub Check: unittest (macos-latest, 18.x, node)
🔇 Additional comments (26)
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (2)
13-13: 实现接口更改符合预期将
UnRecursiveFileSystemWatcher类实现的接口从IFileSystemWatcherServer更改为IWatcher，符合代码重构的目的。
148-154: 更新 unwatchFileChanges 方法以接受 URI
unwatchFileChanges方法已更新为接受 URI 字符串，符合IWatcher接口的定义。这确保了方法签名的一致性。packages/file-service/src/node/hosted/watcher.host.service.ts (1)
55-59: 更新构造函数以支持后端配置构造函数增加了
backend参数，允许配置递归文件系统观察器的后端，实现更灵活的文件监控。packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (1)
376-378: 确认逻辑运算符的正确使用在方法
isEnableNSFW中，使用||操作符来确定是否启用 NSFW 是正确的。因为当this.backend === RecursiveWatcherBackend.NSFW为true或isLinux为true时，函数应返回true。将||更换为??不适用于布尔逻辑，可能导致逻辑错误，因为??仅在左侧操作数为null或undefined时才返回右侧操作数。packages/file-service/src/common/watcher.ts (2)
82-82: 添加了对不同后端实现的支持通过添加可选的
backend参数,使文件监听系统能够支持不同的后端实现(如 nsfw 或 parcel)。这种设计提供了更好的扩展性。
86-89: 增强了文件监听选项通过添加
pollingWatch选项,支持了轮询式文件监听。这对于某些特殊场景(如网络文件系统)非常有用。建议验证在不同文件系统下的性能表现:
packages/file-tree-next/src/common/index.ts (1)
22-23: 添加了子节点解析事件支持通过
onDidResolveChildren事件,可以在文件树子节点解析完成时得到通知。这对于实现 UI 更新和性能优化很有帮助。建议验证事件处理的实现:
✅ Verification successful
事件实现已完整验证
事件实现完整且正确:
- 在
file-tree-api.service.ts中通过Emitter/Event模式实现- 在文件树子节点解析完成时正确触发事件
- 使用 URI 字符串作为事件参数提供节点标识
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查事件的实现和使用 ast-grep --pattern 'onDidResolveChildren.fire($_)'Length of output: 52
Script:
#!/bin/bash # 查找 onDidResolveChildren 的声明和使用 echo "=== 查找事件声明 ===" rg "onDidResolveChildren.*Event" -A 2 echo -e "\n=== 查找事件的所有引用 ===" rg "onDidResolveChildren" -A 2 echo -e "\n=== 查找 Event 相关实现 ===" rg "class.*extends.*Event" -A 2Length of output: 45238
packages/core-common/src/types/file-watch.ts (2)
61-61: 支持轮询式文件监听在
WatchOptions中添加pollingWatch选项,增强了对特殊文件系统的支持。
133-136: 定义了文件监听后端类型通过
RecursiveWatcherBackend枚举明确定义了支持的后端类型,提高了代码的可维护性。建议验证后端实现的完整性:
✅ Verification successful
后端实现验证完成
经过验证，
RecursiveWatcherBackend枚举定义的 NSFW 和 Parcel 两种后端都已在packages/file-service/src/node/hosted/recursive/file-service-watcher.ts中得到完整实现，并在项目中得到了正确使用。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查后端实现 rg -l "class.*implements.*Watcher.*Backend"Length of output: 45
Script:
#!/bin/bash # 搜索与文件监听相关的实现 echo "=== 搜索 RecursiveWatcherBackend 的使用 ===" rg -l "RecursiveWatcherBackend" echo -e "\n=== 搜索 NSFW 相关实现 ===" rg -l "nsfw.*[Ww]atcher|[Ww]atcher.*nsfw" echo -e "\n=== 搜索 Parcel 相关实现 ===" rg -l "parcel.*[Ww]atcher|[Ww]atcher.*parcel" echo -e "\n=== 搜索所有 Watcher 相关文件 ===" rg -l "[Ww]atcher"Length of output: 8717
packages/file-service/src/node/hosted/watcher.process.ts (1)
40-40: 支持配置文件监听后端通过传入
initData.backend参数,使WatcherHostServiceImpl能够使用指定的后端实现。建议验证配置传递的完整性:
✅ Verification successful
文件监听后端配置传递验证完成
通过代码分析确认，backend 配置参数通过 WATCHER_INIT_DATA_KEY 完整地从进程参数传递到 WatcherHostServiceImpl 实现中，配置传递链路完整可靠。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查配置的传递链路 rg "backend.*WatcherHostServiceImpl" -A 3Length of output: 43
Script:
#!/bin/bash # Search for WatcherHostServiceImpl class definition and usage ast-grep --pattern 'class WatcherHostServiceImpl' # Search for initData references in watcher context rg "initData" --type ts packages/file-service # Search for backend configuration patterns rg "backend.*watcher" --type ts packages/file-serviceLength of output: 580
Script:
#!/bin/bash # Search for watcher initialization related types and constants rg "WATCHER_INIT_DATA|interface.*Watcher|type.*Watcher" --type ts packages/file-service # Search for WatcherHostServiceImpl implementation rg "class WatcherHostServiceImpl" -A 10 packages/file-service # Search for backend type definitions ast-grep --pattern 'interface $_ { backend: $_; }'Length of output: 2467
Script:
#!/bin/bash # Search for FileServiceWatcherOptions definition rg -A 10 "interface FileServiceWatcherOptions" packages/file-service # Search for backend type definitions in watcher context rg "backend.*type|type.*backend" --type ts packages/file-serviceLength of output: 868
packages/startup/entry/web/render-app.tsx (1)
33-33: 验证 RecursiveWatcherBackend 导入的必要性该导入语句目前在文件中未被使用。如果这是为未来的功能准备的，建议添加 TODO 注释说明用途；如果不需要，建议移除。
packages/file-service/src/browser/file-service-provider-client.ts (1)
6-6: 导入声明看起来不错！RecursiveWatcherBackend 的导入位置正确，并且与其他相关导入保持一致。
packages/file-service/src/node/watcher-process-manager.ts (1)
10-14: 导入声明组织合理！导入声明的组织结构清晰，相关的类型都已正确导入。
packages/file-tree-next/src/browser/services/file-tree-api.service.ts (1)
15-16: 事件发射器实现正确事件发射器的实现遵循了标准模式，可以正确通知订阅者文件树子节点的解析状态。
packages/file-service/__tests__/browser/file-service-client.test.ts (2)
13-13: 导入语句更新正确导入语句的更新与类名的重命名保持一致。
53-53: Mock 设置更新正确Mock 设置已正确更新以使用新的类名
RecursiveFileSystemWatcher。packages/core-browser/src/react-providers/config-provider.tsx (1)
319-319: 正确标记了废弃的配置项使用
@deprecated标记废弃的配置项是一个好的实践。packages/file-service/__tests__/node/file-service-watcher.test.ts (2)
10-10: 类型引用更新正确导入语句和类型声明已正确更新为
RecursiveFileSystemWatcher。Also applies to: 16-16
23-23: 实例化和变量声明更新正确所有使用
FileSystemWatcherServer的地方都已正确更新为RecursiveFileSystemWatcher。Also applies to: 166-166, 176-176
packages/file-service/src/common/files.ts (2)
32-32: 导入 RecursiveWatcherBackend 类型导入了
RecursiveWatcherBackend类型以支持文件系统提供者的初始化配置。
423-423: IDiskFileProvider 接口更新
initialize方法签名更新添加了可选的backend参数，这样可以在保持向后兼容性的同时支持不同的文件监视后端实现。packages/file-service/src/node/disk-file-system.provider.ts (3)
41-41: 导入 RecursiveWatcherBackend 类型导入了
RecursiveWatcherBackend类型以支持文件系统提供者的初始化配置。
109-110: 更新 initialize 方法实现将
backend参数传递给watcherProcessManager.createProcess，实现了对不同文件监视后端的支持。
147-157: 增强 watch 方法的灵活性添加了
pollingWatch选项以支持基于轮询的文件监视机制，这样可以根据不同场景选择合适的监视策略。packages/file-service/src/browser/file-service-client.ts (2)
146-146: 传递递归监视后端配置将
recursiveWatcherBackend配置传递给文件系统提供者的初始化方法。
357-358: 验证轮询监视的影响基于目录路径添加了轮询监视支持。这个改动可能会影响使用该配置的集成方，需要进行验证。
请运行以下脚本来验证轮询监视的使用情况：
#!/bin/bash # 描述：检查轮询监视的使用情况 # 搜索可能受影响的配置和调用 # 检查配置文件中的 pollingWatcherDirectories 配置 rg -l "pollingWatcherDirectories" # 检查文件监视相关的调用 ast-grep --pattern 'watchFileChanges($$$)'Also applies to: 377-377
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts
Show resolved
Hide resolved
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (1)
Line range hint
251-255: 优化大量事件的处理机制当前实现对超过5000个事件的情况直接忽略，这可能导致文件变更未被正确处理。建议：
- 将事件数量限制设置为可配置项
- 对大量事件进行分批处理而不是直接忽略
- 添加警告日志以便追踪此类情况
-// 对于超过 5000 数量的 events 做屏蔽优化，避免潜在的卡死问题 -if (events.length > 5000) { - // FIXME: 研究此处屏蔽的影响，考虑下阈值应该设置多少，或者更加优雅的方式 - return; -} +const MAX_EVENTS = 5000; // 配置项，可从构造函数传入 +if (events.length > MAX_EVENTS) { + this.logger.warn( + `[Recursive] Received ${events.length} events, splitting into batches of ${MAX_EVENTS}` + ); + // 分批处理事件 + for (let i = 0; i < events.length; i += MAX_EVENTS) { + const batch = events.slice(i, i + MAX_EVENTS); + // 处理当前批次 + this.processBatch(batch); + } + return; +}
🧹 Nitpick comments (2)
packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (1)
84-87: 改进超时处理机制当前实现使用硬编码的3秒超时时间，建议：
- 将超时时间设置为可配置项
- 添加超时原因的详细日志
-const timer = setTimeout(() => { - rej(`Watch ${uri} Timeout`); - // FIXME：暂时写死3秒 -}, 3000); +const WATCH_TIMEOUT = 3000; // 配置项，可从构造函数传入 +const timer = setTimeout(() => { + const message = `Watch ${uri} timeout after ${WATCH_TIMEOUT}ms`; + this.logger.error('[Recursive] ' + message); + rej(message); +}, WATCH_TIMEOUT);packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (1)
165-171: 改进了取消监听的实现新的实现使用 URI 而不是 watcherId，并正确清理资源。建议添加错误处理。
建议应用以下修改：
async unwatchFileChanges(uri: string): Promise<void> { const basePath = FileUri.fsPath(uri); + try { if (this.watcherCollections.has(basePath)) { const watcher = this.watcherCollections.get(basePath); watcher?.close(); this.watcherCollections.delete(basePath); } + } catch (error) { + this.logger.error(`[Un-Recursive] Failed to unwatch ${basePath}: ${error}`); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/file-service/__tests__/node/file-service-watcher.test.ts(5 hunks)packages/file-service/__tests__/node/index.test.ts(2 hunks)packages/file-service/src/node/hosted/recursive/file-service-watcher.ts(10 hunks)packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: build-windows
- GitHub Check: ubuntu-latest, Node.js 20.x
🔇 Additional comments (9)
packages/file-service/__tests__/node/index.test.ts (1)
12-12: 代码变更正确！导入语句和模拟实现已正确更新为使用
RecursiveFileSystemWatcher。这些更改与整体重构保持一致。Also applies to: 30-30
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (5)
13-14: 实现更改和数据结构优化类现在实现
IWatcher接口，使用Map来管理文件监听器，这是一个更好的数据结构选择。
34-36: 改进了监听器管理和日志记录使用 Map 存储监听器并添加了清晰的日志前缀，这有助于调试和维护。
58-60: 统一了错误日志格式错误日志现在统一使用 "[Un-Recursive]" 前缀，提高了日志的可读性和可追踪性。
Also applies to: 116-116
147-150: 优化了重复监听检查使用 Map 来防止重复监听同一路径，这是一个很好的优化。
129-135:⚠️ Potential issue路径存在性检查需要改进
当路径不存在时，代码继续执行并尝试启动监听，这可能导致
undefined错误。建议应用以下修改：
if (exist) { const stat = await fs.lstat(basePath); if (stat) { watchPath = basePath; } } else { this.logger.warn('[Un-Recursive] This path does not exist. Please try again'); + return; }Likely invalid or redundant comment.
packages/file-service/__tests__/node/file-service-watcher.test.ts (3)
16-16: 类型声明更新正确监听器列表的类型已正确更新为
RecursiveFileSystemWatcher[]。Also applies to: 176-176
23-23: 构造函数调用更新正确测试中的监听器实例化已正确更新为使用
RecursiveFileSystemWatcher。Also applies to: 166-166
84-84: 取消监听方法调用更新正确
unwatchFileChanges的调用已正确更新为使用 URI 字符串而不是 watcherId。Also applies to: 149-149, 155-155
|
/next |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (1)
131-137:⚠️ Potential issue修复可能的未定义变量问题
当
exist为false时，变量watchPath可能未被初始化。在这种情况下，随后调用this.start(watchPath)时，watchPath为undefined，可能导致异常。应用以下修改以修复问题：
if (exist) { const stat = await fs.lstat(basePath); if (stat) { watchPath = basePath; } } else { this.logger.warn('[Un-Recursive] This path does not exist. Please try again'); + return; }
🧹 Nitpick comments (3)
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (3)
33-36: 优化日志记录和错误处理建议进一步改进日志记录：
- 添加更多上下文信息，如 watcher ID 或其他标识符
- 使用结构化日志格式，便于日志聚合和分析
- this.logger.log('[Un-Recursive] start watching', basePath); + this.logger.log({ + message: '[Un-Recursive] start watching', + path: basePath, + watcherId: this.watcherCollections.size, + timestamp: new Date().toISOString() + });
83-83: 优化文件变更检测逻辑当前的变更检测逻辑可能存在以下问题：
- 重复的文件系统操作可能影响性能
- 文件删除检测可能不够可靠
- 缺少对临时文件的处理
建议实现文件系统操作的缓存机制，并添加对临时文件的过滤：
+ const isTemporaryFile = (filename: string) => { + return filename.startsWith('.') || filename.endsWith('~'); + }; + + if (isTemporaryFile(changeFileName)) { + return; + } + const changePath = join(basePath, changeFileName); - const pathExist = await fs.pathExists(changePath); + const pathExist = await this.checkPathExistWithCache(changePath);Also applies to: 89-96
167-172: 完善资源清理逻辑建议增强
unwatchFileChanges方法的资源清理：
- 添加错误处理
- 确保所有相关资源都被正确释放
- 添加日志记录以便调试
async unwatchFileChanges(uri: string): Promise<void> { const basePath = FileUri.fsPath(uri); + try { if (this.watcherCollections.has(basePath)) { const watcher = this.watcherCollections.get(basePath); - watcher?.close(); + await new Promise<void>((resolve) => { + watcher?.close(); + resolve(); + }); this.watcherCollections.delete(basePath); + this.logger.debug(`[Un-Recursive] Stopped watching ${basePath}`); } + } catch (error) { + this.logger.error(`[Un-Recursive] Failed to unwatch ${basePath}: ${error}`); + throw error; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
🔇 Additional comments (1)
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (1)
5-7: 验证接口变更的影响范围从
IFileSystemWatcherServer切换到IWatcher接口是一个重要的架构变更。需要确保：
- 所有调用方都已适配新接口
- 新接口的所有方法都已正确实现
- 没有遗漏原接口的关键功能
运行以下脚本来验证接口变更的影响：
Also applies to: 12-13
✅ Verification successful
接口变更验证已完成
经过验证，从
IFileSystemWatcherServer到IWatcher的接口变更是安全的：
- 新旧接口保持了方法签名的兼容性
- 所有调用方都已正确适配
- 新增的可选方法
updateWatcherFileExcludes不会影响现有功能🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索可能受影响的调用方 rg "IFileSystemWatcherServer|UnRecursiveFileSystemWatcher" -A 3 # 检查接口定义确保完整实现 ast-grep --pattern 'interface IWatcher { $$$ }'Length of output: 8787
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts
Outdated
Show resolved
Hide resolved
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts
Outdated
Show resolved
Hide resolved
|
🎉 PR Next publish successful! 3.7.1-next-1736321498.0 |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/file-service/__tests__/node/file-service-watcher.test.ts (1)
Line range hint
112-121: 测试用例需要更新以匹配新的实现此测试用例仍在比较 watcherId（
expect(parentId === childId).toBeTruthy()），这与新的 RecursiveFileSystemWatcher 实现可能不一致。建议：
- 重新设计测试断言逻辑
- 考虑使用其他方式验证父子文件的监视关系
🧹 Nitpick comments (2)
packages/file-service/__tests__/node/file-service-watcher.test.ts (2)
Line range hint
1-289: 建议增加错误处理的测试用例当前测试用例主要覆盖了正常的文件操作场景。建议添加以下测试场景：
- 无效 URI 格式的处理
- 不存在路径的处理
- 权限错误的处理
- 重复 watch/unwatch 的处理
Line range hint
1-289: 建议改进测试实现的一致性虽然整体实现保持一致，但建议以下改进：
- 将测试辅助函数抽取到共享的 helper 文件
- 统一使用 async/await 而不是混用同步和异步操作
- 考虑使用更现代的测试模式，如 beforeEach 清理资源
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/file-service/__tests__/node/file-service-watcher.test.ts(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
🔇 Additional comments (4)
packages/file-service/__tests__/node/file-service-watcher.test.ts (4)
10-10: 导入和类型声明的更改看起来正确！导入语句和 watcherServerList 的类型声明已正确更新为 RecursiveFileSystemWatcher。
Also applies to: 16-16
161-161: 第二个测试套件的更改正确实施成功更新了 RecursiveFileSystemWatcher 的实例化和类型声明。
Also applies to: 171-171
82-82: 验证 unwatchFileChanges 的 URI 处理unwatchFileChanges 方法现在接受字符串 URI 而不是 watcherId。建议：
- 验证 URI 字符串的格式处理
- 确保跨平台兼容性（Windows/Unix 路径）
Also applies to: 144-144, 150-150
✅ Verification successful
已验证 unwatchFileChanges 的 URI 处理是正确的
代码中的 URI 处理方式符合以下要求：
- 使用标准的 URI 对象的 toString() 方法确保格式一致性
- 服务端实现正确处理了字符串 URI
- 跨平台兼容性由 URI 对象本身保证
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查 unwatchFileChanges 的使用方式 rg "unwatchFileChanges\(" packages/file-service/Length of output: 2154
22-26: 请验证移除 watcherId 的影响generateWatcher 函数不再返回 watcherId，这是符合新的 RecursiveFileSystemWatcher 实现的。但是建议验证：
- 所有测试用例是否都已适配这个改动
- 是否有遗漏的测试场景
✅ Verification successful
移除 watcherId 的改动已验证安全
通过分析测试文件，确认：
- 所有测试用例都已经适配了新的实现方式
- 没有遗留的 watcherId 引用
- 测试覆盖了完整的功能场景
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索可能遗漏的 watcherId 使用 rg "watcherId" packages/file-service/__tests__/Length of output: 47
Script:
#!/bin/bash # 查找所有使用 generateWatcher 的测试文件 rg "generateWatcher" packages/file-service/__tests__/ -l # 显示 file-service-watcher.test.ts 的完整内容以分析测试用例 cat packages/file-service/__tests__/node/file-service-watcher.test.ts # 查找可能的解构模式 rg "\{ *root,.*watcher" packages/file-service/__tests__/Length of output: 14151
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (3)
47-67: 建议添加类级别的文档注释请为
RecursiveFileSystemWatcher类添加详细的文档注释，包括：
- 类的主要职责
- 支持的后端类型（NSFW 和 Parcel）
- 构造函数参数的详细说明
+/** + * 递归文件系统监听器，支持多种后端实现 + * + * @description + * - 支持 NSFW 和 Parcel 两种监听后端 + * - 支持轮询模式的文件监听 + * - 自动处理文件系统事件去重和过滤 + * + * @param excludes 要排除的文件模式列表 + * @param logger 日志服务实例 + * @param backend 监听器后端类型，默认使用 NSFW + */ export class RecursiveFileSystemWatcher extends Disposable implements IWatcher {
82-100: 建议将超时时间设置为可配置项当前监听超时时间被硬编码为 3 秒，这可能在不同环境下不够灵活。建议：
- 将超时时间设置为类的配置项
- 移除 FIXME 注释
+private static readonly DEFAULT_WATCH_TIMEOUT = 3000; + async watchFileChanges(uri: string, options?: WatchOptions) { return new Promise<void>((resolve, rej) => { const timer = setTimeout(() => { rej(`Watch ${uri} Timeout`); - // FIXME：暂时写死3秒 - }, 3000); + }, RecursiveFileSystemWatcher.DEFAULT_WATCH_TIMEOUT);
376-381: 建议将事件数量阈值设置为可配置项当前代码硬编码了 5000 作为最大事件数量阈值，建议：
- 将此值设置为类的静态配置项
- 添加相关文档说明阈值的作用
+ /** + * 单次处理的最大事件数量 + * 超过此数量的事件批次将被忽略，以防止性能问题 + */ + private static readonly MAX_EVENTS_PER_BATCH = 5000; + private async handleNSFWEvents(events: INsfw.ChangeEvent[], realPath: string): Promise<void> { - if (events.length > 5000) { + if (events.length > RecursiveFileSystemWatcher.MAX_EVENTS_PER_BATCH) { return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/file-service/src/node/hosted/recursive/file-service-watcher.ts(10 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: build-windows
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build (macos-latest, 20.x)
🔇 Additional comments (1)
packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (1)
373-373: 建议使用空值合并运算符 ??根据之前的评审意见，建议将
||改为??运算符。- return this.backend === RecursiveWatcherBackend.NSFW || isLinux; + return this.backend === RecursiveWatcherBackend.NSFW ?? isLinux;
Aaaaash
force-pushed
the
feat/add-polling-watcher
branch
from
c7f4640 to
4b9a76c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/file-service/src/node/watcher-process-manager.ts (1)
Line range hint
166-174: 修复 watch 方法参数类型定义错误options 中的 pollingWatch 被标记为必需属性，但实际上应该是可选的。这与接口定义不一致，需要修改：
async watch( uri: UriComponents, - options?: { excludes?: string[]; recursive?: boolean; pollingWatch: boolean }, + options?: { excludes?: string[]; recursive?: boolean; pollingWatch?: boolean }, ): Promise<number>
♻️ Duplicate comments (2)
packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (2)
312-348: 🛠️ Refactor suggestion建议添加临时文件清理机制
轮询监听使用了临时文件存储快照，但没有清理机制。建议：
- 在 disposables 中添加临时文件清理逻辑
- 考虑在进程退出时也进行清理
private async pollingWatch(realPath: string, rawOptions?: WatchOptions | undefined) { const disposables = new DisposableCollection(); const snapshotFile = join(tmpdir(), `watcher-snapshot-${realPath}`); // ... existing code ... disposables.push(pollingWatcher); + disposables.push( + Disposable.create(async () => { + try { + await fs.remove(snapshotFile); + } catch (error) { + this.logger.error('[Recursive] Failed to cleanup snapshot file:', error); + } + }) + ); return disposables; }
378-383: 🛠️ Refactor suggestion建议改进大批量事件的处理方式
当前实现中，超过 5000 个事件的批次会被静默丢弃，这可能导致文件变更未被捕获。建议：
- 添加警告日志
- 考虑分批处理而不是完全丢弃
private async handleNSFWEvents(events: INsfw.ChangeEvent[], realPath: string): Promise<void> { if (events.length > 5000) { + this.logger.warn(`[Recursive] Received large batch of ${events.length} events, consider increasing polling interval`); + // 分批处理事件 + const batches = chunk(events, 1000); + for (const batch of batches) { + await this.handleNSFWEvents(batch, realPath); + } return; }
🧹 Nitpick comments (6)
packages/core-common/src/types/file-watch.ts (3)
28-50: 建议对接口文档进行国际化处理接口文档目前使用中文编写，建议：
- 添加英文文档以支持国际化
- 使用 JSDoc 的
@en和@zh-CN标签来区分不同语言的文档另外，
watchFileChanges方法的返回值从Promise<number>改为Promise<void>是一个破坏性更改，需要确保所有调用方都已经适配。
61-61: 需要为新增的 pollingWatch 选项添加文档注释建议添加 JSDoc 文档说明该选项的用途、默认值和使用场景。例如：
export interface WatchOptions { excludes: string[]; + /** + * 是否使用轮询方式监听文件变化 + * 在某些场景下（如网络文件系统）可能需要启用此选项 + * @default false + */ pollingWatch?: boolean; }
133-136: 需要为 RecursiveWatcherBackend 枚举添加详细文档建议添加以下内容：
- 枚举的整体用途说明
- 每个选项的特点和适用场景
- 性能影响说明
例如：
+/** + * 递归文件监听的后端实现类型 + * @description + * - NSFW: 使用 node-watch 实现，性能较好，但在某些文件系统上可能不稳定 + * - PARCEL: 使用 Parcel 的文件监听实现，稳定性好，但资源消耗较大 + */ export enum RecursiveWatcherBackend { NSFW = 'nsfw', PARCEL = 'parcel', }packages/file-service/src/node/hosted/watcher.host.service.ts (1)
20-25: 建议改进 watcherPlaceHolder 的实现当前实现可以通过以下方式改进：
- 使用 Object.freeze 防止意外修改
- 添加明确的类型定义
- 添加注释说明用途
+type WatcherPlaceholder = { + readonly disposable: { dispose: () => void }; + readonly handlers: never[]; +}; +/** + * 监听器占位对象，用于初始化 WATCHER_HANDLERS Map + */ -const watcherPlaceHolder = { +const watcherPlaceHolder: WatcherPlaceholder = Object.freeze({ disposable: { dispose: () => { }, }, handlers: [], -}; +});packages/file-service/__tests__/node/file-service-watcher.test.ts (1)
12-12: 建议改进睡眠时间常量的命名和文档当前的
sleepTime常量使用魔术数字 1000，建议：-const sleepTime = 1000; +/** + * 文件系统事件传播的等待时间（毫秒） + * 在不同的操作系统和文件系统中可能需要调整此值 + */ +const FILE_SYSTEM_EVENT_PROPAGATION_DELAY = 1000;packages/file-service/src/browser/file-service-client.ts (1)
357-358: 建议添加配置变更影响的文档说明新增的轮询监听配置可能会影响现有集成方的行为。建议：
- 在配置文档中说明
pollingWatcherDirectories的用途和影响- 提供配置迁移指南
Also applies to: 377-377
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
packages/core-browser/src/react-providers/config-provider.tsx(2 hunks)packages/core-common/src/types/file-watch.ts(3 hunks)packages/core-common/src/types/file.ts(1 hunks)packages/file-service/__tests__/browser/file-service-client.test.ts(2 hunks)packages/file-service/__tests__/node/file-service-watcher.test.ts(6 hunks)packages/file-service/__tests__/node/index.test.ts(2 hunks)packages/file-service/src/browser/file-service-client.ts(3 hunks)packages/file-service/src/browser/file-service-provider-client.ts(2 hunks)packages/file-service/src/common/files.ts(2 hunks)packages/file-service/src/common/watcher.ts(2 hunks)packages/file-service/src/node/disk-file-system.provider.ts(3 hunks)packages/file-service/src/node/hosted/recursive/file-service-watcher.ts(10 hunks)packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts(8 hunks)packages/file-service/src/node/hosted/watcher.host.service.ts(4 hunks)packages/file-service/src/node/hosted/watcher.process.ts(1 hunks)packages/file-service/src/node/watcher-process-manager.ts(4 hunks)packages/file-tree-next/src/browser/services/file-tree-api.service.ts(3 hunks)packages/file-tree-next/src/common/index.ts(2 hunks)packages/startup/entry/web/render-app.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/startup/entry/web/render-app.tsx
- packages/file-service/src/node/hosted/watcher.process.ts
- packages/file-tree-next/src/common/index.ts
- packages/file-tree-next/src/browser/services/file-tree-api.service.ts
- packages/file-service/src/common/files.ts
- packages/file-service/src/browser/file-service-provider-client.ts
- packages/file-service/tests/browser/file-service-client.test.ts
- packages/core-common/src/types/file.ts
- packages/file-service/tests/node/index.test.ts
- packages/core-browser/src/react-providers/config-provider.tsx
- packages/file-service/src/common/watcher.ts
- packages/file-service/src/node/disk-file-system.provider.ts
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: build-windows
- GitHub Check: build (macos-latest, 20.x)
🔇 Additional comments (6)
packages/file-service/src/node/watcher-process-manager.ts (1)
117-127: 🛠️ Refactor suggestion建议增强进程创建的错误处理
createWatcherProcess 方法的实现需要增强错误处理机制：
- 检查 watcherHost 是否存在
- 添加进程创建的错误处理
- 完善日志记录
private async createWatcherProcess(clientId: string, ipcHandlerPath: string, backend?: RecursiveWatcherBackend) { + if (!this.watcherHost) { + throw new Error('Watcher host path is not configured'); + } const forkArgs = [/*...*/]; this.logger.log('Watcher process path: ', this.watcherHost); - this.watcherProcess = fork(this.watcherHost, forkArgs, { - silent: true, - }); + try { + this.watcherProcess = fork(this.watcherHost, forkArgs, { + silent: true, + }); + } catch (error) { + this.logger.error('Failed to fork watcher process', error); + throw error; + }Likely invalid or redundant comment.
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (2)
131-138:⚠️ Potential issue修复可能的未定义变量问题
当
exist为false时，变量watchPath可能未被初始化。在这种情况下，随后调用this.start(watchPath)时可能导致异常。建议修改：if (exist) { const stat = await fs.lstat(basePath); if (stat) { watchPath = basePath; } } else { this.logger.warn('[Un-Recursive] This path does not exist. Please try again'); + return; }Likely invalid or redundant comment.
167-173: 🛠️ Refactor suggestion建议增强 unwatchFileChanges 方法的实现
当前实现缺乏错误处理和日志记录，建议改进：
async unwatchFileChanges(uri: string): Promise<void> { + try { const basePath = FileUri.fsPath(uri); if (this.watcherCollections.has(basePath)) { const watcher = this.watcherCollections.get(basePath); - watcher?.close(); + if (watcher) { + watcher.close(); + this.watcherCollections.delete(basePath); + this.logger.log(`[Un-Recursive] Stopped watching: ${basePath}`); + } - this.watcherCollections.delete(basePath); } + } catch (error) { + this.logger.error('[Un-Recursive] Failed to unwatch file changes', { + uri, + error: error.message, + }); + throw error; + } }Likely invalid or redundant comment.
packages/file-service/src/node/hosted/recursive/file-service-watcher.ts (2)
375-375: 使用||运算符是正确的选择在这里使用
||运算符是合适的，因为我们需要在以下任一条件为真时启用 NSFW：
- 后端配置为 NSFW
- 运行平台为 Linux
??运算符用于处理 null/undefined 值，而不适用于这种布尔逻辑组合的场景。
82-100: 建议改进超时处理机制当前实现存在以下问题：
- 超时时间是硬编码的 3 秒
- 错误消息缺少详细信息
- 超时后资源可能未正确清理
建议改进：
+ private static readonly WATCH_TIMEOUT = 3000; + async watchFileChanges(uri: string, options?: WatchOptions) { return new Promise<void>((resolve, rej) => { const timer = setTimeout(() => { - rej(`Watch ${uri} Timeout`); + const error = new Error(`Watch ${uri} timed out after ${RecursiveFileSystemWatcher.WATCH_TIMEOUT}ms`); + this.logger.error('[Recursive] Watch timeout:', error); + rej(error); // FIXME：暂时写死3秒 - }, 3000); + }, RecursiveFileSystemWatcher.WATCH_TIMEOUT); if (options?.excludes) { this.updateWatcherFileExcludes(options.excludes); } - this.doWatchFileChange(uri, options).then(() => { + this.doWatchFileChange(uri, options).then(() => { resolve(void 0); + }).catch((error) => { + this.logger.error('[Recursive] Watch failed:', error); + rej(error); + }).finally(() => { if (timer) { clearTimeout(timer); } }); }); }packages/file-service/src/browser/file-service-client.ts (1)
146-146: 配置传递正确
recursiveWatcherBackend配置从 AppConfig 正确传递给了 provider。
|
/next |
|
🎉 PR Next publish successful! 3.7.1-next-1736846464.0 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/file-service/__tests__/node/file-service-watcher.test.ts (2)
22-26: 建议增加错误处理机制虽然
watchFileChanges的调用已更新，但缺少错误处理机制。建议添加 try-catch 块来处理可能的文件系统错误。- await watcherServer.watchFileChanges(root.path.toString()); + try { + await watcherServer.watchFileChanges(root.path.toString()); + } catch (error) { + watcherServer.dispose(); + throw error; + }
24-24: 建议提取路径转换逻辑多处使用
path.toString()的模式建议提取为工具函数，以提高代码的可维护性和一致性。+ function getWatchPath(uri: URI): string { + return uri.path.toString(); + } - await watcherServer.watchFileChanges(root.path.toString()); + await watcherServer.watchFileChanges(getWatchPath(root)); - await watcherServer.unwatchFileChanges(root.path.toString()); + await watcherServer.unwatchFileChanges(getWatchPath(root));Also applies to: 83-83, 145-145, 151-151, 168-168, 210-210
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/file-service/__tests__/node/file-service-watcher.test.ts(12 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
🔇 Additional comments (3)
packages/file-service/__tests__/node/file-service-watcher.test.ts (3)
10-10: 导入和类型声明更新正确导入语句和类型声明已正确更新为
RecursiveFileSystemWatcher，保持了一致性。Also applies to: 16-16
66-66: 测试断言变得更灵活测试断言从严格相等改为使用
some()方法，这表明文件监视器可能会报告额外的 URI。建议添加测试用例来验证这些额外 URI 的合理性。Also applies to: 250-250, 318-318
✅ Verification successful
测试断言修改是合理的
使用
some()方法进行断言是正确的实现方式，因为：
- 文件系统事件可能包含额外的元数据变更
- 父目录的变更可能会被一并报告
- 事件触发顺序可能会有所不同
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查是否有其他测试文件验证了额外的 URI 报告 rg -l "expectedUris.*some.*includes" "packages/file-service/__tests__"Length of output: 70
Script:
#!/bin/bash # Get the content of the test file to understand the context cat "packages/file-service/__tests__/node/file-service-watcher.test.ts" # Search for other URI-related assertions in the same file rg "expectedUris" "packages/file-service/__tests__/node/file-service-watcher.test.ts" -B 2 -A 2Length of output: 12077
181-211: 需要说明注释掉测试用例的原因大段测试用例被注释掉，但没有说明原因。这可能会降低测试覆盖率并影响重要功能的验证。建议：
- 如果测试用例已过时，应该删除
- 如果测试用例仍然有效，应该修复并启用
- 至少添加注释说明为什么被禁用
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/file-service/__tests__/node/file-service-watcher.test.ts (1)
Line range hint
274-287: 修复测试断言失败CI 管道显示此测试用例失败。考虑到文件系统事件的异步性质，建议：
- 放宽断言条件，使用集合比较而不是严格的数组比较
- 确保测试等待时间足够长，以捕获所有文件系统事件
-expect(Array.from(addUris)).toEqual(expectedAddUris); +expect(new Set(Array.from(addUris))).toEqual(new Set(expectedAddUris));
♻️ Duplicate comments (1)
packages/file-service/src/node/watcher-process-manager.ts (1)
117-127: 🛠️ Refactor suggestion建议增强进程创建的错误处理机制
createWatcherProcess方法仍然需要更好的错误处理机制，特别是在创建子进程时可能出现的异常情况。建议添加以下错误处理：
private async createWatcherProcess(clientId: string, ipcHandlerPath: string, backend?: RecursiveWatcherBackend) { + if (!this.watcherHost) { + throw new Error('未配置 Watcher host 路径'); + } + const forkArgs = [/*...*/]; this.logger.log('Watcher process path: ', this.watcherHost); - this.watcherProcess = fork(this.watcherHost, forkArgs, { - silent: true, - }); + try { + this.watcherProcess = fork(this.watcherHost, forkArgs, { + silent: true, + }); + } catch (error) { + this.logger.error('创建监视进程失败', error); + throw error; + }
🧹 Nitpick comments (2)
packages/file-service/__tests__/node/file-service-watcher.test.ts (2)
66-66: 建议使用更精确的断言方法当前使用
Array.some()进行断言使测试更灵活，但可能会掩盖具体的匹配问题。建议：
- 使用更精确的断言方法，如比较排序后的数组
- 或者添加详细的测试用例说明为什么需要这种灵活的匹配
-expect(Array.from(actualUris).some((val) => expectedUris.includes(val))).toBeTruthy(); +expect(new Set(Array.from(actualUris))).toEqual(new Set(expectedUris));
Line range hint
1-322: 建议改进测试结构为提高测试的可维护性和可靠性，建议以下改进：
- 将
sleepTime常量改为可配置参数，便于调整等待时间- 创建共用的测试工具函数处理重复的文件操作
- 使用
beforeEach统一处理 watcherClient 的创建- 考虑使用更可靠的方式等待文件系统事件，而不是固定的 sleep 时间
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/file-service/__tests__/node/file-service-watcher.test.ts(12 hunks)packages/file-service/src/node/watcher-process-manager.ts(4 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
packages/file-service/__tests__/node/file-service-watcher.test.ts
[error] 287-287: Test assertion failed: Array comparison mismatch in file watcher test
🔇 Additional comments (5)
packages/file-service/src/node/watcher-process-manager.ts (3)
10-14: 导入声明看起来没有问题！新增的
RecursiveWatcherBackend类型导入与相关的文件监视类型放在一起，保持了良好的代码组织。
144-151: 参数传递链路完整，实现正确！
createProcess方法正确地将backend参数传递给了createWatcherProcess，保持了参数传递的一致性。
166-169: 接口设计符合预期！
watch方法的pollingWatch选项已正确设置为可选参数，这与用户反馈保持一致。packages/file-service/__tests__/node/file-service-watcher.test.ts (2)
10-10: 导入和类型声明的更改看起来不错！类名从
FileSystemWatcherServer更改为RecursiveFileSystemWatcher的修改保持了一致性。Also applies to: 16-16
22-26: 请说明构造函数中空数组参数的用途构造函数调用
new RecursiveFileSystemWatcher([], ...)中传入了一个空数组，这个参数的作用不够明确。建议添加注释说明该参数的用途，或者考虑使用更有意义的默认值。
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/file-service/src/node/hosted/watcher.host.service.ts (1)
160-163:⚠️ Potential issue建议增强递归监听器的错误处理机制
当前的错误处理过于简单，需要更完善的错误处理策略。
🧹 Nitpick comments (3)
packages/file-service/src/node/hosted/watcher.host.service.ts (3)
20-25: 建议优化 watcherPlaceHolder 的实现建议将 watcherPlaceHolder 重构为一个类型安全的实现：
-const watcherPlaceHolder = { - disposable: { - dispose: () => {}, - }, - handlers: [], -}; +interface WatcherPlaceHolder { + disposable: IDisposable; + handlers: any[]; +} + +const watcherPlaceHolder: WatcherPlaceHolder = { + disposable: new Disposable(), + handlers: [], +};
55-59: 建议添加构造函数参数的文档注释为了提高代码可维护性，建议为构造函数参数添加详细的文档注释，特别是新增的 backend 参数。
+/** + * @param rpcProtocol - RPC 通信协议实例 + * @param logger - 文件监听日志记录器 + * @param backend - 递归文件监听后端实现 + */ constructor( private rpcProtocol: SumiConnectionMultiplexer, private logger: WatcherProcessLogger, private backend: RecursiveWatcherBackend, )
109-112: 建议增强类型安全性建议为 watch 选项创建专门的接口类型，以提高代码的类型安全性和可维护性。
+interface WatchOptions { + excludes?: string[]; + recursive?: boolean; + pollingWatch?: boolean; +} + -private async doWatch( - uri: Uri, - options?: { excludes?: string[]; recursive?: boolean; pollingWatch?: boolean }, -): Promise<number> +private async doWatch( + uri: Uri, + options?: WatchOptions +): Promise<number> -async $watch( - uri: UriComponents, - options?: { excludes?: string[]; recursive?: boolean; pollingWatch?: boolean }, -): Promise<number> +async $watch( + uri: UriComponents, + options?: WatchOptions +): Promise<number>Also applies to: 173-176
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/file-service/src/node/hosted/watcher.host.service.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: build-windows
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (2)
55-55: 异常处理改进建议捕获
watch出错时只执行watcher.close()。如果需要更高可用性，建议添加重试机制或更详细的错误诊断。
159-164: unwatchFileChanges 的错误处理建议目前只在集合中删除并关闭 watcher，若在
close()过程中发生异常可能缺少处理流程，可考虑补充错误捕获及日志记录。packages/file-service/__tests__/node/file-service-watcher.test.ts (3)
49-49: URI 拼接方式此处保留
root.toString()，与上方root.path.toString()写法不同，但不破坏逻辑，注意统一性。
66-66: 模糊匹配断言
some(...).toBeTruthy()的断言可减少测试容错性，也可以改用.toContain()来加强可读性。
108-108: 新增 watchFileChanges 对目标文件夹允许对不同路径进行额外 watch，可能重复监听注意后续释放资源。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/file-service/__tests__/node/file-node-watcher.test.ts(1 hunks)packages/file-service/__tests__/node/file-service-watcher.test.ts(13 hunks)packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: build-windows
🔇 Additional comments (33)
packages/file-service/__tests__/node/file-node-watcher.test.ts (1)
21-21: 修正 watchFileChanges 的参数形式将
root.path.toString()作为参数能够保证仅传入文件路径，而非完整 URI。请确保后续流程对纯文件路径的处理是预期的。packages/file-service/src/node/hosted/un-recursive/file-service-watcher.ts (10)
5-5: 引入新依赖的合理性新增了
isMacintosh与path的导入，可在后续实现中提供平台判断与路径操作支持，逻辑上没有问题。
7-7: 使用 IWatcher 接口在此引入
IWatcher，说明该类将实现统一的文件监听接口，与代码整体风格保持一致。
12-12: 实现自定义的 UnRecursiveFileSystemWatcher通过实现
IWatcher接口可统一管理文件监听功能，为后续扩展奠定基础。
13-13: 使用 Map 来管理 watcher
watcherCollections使用Map<string, fs.FSWatcher>，有助于清晰地映射路径与监听器实例。设计上可读性较好。
24-24: 构造函数依赖注入通过构造函数注入
logger，可方便记录日志和管理调试信息。实现简洁明了。
33-33: 开始监听时的日志输出在开始监听时输出日志，有助于排查和定位问题，无明显问题。
111-111: 记录日志时机在此处捕获并记录对
fs.watch()的异常，保证问题可追溯，无明显问题。
124-124: 检查路径状态此处通过
fs.lstat获取文件状态，若此时出错（例如权限问题）可能抛出异常，可考虑增加额外的错误处理。
129-129: 路径不存在的警告当路径不存在时，仅输出
warn而未终止流程，可能导致后续行为与期望不符。请确认此逻辑的合理性。
142-144: 避免重复监听若已经有相同路径的监听器存在，则直接返回，避免重复开启。逻辑正常。
packages/file-service/__tests__/node/file-service-watcher.test.ts (22)
10-10: 改用新的 RecursiveFileSystemWatcher 类更新 import 为
RecursiveFileSystemWatcher，和代码主体保持一致，可读性更高。
16-16: 监控列表类型改动将
watcherServerList定义为RecursiveFileSystemWatcher[]，契合新的类名和类型，符合预期。
22-22: 创建新的 RecursiveFileSystemWatcher在测试中直接实例化新的类，参数为空数组与 logger，需确保第一参数符合接口需求。
24-24: watchFileChanges 使用 path.toString()传入
root.path.toString()的方式与新逻辑保持一致。请确认后续与旧有 URI 逻辑无冲突。
26-26: 返回对象不再包括 watcherId此处省略
watcherId，符合新接口IWatcher的实现方式。测试可正常执行。
79-79: 使用 generateWatcher 创建 watcher此处创建和之前逻辑一致，无明显问题。
83-83: unwatchFileChanges 调用对
root.path.toString()调用停止监控，逻辑符合预期。
104-104: 再次调用 generateWatcher与前面测试类似，不存在破坏性变更。
113-113: 再次初始化 watcher与之前用途相同，无突出问题。
145-145: unwatchFileChanges 调用停止对 newFolder 的监听，测试覆盖更全面。
150-150: 断言回调调用此断言正常，确认回调被调用一次即可。
151-151: 再次取消监听无冗余操作，可避免后续对同路径重复监听。
162-162: 覆盖 NSFW 属性测试中用
isEnableNSFW = () => false以模拟不同后端，便于验证功能。没问题。
168-168: 调用 watchFileChanges同样使用
root.path.toString()，风格与之前保持一致。
172-172: 追踪多个 Watcher在同一文件中定义多个测试场景共用
watcherServerList，逻辑清晰。
207-207: 对比预期结果时使用 .some不严格要求完整匹配，仅需验证目标是否在集合中，这种断言满足测试需求。
210-210: 在测试结束时取消监听可避免后续用例的干扰，逻辑合理。
233-237: 批量新增预期的 URI此处将多条 URI 统一放入数组，使用
some方法检查，断言策略较为灵活。
250-250: 断言添加事件再次使用
some方式验证添加事件，与前文一致，符合用例要求。
274-278: 多条路径合并检查测试移动文件时的新增事件，统一验证
[root, root.resolve('for_rename_1'), ...]，无明显问题。
287-287: 使用 some 时缺少断言与先前评论相同，此处缺少
.toBeTruthy()或同等匹配器。请补充，以免测试失效。
318-318: 使用 some 时缺少断言与之前相同，需要为
expect(...).some(...)补充匹配器.toBeTruthy()以防测试结果不生效。
Types
Background or solution
Changelog
Summary by CodeRabbit
发布说明
新功能
改进
弃用
unRecursiveDirectories属性为已弃用pollingWatcherDirectories替代旧的监听方法技术调整
Event类型的支持，以便在文件树 API 中发射事件