chore: useEvent type support undefined #706
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @zombieJ, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough将 Changes
Sequence Diagram(s)sequenceDiagram
participant C as Component
participant H as useEvent hook
participant R as fnRef
participant M as memoizedFn
C->>H: 提供 callback (可能为 undefined)
H->>R: 初始化 / 更新 `fnRef.current = callback`
H->>M: 返回 memoizedFn
C->>M: 调用 memoizedFn(...args)
M->>R: 读取 `fnRef.current`
alt fnRef.current 存在
R->>R: 执行 `fnRef.current(...args)`
else fnRef.current undefined
note right of R `#f8f3d7`: 使用可选链，静默返回 undefined
end
Estimated code review effort🎯 2 (简单) | ⏱️ ~10 分钟
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #706 +/- ##
=======================================
Coverage 90.28% 90.28%
=======================================
Files 37 37
Lines 885 885
Branches 301 293 -8
=======================================
Hits 799 799
Misses 84 84
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to add support for an undefined callback in the useEvent hook. The changes to the type constraints and useRef are good improvements. However, there is a significant type safety issue with the return type of the hook. When an undefined callback is passed, the returned function's type does not accurately represent its runtime behavior, which can lead to unexpected errors. My review includes a detailed explanation and a suggested fix to make the hook type-safe.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/hooks/useEvent.ts(1 hunks)
⏰ 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). (1)
- GitHub Check: test / react component workflow
🔇 Additional comments (1)
src/hooks/useEvent.ts (1)
8-8: 改进：ref 类型更加精确将 ref 类型从
any改为T | undefined并使用callback进行初始化是一个很好的改进，提升了类型安全性。
|
@gemini-code-assist 请再看一下 |
There was a problem hiding this comment.
Code Review
这个 PR 旨在改进 useEvent hook，使其支持 undefined 回调，这是一个很好的增强。类型定义变得更加精确，这是一个正确的方向。
我在代码中发现了一些可以改进的地方：
useEvent的实现与其新的类型签名存在一个关键的类型不匹配问题，可能导致编译错误或类型不安全。useRef的类型可以简化，使其更清晰。useCallback中的参数类型可以更精确，以增强类型安全。- 为
useEvent添加的测试用例目前只检查编译时类型，建议扩展测试以覆盖运行时的各种场景，确保 hook 的健壮性。
详细的建议请见具体的代码注释。
| describe('useEvent', () => { | ||
| it('extract type', () => { | ||
| const Demo = (props: { | ||
| canUndefined?: (a: number) => boolean; | ||
| notUndefined: (a: number) => boolean; | ||
| }) => { | ||
| const ua = useEvent(props.canUndefined); | ||
| const ub = useEvent(props.notUndefined); | ||
|
|
||
| ua(1); | ||
| ub(2); | ||
|
|
||
| return null; | ||
| }; | ||
|
|
||
| expect(Demo).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
这个测试用例目前只在编译时检查类型，通过 expect(Demo).toBeTruthy() 来断言组件定义本身。这并不能验证 useEvent hook 的运行时行为。
建议增加更全面的测试，覆盖以下场景：
- 运行时行为：渲染组件，并实际调用
useEvent返回的函数，断言回调函数是否被正确调用。 - 回调更新：测试当
callbackprop 变更后，useEvent返回的函数是否会调用最新的回调。 undefined场景：测试当传入undefined时，调用返回的函数不会抛出错误。
一个更完善的测试可能如下所示：
it('should call the latest function', () => {
const fn1 = jest.fn();
const fn2 = jest.fn();
const Test = ({ cb }: { cb: () => void }) => {
const eventFn = useEvent(cb);
return <button onClick={eventFn}>Click</button>;
};
const { getByText, rerender } = render(<Test cb={fn1} />);
fireEvent.click(getByText('Click'));
expect(fn1).toHaveBeenCalledTimes(1);
expect(fn2).toHaveBeenCalledTimes(0);
rerender(<Test cb={fn2} />);
fireEvent.click(getByText('Click'));
expect(fn1).toHaveBeenCalledTimes(1);
expect(fn2).toHaveBeenCalledTimes(1);
});
it('should not throw error when callback is undefined', () => {
const Test = ({ cb }: { cb?: () => void }) => {
const eventFn = useEvent(cb);
return <button onClick={eventFn}>Click</button>;
};
const { getByText } = render(<Test cb={undefined} />);
expect(() => {
fireEvent.click(getByText('Click'));
}).not.toThrow();
});
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/hooks.test.tsx (1)
711-728: 建议增强测试以验证运行时行为。当前测试主要验证类型编译正确性，但缺少运行时行为验证。建议添加以下测试场景：
- 验证当
canUndefined实际为undefined时，调用ua()返回undefined- 验证当
canUndefined为函数时，调用ua()返回正确的结果- 验证
notUndefined的调用行为可以改进为：
describe('useEvent', () => { it('extract type', () => { const Demo = (props: { canUndefined?: (a: number) => boolean; notUndefined: (a: number) => boolean; }) => { const ua = useEvent(props.canUndefined); const ub = useEvent(props.notUndefined); - ua(1); - ub(2); + const resultA = ua(1); + const resultB = ub(2); return null; }; - expect(Demo).toBeTruthy(); + const { rerender } = render(<Demo notUndefined={x => x > 0} />); + + // Test with canUndefined as a function + rerender(<Demo canUndefined={x => x > 5} notUndefined={x => x > 0} />); + + // Test with canUndefined as undefined + rerender(<Demo canUndefined={undefined} notUndefined={x => x > 0} />); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/hooks/useEvent.ts(1 hunks)tests/hooks.test.tsx(2 hunks)
⏰ 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). (2)
- GitHub Check: test / react component workflow
- GitHub Check: test / react component workflow
🔇 Additional comments (3)
tests/hooks.test.tsx (1)
12-12: 导入语句正确。新增的
useEvent导入语句符合测试需求。src/hooks/useEvent.ts (2)
15-18: 移除类型断言是好的，但需确保类型匹配。相比旧代码的
as T断言，当前实现移除了强制类型转换，这提高了类型安全性。可选链?.的使用也是正确的，可以安全处理undefined回调。但如上一条评论所述，可选链会导致返回类型包含
| undefined，这与条件返回类型（当T为确定函数时应返回T）可能存在不匹配。如果 TypeScript 编译通过，可能是由于any类型的存在。建议在修复 ref 类型后，验证是否需要类型断言来匹配条件返回类型。
12-13: 简化 ref 的类型定义。当前 ref 被类型化为
T | undefined，但callback参数本身就是T。当T已经包含undefined（如T = Func | undefined）时，T | undefined与T等价；当T是确定的函数时，T | undefined则引入了不必要的类型扩宽。建议简化为：
- const fnRef = React.useRef<T | undefined>(callback); + const fnRef = React.useRef<T>(callback);这样可以：
- 避免不必要的类型扩宽
- 让类型推断更精确
- 使实现与条件返回类型更好地匹配
Likely an incorrect or invalid review comment.
Summary by CodeRabbit
发布说明