-
Notifications
You must be signed in to change notification settings - Fork 310
修正 FileSystemObserver 未能持续监听问题 #1160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
此 PR 修复了 FileSystemObserver 在某些编辑器操作后无法继续监听文件变更的问题(issue #1159)。主要针对编辑器先删除再保存、或用新文件覆盖原文件的场景(如 GitHub Desktop 切换分支时的行为)。
变更内容:
- 添加了
onFileError回调机制,在文件访问失败时通知调用方 - 重构了文件变更回调逻辑,移除了
type !== "modified"的限制,处理更多类型的文件系统事件 - 增加了对
"errored"类型事件的处理,在文件恢复可访问后自动重新观察
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/pkg/utils/file-tracker.ts | 核心修复:添加 getHandleRecord 辅助函数、重构 callback 函数以处理更多文件系统事件类型、添加 onFileError 回调、实现 "errored" 事件的自动重新观察机制 |
| src/pages/install/App.tsx | 集成修复:实现 onWatchFileError 函数,在文件错误时禁用监听功能 |
src/pkg/utils/file-tracker.ts
Outdated
| } catch (e) { | ||
| // 档案改名或删掉时,或会被此捕捉(预期报错) | ||
| console.warn(e); | ||
| ftInfo.onFileError(); |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在 getHandleRecord 函数中,当捕捉到错误后调用 ftInfo.onFileError(),然后继续循环。这可能导致同一个文件被多次调用 onFileError()(如果 handleRecords 中有多个匹配的记录)。建议在调用 ftInfo.onFileError() 后立即 return,或者添加逻辑防止重复调用。
| ftInfo.onFileError(); | |
| ftInfo.onFileError(); | |
| // 触发错误回调后立即返回,避免同一文件在一次处理流程中多次触发 onFileError | |
| return { ftInfo: null, file: null }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
应该是找第一个有效的,没有会返回 null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| try { | ||
| for (const record of records) { | ||
| const { root, type } = record; | ||
| if (!(root instanceof FileSystemFileHandle)) continue; |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
移除了 type !== "modified" 的检查,现在任何类型的变更事件都会被处理。这是一个重要的行为变更。建议在代码注释中说明为什么要处理所有类型的事件(包括 "appeared"、"disappeared"、"errored"、"moved" 等),特别是明确说明如何处理 "disappeared" 和 "moved" 事件,以符合 PR 描述中"对于改名或档案移动等,则不会继续监听"的设计意图。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR有寫啦
src/pkg/utils/file-tracker.ts
Outdated
| if (type === "errored") { | ||
| observer.observe(root); | ||
| } |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
当 type === "errored" 且 file.size > 30 时会重新调用 observer.observe(root)。但这里存在一个问题:observe() 可能会抛出异常(例如文件权限问题、文件已被删除等),但这个调用没有进行错误处理。建议添加 try-catch 来捕获 observe() 可能抛出的异常,避免整个回调函数因未处理的异常而中断。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
有try catch 啦
src/pkg/utils/file-tracker.ts
Outdated
| const isSame = await root.isSameEntry(fileHandle); | ||
| if (!isSame) continue; | ||
| const file = await root.getFile(); | ||
| if (file && file.lastModified > 0) { |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在第 18 行检查 file.lastModified > 0 似乎不必要。根据 File API 规范,lastModified 返回文件最后修改时间的毫秒时间戳,对于有效的文件应该总是 >= 0。建议移除这个检查,或者说明为什么需要排除 lastModified === 0 的情况。如果是为了检查文件有效性,建议使用更明确的检查方式。
| if (file && file.lastModified > 0) { | |
| if (file) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
小心為上
| const getHandleRecord = async (root: FileSystemFileHandle, observer: FileSystemObserverInstance) => { | ||
| for (const [fileHandle, ftInfo, fileObserver] of handleRecords) { | ||
| if (fileObserver !== observer) continue; | ||
| try { | ||
| const isSame = await root.isSameEntry(fileHandle); | ||
| if (!isSame) continue; | ||
| const file = await root.getFile(); | ||
| if (file && file.lastModified > 0) { | ||
| return { ftInfo, file }; | ||
| } | ||
| } catch (e) { | ||
| // 档案改名或删掉时,或会被此捕捉(预期报错) | ||
| console.warn(e); | ||
| ftInfo.onFileError(); | ||
| } | ||
| } | ||
| return { ftInfo: null, file: null }; | ||
| }; | ||
|
|
||
| const callback = async (records: FileSystemChangeRecord[], observer: FileSystemObserverInstance) => { | ||
| for (const record of records) { | ||
| const { root, type } = record; | ||
| if (!(root instanceof FileSystemFileHandle) || type !== "modified") continue; | ||
| for (const [fileHandle, ftInfo, fileObserver] of handleRecords) { | ||
| if (fileObserver !== observer) continue; | ||
| try { | ||
| const isSame = await root.isSameEntry(fileHandle); | ||
| if (!isSame) continue; | ||
| // 调用安装 | ||
| const file = await root.getFile(); | ||
| // 避免重复更新 | ||
| try { | ||
| for (const record of records) { | ||
| const { root, type } = record; | ||
| if (!(root instanceof FileSystemFileHandle)) continue; | ||
| // 只要 FileSystemObserver 侦测到档案改变,就试一下找记录和读档 | ||
| const { ftInfo, file } = await getHandleRecord(root, observer); | ||
| // 如没有记录或读档失败则忽略 | ||
| if (!ftInfo || !file) continue; | ||
| // 读档成功且有效 ( 文字档考虑 ==UserScript== 等东西,至少应有 30 bytes ) | ||
| if (file.size > 30) { | ||
| // 如成功读档但显示为失败,则重新 observe | ||
| if (type === "errored") { | ||
| observer.observe(root); | ||
| } | ||
| // 以 lastModified 判断避免重复更新 | ||
| if (ftInfo.lastModified === file.lastModified) continue; | ||
| ftInfo.lastModified = file.lastModified; | ||
| const code = await file.text(); | ||
| if (code && typeof code === "string") { | ||
| ftInfo.setCode(code, false); | ||
| } | ||
| } catch (e) { | ||
| console.warn(e); | ||
| } | ||
| } | ||
| } catch (e) { | ||
| // 捕捉其他非遇期错误 | ||
| console.warn(e); | ||
| } | ||
| }; |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file-tracker.ts 缺少测试覆盖。鉴于此文件中的其他工具函数(如 async_queue.ts、crypto.ts、match.ts 等)都有对应的测试文件,建议为 file-tracker.ts 添加测试,特别是针对新增的错误处理逻辑和 "errored" 类型的重新观察机制,这些是关键的修复点,应该有测试来验证其正确性。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
硬做一個假的模擬行為來試?別玩了
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
概述 Descriptions
主要是因为有些编辑器的操作是先删再储
又或者,把档案取代等方式 (例如代码写好后另存新档直接覆盖此档)(GitHub desktop的branch切換同属此行為)
这个PR的修改能对应不同情况。
但,对于改名或档案移动等,则不会继续监听(故意) -> 重置 Watch File
close #1159
变更内容 Changes
截图 Screenshots