feat: add ref forwarding to internal img element. Closes #372 #379
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改对 Changes
Sequence Diagram(s)sequenceDiagram
participant P as 父组件
participant I as ImageInternal 组件
participant H as handleRef 函数
participant IMG as HTML Image 元素
P->>I: 渲染组件并传入 ref
I->>H: 调用 handleRef 传入 props 与 ref
H->>IMG: 分配引用给图片元素
IMG-->>H: 返回图片元素
H-->>I: 更新内部引用
I-->>P: 完成 ref 转发
Assessment against linked issues
Possibly related PRs
Poem
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
tests/ref.test.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
🪧 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: 0
🧹 Nitpick comments (3)
tests/ref.test.tsx (2)
43-61: 验证了引用对 img 元素属性的访问能力，但可以考虑扩展测试。测试成功验证了通过 ref 可以访问 img 元素的宽高属性。注释中提到了可能测试调用 img 元素的方法，但没有实际实现。考虑添加对 img 方法的测试，比如
getBoundingClientRect()，这些方法在 jsdom 环境中可用。- // 可以测试调用 img 元素的方法 - // 注意：某些方法可能在 jsdom 环境中不可用，根据实际情况调整 + // 测试 img 元素的方法 + expect(typeof ref.current?.getBoundingClientRect).toBe('function'); + // 模拟 complete 属性 + Object.defineProperty(ref.current as HTMLImageElement, 'complete', { value: true }); + expect(ref.current?.complete).toBe(true);
1-4: 导入语句组织合理。导入了必要的测试工具和组件，没有多余的导入。注意到导入了
fireEvent，但在测试中没有使用。如果未来不需要，可以考虑移除这个未使用的导入。-import { act, fireEvent, render } from '@testing-library/react'; +import { act, render } from '@testing-library/react';src/Image.tsx (1)
183-198: 引用处理函数实现完整，但有微小的格式问题。
handleRef函数正确处理了内部引用和外部引用的结合，支持对象引用和回调引用两种类型。代码逻辑清晰，但有一行多余的空白行（第187行），应保持代码格式一致性。const handleRef = (img: HTMLImageElement | null) => { if (img) { getImgRef(img); - // 处理外部传入的 ref if (ref) { if (typeof ref === 'function') { ref(img); } else { ref.current = img; } } } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Image.tsx(5 hunks)tests/ref.test.tsx(1 hunks)
🔇 Additional comments (8)
tests/ref.test.tsx (3)
5-22: 测试用例设计合理，覆盖了对象引用的转发场景。测试用例很好地验证了对象类型 ref 能够正确转发到内部 img 元素，并且能够访问 img 元素的属性（如 tagName 和 alt）。测试结构清晰，断言完整。
24-41: 回调引用类型的测试实现得当。这个测试用例很好地验证了回调类型的 ref 能被正确调用并指向正确的 img 元素。这是一个必要的测试，因为 React 支持两种引用类型，测试覆盖了这两种情况。
63-88: 重渲染稳定性测试设计合理。这个测试用例很好地验证了在组件重新渲染后，ref 引用的仍然是同一个 img 元素，并且属性更新正确反映在 DOM 元素上。这对于确保 ref 的稳定性至关重要。
src/Image.tsx (5)
6-6: 导入语句更新合理，明确分离了具名导入。从 React 中导入了必要的
forwardRef函数，并保持了之前的useContext,useMemo,useState导入。这种显式导入方式比使用通配符导入更好，有助于代码可读性和树摇。
72-74: 接口更新符合 TypeScript 类型体系。将
CompoundedComponent接口更新为扩展React.ForwardRefExoticComponent，并添加了React.RefAttributes<HTMLImageElement>是正确的做法。这确保了类型安全和 IDE 提示支持。
76-76: 组件重构为 forwardRef 实现得当。使用
forwardRef改造组件是正确的实现方式，这允许父组件直接访问内部的 img 元素。定义清晰地包含了泛型参数，指定了正确的引用类型和属性类型。
225-226: img 元素引用属性更新正确。将
ref属性从getImgRef更新为handleRef是正确的改动，这使得外部引用能够正确转发到内部 img 元素。
276-276: 类型断言使用恰当。使用
as CompoundedComponent<ImageProps>类型断言是必要的，因为forwardRef返回的类型需要与之前定义的CompoundedComponent接口匹配。这确保了组件类型的一致性和PreviewGroup属性的可访问性。
liamli-0822
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #379 +/- ##
=======================================
Coverage 99.40% 99.41%
=======================================
Files 17 17
Lines 506 513 +7
Branches 148 149 +1
=======================================
+ Hits 503 510 +7
Misses 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
想了下 ref 指向 image 元素也许不是最好的方式，未来可能会加 ref.next ref.rotate ref.focus 这些实例方法。 |
optimized the logic of the handleRef function Co-authored-by: afc163 <[email protected]>
那这个先close？ |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Image.tsx (1)
183-197: 引用处理实现得很好处理内部和外部引用的函数实现得很好，同时支持回调引用和对象引用。建议可以考虑将中文注释翻译为英文，以保持代码国际化一致性。
- // 处理外部传入的 ref + // Handle externally passed ref
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Image.tsx(5 hunks)
🔇 Additional comments (5)
src/Image.tsx (5)
6-6: 非常好的导入 forwardRef 函数！从 React 中导入 forwardRef 函数以支持引用转发功能是一个很好的做法。
72-72: 接口类型更新正确适配了 forwardRef 的使用将 CompoundedComponent 接口从 React.FC 更新为 React.ForwardRefExoticComponent 是正确的做法，这样能够正确地支持 HTMLImageElement 的引用属性。
76-76: 组件定义正确使用了 forwardRef API使用 forwardRef 包装组件使其能够接收和处理引用参数，实现是正确的。
225-225: 正确更新了图像元素的引用属性将 img 元素的 ref 属性从 getImgRef 更新为 handleRef 确保了内部功能和转发的引用都能正常工作。
276-276: 组件类型断言正确使用类型断言将组件转换为 CompoundedComponent 类型是正确的，这样能保持复合组件模式与 PreviewGroup 的兼容性。
|
可以先做个 ref.nativeElement 指向 image 元素。 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/ref.test.tsx (1)
45-63: 测试通过 nativeElement 访问属性的注释需更新测试验证了通过
nativeElement访问 img 元素的宽度和高度属性，但有一个未实现的注释提到可以测试方法调用。建议删除或实现该注释以避免混淆。- // 可以测试调用 img 元素的方法 - // 注意：某些方法可能在 jsdom 环境中不可用，根据实际情况调整
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Image.tsx(6 hunks)tests/ref.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/ref.test.tsx (1)
src/Image.tsx (1)
ImageRef(75-77)
🔇 Additional comments (13)
tests/ref.test.tsx (5)
1-4: 导入和设置文件结构良好测试文件导入了必要的测试工具和组件，包括正确导入了有类型定义的
ImageRef。这为测试 ref 转发功能提供了良好的基础。
5-23: 对象引用测试覆盖全面这个测试用例很好地验证了对象引用的功能，确保
ref.current.nativeElement正确指向内部的 img 元素。测试断言覆盖了多个方面，包括非空检查、DOM 元素匹配和属性访问。
25-43: 回调引用测试完整有效回调引用的测试实现正确，并确认了与对象引用相同的功能。测试用例清晰地展示了如何使用回调引用模式，并验证了对内部 img 元素的访问。
65-90: 再渲染时引用稳定性测试设计良好此测试确保在组件重新渲染后引用保持稳定，同时属性可以正确更新。测试设计考虑周全，验证了重要的组件生命周期行为。
92-109: API 边界测试确保引用接口安全性这个测试验证了 ref 的正确封装，确保不能直接访问 img 元素的原生属性，而必须通过
nativeElement属性。使用@ts-ignore来测试运行时行为是一个合适的方法。src/Image.tsx (8)
4-4: 增加了必要的 React Hooks 导入正确导入了
forwardRef,useImperativeHandle和useRefhooks，这些是实现引用转发所必需的。
74-77: 清晰定义了 ImageRef 接口接口定义简洁明了，只包含一个
nativeElement属性。这符合 afc163 在评论中建议的实现方法，提供了一个通过ref.nativeElement访问内部 img 元素的方式。
79-79: 正确更新了 CompoundedComponent 接口将接口从
React.FC更新为React.ForwardRefExoticComponent，同时添加了对React.RefAttributes<ImageRef>的支持，确保类型安全。
83-83: 使用 forwardRef 重构组件通过
forwardRef包装组件，允许 ref 从父组件传递到内部的 img 元素，同时保持类型安全。
115-121: 使用 useImperativeHandle 暴露自定义 ref 对象通过
useImperativeHandle暴露了符合ImageRef接口的对象，只包含nativeElement属性。这种方式既提供了对内部 img 元素的访问，又确保了组件的封装性，为未来扩展留出了空间。这符合 afc163 提到的未来可能添加像ref.next,ref.rotate,ref.focus等实例方法的考虑。
248-248: 更新了 img 元素的 ref 属性将 img 元素的 ref 从
getImgRef更改为handleRef，确保正确处理内部和外部引用。
294-294: 类型断言确保组件接口一致性使用类型断言确保
ImageInternal符合CompoundedComponent<ImageProps>接口，保持了组件 API 的一致性。
301-301: 组件导出正确组件导出没有变化，保持了向后兼容性。
| // ========================== Image Ref ========================== | ||
| const handleRef = (img: HTMLImageElement | null) => { | ||
| if (!img) { | ||
| return; | ||
| } | ||
| // 保存到内部引用 | ||
| imageElementRef.current = img; | ||
| getImgRef(img); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
集成内部和外部引用的处理函数
handleRef 函数巧妙地处理了内部引用的存储和对 getImgRef 的调用，确保了与现有功能的兼容性。但是，这里有一个潜在的问题：当 img 元素为 null 时直接返回了，但没有更新 imageElementRef.current。应该考虑在这种情况下将 imageElementRef.current 也设置为 null。
const handleRef = (img: HTMLImageElement | null) => {
if (!img) {
+ imageElementRef.current = null;
return;
}
// 保存到内部引用
imageElementRef.current = img;
getImgRef(img);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // ========================== Image Ref ========================== | |
| const handleRef = (img: HTMLImageElement | null) => { | |
| if (!img) { | |
| return; | |
| } | |
| // 保存到内部引用 | |
| imageElementRef.current = img; | |
| getImgRef(img); | |
| }; | |
| // ========================== Image Ref ========================== | |
| const handleRef = (img: HTMLImageElement | null) => { | |
| if (!img) { | |
| imageElementRef.current = null; | |
| return; | |
| } | |
| // 保存到内部引用 | |
| imageElementRef.current = img; | |
| getImgRef(img); | |
| }; |
tests/ref.test.tsx
Outdated
| expect(ref.current?.nativeElement?.alt).toBe('updated alt'); | ||
| }); | ||
|
|
||
| // 测试 ref 不能直接访问 img 元素属性 |
tests/ref.test.tsx
Outdated
| expect(ref.current?.nativeElement).not.toBeNull(); | ||
| expect(ref.current?.nativeElement).toBe(container.querySelector('.rc-image-img')); |
There was a problem hiding this comment.
expect(ref.current?.nativeElement).not.toBeNull(); 这个是多余的。和下面的互斥。
tests/ref.test.tsx
Outdated
|
|
||
| // 确保回调 ref 被调用，且 nativeElement 指向正确的 img 元素 | ||
| expect(imgRef).not.toBeNull(); | ||
| expect(imgRef?.nativeElement).not.toBeNull(); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/ref.test.tsx (3)
5-22: 测试对象引用的实现很完善这个测试用例很好地验证了对象引用是否能够正确访问内部的图像元素。测试验证了
ref.current存在且nativeElement指向正确的 img 元素，同时检查了标签名和 alt 属性是否正确。建议：考虑添加一个额外的测试，验证
ref.current直接访问属性（如ref.current.tagName或ref.current.src）时的行为，确保这些属性不是直接暴露的。
43-61: 属性访问测试可以更完善测试有效验证了通过
nativeElement访问 img 元素的 width 和 height 属性。注意到第59-61行有关于测试方法的注释，但没有实际实现。建议要么实现这些方法的测试，要么删除注释，以避免混淆。如果某些方法在 jsdom 环境中确实不可用，可以考虑添加一个简单的 mock 来测试方法调用。
90-90: 考虑添加额外的测试场景当前的测试套件覆盖了基本的引用转发功能，但可以考虑添加以下场景的测试：
- 验证与现有
getImgRef功能的兼容性（PR 目标中提到）- 测试当未提供引用时的行为
- 添加一个显式的测试，确认不能直接通过
ref.current访问图像元素的属性（如tagName或src）这些额外的测试将有助于确保新功能的完整性和与现有功能的兼容性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/ref.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/ref.test.tsx (1)
src/Image.tsx (1)
ImageRef(75-77)
🔇 Additional comments (3)
tests/ref.test.tsx (3)
1-4: 导入和设置看起来很合理导入了必要的测试工具和组件，包括从源文件导入的
ImageRef接口。
24-41: 回调引用测试逻辑清晰这个测试用例有效地验证了回调式引用是否正常工作。测试确保回调被调用并且
nativeElement指向正确的图像元素。
63-88: 重渲染稳定性测试很好，但有冗余的断言这个测试用例很好地验证了在组件重新渲染后引用的稳定性，确保
nativeElement引用的仍然是同一个图像元素，同时属性更新也能正确反映。第74行的
expect(initialImgElement).not.toBeNull();断言是多余的。因为在第86行已经有测试确保ref.current?.nativeElement与initialImgElement相同，如果initialImgElement为 null，那么第86行的测试将失败。
Closes #372
Summary by CodeRabbit
Summary by CodeRabbit
新功能
测试