fix(): fix accessing images of v2 micro-apps in v3 container, and vice versa #1481
Conversation
This file contains 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
总览变更概述此次变更主要涉及图标处理逻辑的增强，特别是在处理微应用版本相关的图标URL时。在 变更
详细变更功能变更
测试变更
类型导出变更
这些变更旨在提高图标加载和错误处理的健壮性，特别是在处理不同版本微应用的图标时，同时增强了代码的类型安全性。 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 (
|
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1481 +/- ##
==========================================
+ Coverage 90.38% 90.47% +0.08%
==========================================
Files 124 124
Lines 2757 2762 +5
Branches 377 379 +2
==========================================
+ Hits 2492 2499 +7
+ Misses 171 169 -2
Partials 94 94
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
bricks/icons/src/shared/SvgCache.ts (2)
21-23: 建议改进参数命名布尔参数
isFix的命名可以更具描述性，建议修改为更明确的名称，如isVersionRetry或isUrlVersionFixed，以便更好地表达其用途。async function resolveIcon( url: string, options?: ResolveIconOptions, - isFix?: boolean + isVersionRetry?: boolean ): Promise<SVGResult> {
28-42: 建议增强错误处理机制当前实现存在以下可以改进的地方：
- 建议添加重试次数限制，防止潜在的无限递归
- 考虑添加日志记录，便于问题追踪
- 可以优化错误信息的返回，提供更多上下文信息
if (!fileData.ok) { // Fix accessing images of v2 micro-apps in v3 container, and vice versa. if ( !isFix && fileData.status === 404 && REGEX_MICRO_APPS_WITH_VERSION.test(url) ) { + console.debug(`Attempting to fix version mismatch for URL: ${url}`); const fixedUrl = url.replace( REGEX_MICRO_APPS_WITH_VERSION, (_, v) => `/micro-apps/v${v === "2" ? "3" : "2"}/` ); return resolveIcon(fixedUrl, options, true); } - return fileData.status === 410 ? CACHEABLE_ERROR : RETRYABLE_ERROR; + const error = fileData.status === 410 ? CACHEABLE_ERROR : RETRYABLE_ERROR; + console.debug(`Failed to fetch icon: ${url}, status: ${fileData.status}`); + return error; }bricks/icons/src/svg-icon/index.spec.ts (3)
Line range hint
5-32: 建议扩展测试用例覆盖范围当前的模拟实现很好，但建议添加以下测试场景：
- 网络超时情况
- 畸形SVG响应
- 多次重试场景
- 不同版本号的边界情况
(global as any).fetch = jest.fn((url) => url?.includes("/micro-apps/v3/") ? Promise.resolve({ ok: false, status: 404, }) + : url?.includes("/timeout/") + ? Promise.reject(new Error("Network timeout")) + : url?.includes("/malformed/") + ? Promise.resolve({ + ok: true, + text: () => Promise.resolve("<invalid>svg</invalid>") + }) : url?.includes("/fail/")
119-129: 建议增强URL修复测试用例当前测试用例可以通过以下方式加强：
- 验证切换后的URL是否正确
- 检查SVG内容是否符合预期
- 添加错误事件处理的测试
- 验证版本切换的性能表现
test("fix url", async () => { const element = document.createElement("eo-svg-icon") as SvgIcon; element.imgSrc = "/sa-static/micro-apps/v3/app/test.svg"; + const onIconFound = jest.fn(); + element.addEventListener("icon.found", (e) => { + onIconFound((e as CustomEvent).detail); + }); + document.body.appendChild(element); await (global as any).flushPromises(); expect(element.shadowRoot?.querySelector("svg")).toBeTruthy(); + expect(onIconFound).toHaveBeenCalledWith(true); + expect(fetch).toHaveBeenCalledWith(expect.stringContaining("/micro-apps/v2/")); document.body.removeChild(element); });
131-146: 建议添加更多失败场景测试建议增加以下失败场景的测试：
- 连续失败的情况
- 不同HTTP状态码的处理
- 网络错误的处理
- 资源加载超时的处理
+ test("consecutive failures", async () => { + const element = document.createElement("eo-svg-icon") as SvgIcon; + element.imgSrc = "/fail/test.svg"; + + document.body.appendChild(element); + await (global as any).flushPromises(); + + // Try changing the URL and verify it still fails + element.imgSrc = "/fail/another-test.svg"; + await (global as any).flushPromises(); + + expect(element.shadowRoot?.childNodes.length).toBe(1); + document.body.removeChild(element); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bricks/icons/src/shared/SvgCache.ts(1 hunks)bricks/icons/src/svg-icon/index.spec.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Size check (20.x)
- GitHub Check: Build docs (20.x)
🔇 Additional comments (1)
bricks/icons/src/shared/SvgCache.ts (1)
17-17: 正则表达式实现合理！正则表达式能够准确匹配并捕获微应用版本号（v2或v3）。
|
🚀 Deployed on https://docs-preview-1481--next-bricks.netlify.app |
|
📐🤏 Size check result (579c3c3...f1ae2bf): Load all bricks together
Critical changes: None. See full changes
Load bricks by each packageCritical changes: None. See full changes
Load by each brickCritical changes: None. See full changes
|
weareoutman
force-pushed
the
steve/fix-icon-url
branch
from
69f1ee9 to
d5a0343
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
bricks/icons/src/shared/SvgCache.ts (1)
28-42: 建议添加错误日志记录！当前实现在版本切换时能够正确处理404错误，但建议添加日志记录以便于问题排查。
建议添加如下修改：
if (!fileData.ok) { + console.warn(`Failed to fetch icon from ${url}: ${fileData.status}`); // Fix accessing images of v2 micro-apps in v3 container, and vice versa. if ( !isFix &&bricks/icons/src/svg-icon/index.spec.ts (1)
119-129: 建议增强测试用例的断言！当前测试用例验证了版本切换功能，但建议添加更多断言以确保行为正确性。
建议添加如下断言：
expect(element.shadowRoot?.querySelector("svg")).toBeTruthy(); + // 验证请求次数 + expect(fetch).toHaveBeenCalledTimes(2); + // 验证请求的URL + expect(fetch).toHaveBeenCalledWith("/sa-static/micro-apps/v2/app/test.svg");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bricks/icons/src/shared/SvgCache.ts(1 hunks)bricks/icons/src/svg-icon/index.spec.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: CodeQL
- GitHub Check: Build bricks (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
bricks/icons/src/shared/SvgCache.ts (2)
17-17: 正则表达式实现合理！正则表达式能够准确匹配微应用的版本号（v2或v3），并通过捕获组方便后续的URL转换。
21-23: 函数签名更新合理！新增的可选参数
isFix用于防止无限递归，同时保持了向后兼容性。bricks/icons/src/svg-icon/index.spec.ts (2)
Line range hint
5-32: 测试用例模拟实现完善！fetch模拟实现合理地处理了不同的URL模式和状态码，包括：
- v3微应用的404错误
- 失败URL的410错误
- 成功请求的SVG响应
131-146: 错误处理测试用例设计合理！测试用例完整验证了：
- 失败URL的错误处理
- 事件派发机制
- DOM状态变化
weareoutman
force-pushed
the
steve/fix-icon-url
branch
from
d5a0343 to
cc5357e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bricks/icons/src/shared/SvgCache.ts (1)
28-42: 版本切换逻辑实现完善修复了 v2/v3 容器间图片访问的问题，实现要点：
- 仅对 404 错误进行处理
- 使用
isFix标志防止递归- 版本切换逻辑清晰（v2 ↔ v3）
建议添加日志记录以便追踪版本切换情况。
if ( !isFix && fileData.status === 404 && REGEX_MICRO_APPS_WITH_VERSION.test(url) ) { const fixedUrl = url.replace( REGEX_MICRO_APPS_WITH_VERSION, (_, v) => `/micro-apps/v${v === "2" ? "3" : "2"}/` ); + console.debug(`[SvgCache] Switching version for URL: ${url} -> ${fixedUrl}`); return resolveIcon(fixedUrl, options, true); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
bricks/basic/package.json(1 hunks)bricks/basic/src/link/index.tsx(1 hunks)bricks/basic/src/popover/index.tsx(1 hunks)bricks/icons/src/shared/SvgCache.ts(1 hunks)bricks/icons/src/svg-icon/index.spec.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- bricks/basic/src/popover/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- bricks/icons/src/svg-icon/index.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build bricks (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
bricks/basic/src/link/index.tsx (1)
23-23: 类型导入导出的改进！将
ExtendedLocationDescriptor和Target的导入导出更改为类型导入导出，提高了类型安全性和代码清晰度。Also applies to: 27-28
bricks/basic/package.json (1)
43-43: 请验证新依赖的必要性和版本兼容性新增了
@next-core/easyops-runtime依赖，版本为^0.13.3。请运行以下脚本验证依赖版本的安全性和必要性：
bricks/icons/src/shared/SvgCache.ts (2)
17-18: 正则表达式定义清晰明确
REGEX_MICRO_APPS_WITH_VERSION正则表达式用于匹配和识别 v2/v3 微应用的 URL，实现方式简洁有效。
21-23: 函数参数扩展合理为
resolveIcon函数添加isFix参数，用于防止无限递归，这是一个很好的安全措施。
qiaofengxi
approved these changes
Jan 20, 2025
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.
依赖检查
组件之间的依赖声明，是微服务组件架构下的重要信息，请确保其正确性。
请勾选以下两组选项其中之一：
或者：
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG，请检查工作内容与提交信息是否相符，并在以下每组选项中都依次确认。
破坏性变更：
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性：
feat作为提交类型。问题修复：
fix作为提交类型。杂项工作：
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动，例如修改注释、测试用例、开发文档等：
chore,docs,test等作为提交类型。Summary by CodeRabbit
新功能
Bug 修复
测试
依赖更新
@next-core/easyops-runtime，版本为^0.13.3类型导出