fix: Skip first frame render with motion appear #73
Conversation
On initial mount when status is STATUS_NONE, return 'NONE' instead of true to prevent rendering children until style is ready. This improves the appear animation behavior by ensuring style synchronization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Removed TODO and updated className assertion to verify the first render (prepare stage) with correct className value. Also changed from toHaveBeenCalled to checking mockRender.mock.calls[0][0] to match the first render call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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! 此拉取请求旨在优化组件的初始渲染行为，特别是在使用动画入场（ Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
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
|
Walkthrough在 CSSMotion 的渲染路径中新增对 styleReady 为 Changes
Estimated code review effort🎯 3 (中等复杂度) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/CSSMotion.spec.tsx`:
- Around line 495-510: The TypeScript error comes from using jest.fn(() => null)
without a parameter type so TypeScript infers the mock's call tuple as empty;
update the mock declaration for mockRender to include an explicit call signature
(e.g., use jest.fn<(arg: any) => any>(() => null) or jest.fn<[any], any>(() =>
null) or type it as jest.Mock<any, any>) so that mockRender.mock.calls[0][0] is
correctly typed; change the jest.fn instantiation where mockRender is declared
in the CSSMotion test.
🧹 Nitpick comments (2)
src/hooks/useStatus.ts (1)
314-321: 建议将'NONE'提取为命名常量，与现有的STATUS_NONE风格保持一致。当前
'NONE'作为魔法字符串散落在useStatus.ts和CSSMotion.tsx两处，容易拼写错误且缺乏语义关联。建议在interface.ts中导出一个常量（如STYLE_READY_NONE），在两处引用。♻️ 建议的修改
在
src/interface.ts中添加：export const STYLE_READY_NONE = 'NONE' as const;然后在
useStatus.ts和CSSMotion.tsx中使用该常量替代字符串字面量'NONE'。src/CSSMotion.tsx (1)
187-195: 注意：'NONE'是 truthy 值，会触发idRef.current自增。Line 187 的
if (styleReady)在styleReady === 'NONE'时为true（字符串是 truthy），这会使idRef.current递增并触发useMemo重新计算。当前逻辑下这是正确的行为——memo 重新执行后在 Line 193 返回null，从而跳过首帧渲染。但如果未来有人将
'NONE'改为 falsy 值（如0或null），此处行为会静默改变。建议在if条件中显式表达意图：- if (styleReady) { + if (styleReady === true || styleReady === 'NONE') {这样可以更清晰地传达"只有
false时不更新"的语义。
| it('styleReady returns NONE on first mount when status is STATUS_NONE', () => { | ||
| const mockRender = jest.fn(() => null); | ||
|
|
||
| render( | ||
| <CSSMotion visible motionAppear motionName="test"> | ||
| {mockRender} | ||
| </CSSMotion>, | ||
| ); | ||
|
|
||
| // First render (prepare stage) | ||
| expect(mockRender.mock.calls[0][0]).toEqual( | ||
| expect.objectContaining({ | ||
| className: 'test-appear test-appear-prepare test', | ||
| }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
CI 构建失败：TypeScript 类型错误 TS2493，需要为 mock 函数添加参数类型。
流水线报错 Tuple type '[]' of length '0' has no element at index '0'，原因是 jest.fn(() => null) 没有声明参数，TypeScript 推断 mock.calls 中每次调用的参数元组为 []，导致 calls[0][0] 无法通过类型检查。
🔧 修复建议
- const mockRender = jest.fn(() => null);
+ const mockRender = jest.fn((_props: Record<string, any>, _ref?: any) => null);或者使用类型断言：
- expect(mockRender.mock.calls[0][0]).toEqual(
+ expect((mockRender.mock.calls[0] as [any, any])[0]).toEqual(📝 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.
| it('styleReady returns NONE on first mount when status is STATUS_NONE', () => { | |
| const mockRender = jest.fn(() => null); | |
| render( | |
| <CSSMotion visible motionAppear motionName="test"> | |
| {mockRender} | |
| </CSSMotion>, | |
| ); | |
| // First render (prepare stage) | |
| expect(mockRender.mock.calls[0][0]).toEqual( | |
| expect.objectContaining({ | |
| className: 'test-appear test-appear-prepare test', | |
| }), | |
| ); | |
| }); | |
| it('styleReady returns NONE on first mount when status is STATUS_NONE', () => { | |
| const mockRender = jest.fn((_props: Record<string, any>, _ref?: any) => null); | |
| render( | |
| <CSSMotion visible motionAppear motionName="test"> | |
| {mockRender} | |
| </CSSMotion>, | |
| ); | |
| // First render (prepare stage) | |
| expect(mockRender.mock.calls[0][0]).toEqual( | |
| expect.objectContaining({ | |
| className: 'test-appear test-appear-prepare test', | |
| }), | |
| ); | |
| }); |
🧰 Tools
🪛 GitHub Actions: ✅ test
[error] 505-505: Command 'bunx tsc --noEmit' failed with TS2493: Tuple type '[]' of length '0' has no element at index '0'.
🤖 Prompt for AI Agents
In `@tests/CSSMotion.spec.tsx` around lines 495 - 510, The TypeScript error comes
from using jest.fn(() => null) without a parameter type so TypeScript infers the
mock's call tuple as empty; update the mock declaration for mockRender to
include an explicit call signature (e.g., use jest.fn<(arg: any) => any>(() =>
null) or jest.fn<[any], any>(() => null) or type it as jest.Mock<any, any>) so
that mockRender.mock.calls[0][0] is correctly typed; change the jest.fn
instantiation where mockRender is declared in the CSSMotion test.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #73 +/- ##
==========================================
+ Coverage 98.52% 98.55% +0.02%
==========================================
Files 11 11
Lines 408 414 +6
Branches 112 117 +5
==========================================
+ Hits 402 408 +6
Misses 6 6 ☔ 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}
| !mountedRef.current && currentStatus === STATUS_NONE | ||
| ? // Appear | ||
| 'NONE' | ||
| : // Enter or Leave | ||
| step === STEP_START || step === STEP_ACTIVE | ||
| ? styleStep === step | ||
| : true, |
There was a problem hiding this comment.
将 styleReady 设置为 'NONE' 的条件 !mountedRef.current && currentStatus === STATUS_NONE 过于宽泛。如果组件在挂载时 visible={true} 但 motionAppear={false}，这会导致组件渲染为 null 并且卡在这个状态。
该条件应该更具体，仅适用于 "appear" 动画。它应该检查 motionAppear 和 visible 属性，类似于为 STATUS_APPEAR 确定 nextStatus 的方式。
!mountedRef.current && visible && motionAppear
? 'NONE'
: step === STEP_START || step === STEP_ACTIVE
? styleStep === step
: true,| it('styleReady returns NONE on first mount when status is STATUS_NONE', () => { | ||
| const mockRender = jest.fn(() => null) as jest.Mock; | ||
| (mockRender as any).mock.calls = [] as any; | ||
|
|
||
| render( | ||
| <CSSMotion visible motionAppear motionName="test"> | ||
| {mockRender} | ||
| </CSSMotion>, | ||
| ); | ||
|
|
There was a problem hiding this comment.
这个测试的标题和注释有点误导性。该测试并未直接检查 styleReady 的值，而是检查其导致跳过第一次渲染的后果。一个更具描述性的标题和注释将提高代码的清晰度和可维护性。
例如：
标题: it('should skip initial render frame when motionAppear is enabled')
注释: // The first render of children should be in the 'prepare' stage
| it('styleReady returns NONE on first mount when status is STATUS_NONE', () => { | |
| const mockRender = jest.fn(() => null) as jest.Mock; | |
| (mockRender as any).mock.calls = [] as any; | |
| render( | |
| <CSSMotion visible motionAppear motionName="test"> | |
| {mockRender} | |
| </CSSMotion>, | |
| ); | |
| it('should skip initial render frame when motionAppear is enabled', () => { | |
| const mockRender = jest.fn(() => null); | |
| render( | |
| <CSSMotion visible motionAppear motionName="test"> | |
| {mockRender} | |
| </CSSMotion>, | |
| ); | |
| // The first render of children should be in the 'prepare' stage |
This addresses the PR comment suggesting that styleReady should only return 'NONE' when motionAppear is true, otherwise it should always render the children. Ref: #73 (comment) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
修复部分机型下，
useLayoutEffect仍会多渲染一帧的问题。ref ant-design/ant-design#56604
PS: 需要到 antd 里也抬一下。
Summary by CodeRabbit
发布说明