feat: triggered rendering image after the image content changed #4662
+82
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Walkthrough在文件服务中新增图像专用变更事件通道，并在预览组件中订阅该事件；预览组件维护 Changes
Sequence Diagram(s)sequenceDiagram
participant FS as 文件系统
participant FSC as FileServiceClient
participant PV as PreviewView
participant B as 浏览器缓存
FS->>FSC: 通知文件变更列表
activate FSC
FSC->>FSC: 过滤 EXT_LIST_IMAGE 得到 imageChanges
alt imageChanges 非空
FSC->>PV: 触发 onImageFilesChanged(imageChanges)
end
deactivate FSC
activate PV
PV->>PV: useResourceChange 匹配当前资源 URI
PV->>PV: fileChanged += 1
PV->>PV: 构建新 src（原始 src + ?_t=fileChanged）
PV->>B: 请求新 src（强制刷新）
B-->>PV: 返回最新图像
deactivate PV
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分钟 需重点复核：
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2024-10-12T07:43:08.790ZApplied to files:
🧬 Code graph analysis (2)packages/file-service/src/browser/file-service-client.ts (5)
packages/file-service/src/common/file-service-client.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/file-service/src/browser/file-service-client.ts (1)
361-369: 考虑性能优化：减少 URI 对象创建在高频文件变更场景下，为每个变更事件都创建
URI对象可能影响性能。考虑以下优化方案：优化方案 1 - 使用字符串操作预筛选：
// 过滤图片文件变化并触发专门的事件 const imageChanges = changes.filter((change) => { + // 快速字符串检查，避免创建 URI 对象 + const uriStr = change.uri; + const lastDotIndex = uriStr.lastIndexOf('.'); + if (lastDotIndex === -1) return false; + + const ext = uriStr.substring(lastDotIndex + 1).toLowerCase(); + if (!EXT_LIST_IMAGE.has(ext)) return false; + + // 只有匹配时才创建 URI 对象进行精确验证 const uri = new URI(change.uri); - const ext = uri.path.ext?.toLowerCase().replace('.', ''); - return ext && EXT_LIST_IMAGE.has(ext); + return uri.path.ext?.toLowerCase().replace('.', '') === ext; });优化方案 2 - 批量处理：
const imageChanges = changes.filter((change) => { const uri = new URI(change.uri); const ext = uri.path.ext?.toLowerCase().replace('.', ''); return ext && EXT_LIST_IMAGE.has(ext); }); if (imageChanges.length > 0) { + // 考虑使用 debounce 或节流来避免频繁触发 this._onImageFilesChanged.fire(imageChanges); }packages/file-service/src/common/file-service-client.ts (1)
35-35: 接口扩展正确新增的
onImageFilesChanged事件接口定义正确，类型与实现保持一致。建议添加 JSDoc 注释说明事件的用途和触发时机，以提高可维护性。可以添加文档注释：
onFilesChanged: Event<FileChangeEvent>; + /** + * Event that fires when image files are changed. + * This event only includes changes to files with image extensions. + */ onImageFilesChanged: Event<FileChangeEvent>;packages/file-scheme/src/browser/preview.view.tsx (1)
28-28: 优化 effect 依赖项
fileServiceClient是通过 DI 注入的稳定实例，通常不需要作为依赖项。可以考虑移除以避免不必要的重新订阅。- }, [resource.uri, fileServiceClient]); + }, [resource.uri]);或者使用
useRef保存fileServiceClient以确保稳定引用。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/file-scheme/src/browser/preview.view.tsx(2 hunks)packages/file-service/src/browser/file-service-client.ts(3 hunks)packages/file-service/src/common/file-service-client.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/file-scheme/src/browser/preview.view.tsx (3)
packages/editor/src/common/resource.ts (1)
IResource(114-132)packages/core-browser/src/react-hooks/injectable-hooks.tsx (1)
useInjectable(16-39)packages/file-service/src/common/file-service-client.ts (2)
IFileServiceClient(27-27)IFileServiceClient(29-118)
packages/file-service/src/common/file-service-client.ts (1)
packages/core-common/src/types/file-watch.ts (1)
FileChangeEvent(91-91)
packages/file-service/src/browser/file-service-client.ts (5)
packages/utils/src/event.ts (2)
Emitter(590-775)Event(24-24)packages/core-common/src/types/file-watch.ts (1)
FileChangeEvent(91-91)packages/core-browser/src/fs/index.ts (1)
FilesChangeEvent(3-3)packages/utils/src/uri.ts (2)
URI(6-6)URI(17-311)packages/file-service/src/common/file-ext.ts (1)
EXT_LIST_IMAGE(3-27)
🔇 Additional comments (1)
packages/file-service/src/browser/file-service-client.ts (1)
361-365: 代码扩展名提取逻辑验证完毕，无需改动基于代码库验证，
uri.path.ext的行为已确认：
- 返回带点的扩展名（如
.png），或空字符串（文件无扩展名）- 多个点的情况下返回最后一个点之后的部分（如
file.backup.png返回.png）- 第 363-364 行的逻辑正确处理所有边界情况：
- 无扩展名时，
ext为空字符串（falsy），过滤返回 false ✓replace('.', '')仅移除首个点，结果准确 ✓无附加问题发现。
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/file-service/__tests__/browser/file-service-client.test.ts (1)
168-190: 建议改进异步测试的错误处理当前测试使用了
inner()异步函数但没有等待其完成或处理错误。如果fireFilesChange抛出异常，可能导致未捕获的 Promise 拒绝或测试挂起。建议改进错误处理：
it('onImageFilesChanged event', (done) => { async function inner() { fileServiceClient.onImageFilesChanged((event) => { // 期望只收到一个image的event expect(event.every((v) => v.uri.includes('a.jpg'))).toBeTruthy(); done(); }); // 触发两个change await fileServiceClient.fireFilesChange({ changes: [ { uri: 'a.jpg', type: FileChangeType.UPDATED, }, { uri: 'a.log', type: FileChangeType.UPDATED, }, ], }); } - inner(); + inner().catch(done); });packages/file-scheme/src/browser/preview.view.tsx (1)
61-76: 移除未使用的 Disposable 对象Effect 中创建的
Disposable对象没有被实际使用，只是创建后立即销毁，没有实际用途。建议简化代码：
React.useEffect(() => { - const disposer = new Disposable(); if (imgRef.current) { try { const url = new URL(src); url.searchParams.set('_t', fileChanged.toString()); imgRef.current.src = url.href; } catch (error) { // 如果 src 不是有效的 URL，直接使用原始 src imgRef.current.src = src; } } - return () => { - disposer.dispose(); - }; }, [src, fileChanged]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/file-scheme/src/browser/preview.view.tsx(2 hunks)packages/file-service/__tests__/browser/file-service-client.test.ts(2 hunks)packages/file-service/src/browser/file-service-client.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/file-service/src/browser/file-service-client.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-30T12:55:07.459Z
Learnt from: weiwen111
Repo: opensumi/core PR: 4139
File: packages/core-browser/src/window/window.service.ts:0-0
Timestamp: 2024-10-30T12:55:07.459Z
Learning: 在 `packages/core-browser/src/window/window.service.ts` 中，`workspace` 可能是以 `http` 开头的 URI，此时 `fsPath` 方法不可用。需要根据 `workspaceUri.scheme` 来判断并分别处理不同协议的 URI。
Applied to files:
packages/file-service/__tests__/browser/file-service-client.test.tspackages/file-scheme/src/browser/preview.view.tsx
📚 Learning: 2024-10-30T12:51:05.546Z
Learnt from: weiwen111
Repo: opensumi/core PR: 4139
File: packages/core-browser/src/window/window.service.ts:0-0
Timestamp: 2024-10-30T12:51:05.546Z
Learning: 在 Web 环境中，使用 `workspaceUri.path.toString()` 获取路径可能仍然包含 `file://` 前缀，应该使用 `workspaceUri.fsPath` 来获取不含 `file://` 前缀的路径。
Applied to files:
packages/file-scheme/src/browser/preview.view.tsx
🧬 Code graph analysis (1)
packages/file-scheme/src/browser/preview.view.tsx (3)
packages/editor/src/common/resource.ts (1)
IResource(114-132)packages/core-browser/src/react-hooks/injectable-hooks.tsx (1)
useInjectable(16-39)packages/file-service/src/common/file-service-client.ts (2)
IFileServiceClient(27-27)IFileServiceClient(29-118)
🔇 Additional comments (1)
packages/file-scheme/src/browser/preview.view.tsx (1)
14-34: 实现正确，已解决之前的 URI 比较问题Hook 的实现逻辑正确：
- 正确订阅图片文件变更事件
- 使用
URI.isEqual进行可靠的 URI 比较（已解决之前的问题）- 依赖项设置合理，确保资源变更时重新订阅
Contributor
Author
|
/next |
Contributor
|
🎉 PR Next publish successful! 3.9.1-next-1762421442.0 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4662 +/- ##
=======================================
Coverage 52.63% 52.64%
=======================================
Files 1686 1686
Lines 104458 104468 +10
Branches 22707 22701 -6
=======================================
+ Hits 54986 55000 +14
+ Misses 41103 41099 -4
Partials 8369 8369
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Types
Background or solution
Changelog
Summary by CodeRabbit
发布说明