fix(swipe): resolve infinite re-render loop causing page freeze#3475
Conversation
- Fix useCallback refs with unstable deps (props.leftAction/rightAction) that triggered setActionWidth on every render, causing an infinite loop - Add width equality check before updating state to break the cycle - Fix `opened` ref accessed without `.current` in both H5 and Taro versions Closes #3433 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Walkthrough该PR将 Swipe 组件中对打开状态的判断由 ChangesSwipe组件状态与闭包修复
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 @@
## feat_v4.x #3475 +/- ##
============================================
Coverage ? 88.33%
============================================
Files ? 295
Lines ? 19747
Branches ? 3117
============================================
Hits ? 17443
Misses ? 2298
Partials ? 6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add tests for swipe open/close via touch events and disabled state to cover the useCallback ref, onTouchMove, and toggle logic paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/packages/swipe/__tests__/swipe.spec.tsx (1)
102-145: ⚡ Quick win把
getRect的恢复放到afterEach或finally。现在
spy.mockRestore()都放在断言后面;一旦前面的断言失败,mock 会泄漏到后续 case,测试会串掉。统一用afterEach(() => jest.restoreAllMocks())会更稳。🔧 可参考的调整
import * as getRectModule from '`@/utils/get-rect`' + +afterEach(() => { + jest.restoreAllMocks() +}) test('swipe right to open via touch', () => { const spy = jest.spyOn(getRectModule, 'getRect').mockReturnValue({ @@ expect(onOpen).toHaveBeenCalled() - spy.mockRestore() })Also applies to: 148-190, 194-255
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/packages/swipe/__tests__/swipe.spec.tsx` around lines 102 - 145, The test creates a jest spy on getRect (const spy = jest.spyOn(getRectModule, 'getRect')) and calls spy.mockRestore() only after assertions, which can leak mocks if a test fails; move mock cleanup into a shared afterEach by removing per-test spy.mockRestore() calls and adding afterEach(() => jest.restoreAllMocks()) (or ensure each test wraps spy creation in try/finally and always calls spy.mockRestore()); update tests referencing getRect/spy across the file (including the other ranges noted) to rely on the centralized afterEach cleanup instead of per-test restores.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/packages/swipe/__tests__/swipe.spec.tsx`:
- Around line 101-255: Add explicit boundary tests for the Swipe thresholds:
when getRect (mocked via getRectModule.getRect) returns width = 80, add one test
that starts closed, performs a touchStart at x=200 then touchMove to x=168 (≈40%
drag / 32px) and touchEnd and assert onOpen was called; add another test that
starts opened (simulate initial open by performing the >70% drag first as in
existing "Open first" steps) then touchStart at the opened position and
touchMove back to x=168 (≈40%) and touchEnd and assert onClose was NOT called,
then touchMove back further to x=144 (≈70% / 56px) and touchEnd and assert
onClose was called; reference the existing tests' use of getRectModule.getRect,
the Swipe component and the wrapper queried via '.nut-swipe', and the
onOpen/onClose mocks when adding these boundary assertions.
---
Nitpick comments:
In `@src/packages/swipe/__tests__/swipe.spec.tsx`:
- Around line 102-145: The test creates a jest spy on getRect (const spy =
jest.spyOn(getRectModule, 'getRect')) and calls spy.mockRestore() only after
assertions, which can leak mocks if a test fails; move mock cleanup into a
shared afterEach by removing per-test spy.mockRestore() calls and adding
afterEach(() => jest.restoreAllMocks()) (or ensure each test wraps spy creation
in try/finally and always calls spy.mockRestore()); update tests referencing
getRect/spy across the file (including the other ranges noted) to rely on the
centralized afterEach cleanup instead of per-test restores.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c0991142-773c-4378-b2c0-e03c5d4799fd
📒 Files selected for processing (1)
src/packages/swipe/__tests__/swipe.spec.tsx
| test('swipe right to open via touch', () => { | ||
| const spy = jest.spyOn(getRectModule, 'getRect').mockReturnValue({ | ||
| width: 80, | ||
| height: 40, | ||
| top: 0, | ||
| left: 0, | ||
| right: 80, | ||
| bottom: 40, | ||
| }) | ||
|
|
||
| const onOpen = jest.fn() | ||
| const { container } = render( | ||
| <Swipe | ||
| rightAction={ | ||
| <Button type="primary" shape="square"> | ||
| 删除 | ||
| </Button> | ||
| } | ||
| onOpen={onOpen} | ||
| > | ||
| <Cell title="滑动" radius={0} /> | ||
| </Swipe> | ||
| ) | ||
|
|
||
| const wrapper = container.querySelector('.nut-swipe') as HTMLElement | ||
|
|
||
| act(() => { | ||
| fireEvent.touchStart(wrapper, { | ||
| touches: [{ clientX: 200, clientY: 0, pageX: 200, pageY: 0 }], | ||
| }) | ||
| }) | ||
| act(() => { | ||
| fireEvent.touchMove(wrapper, { | ||
| touches: [{ clientX: 100, clientY: 0, pageX: 100, pageY: 0 }], | ||
| }) | ||
| }) | ||
| act(() => { | ||
| fireEvent.touchEnd(wrapper, { | ||
| changedTouches: [{ clientX: 100, clientY: 0 }], | ||
| }) | ||
| }) | ||
|
|
||
| expect(onOpen).toHaveBeenCalled() | ||
| spy.mockRestore() | ||
| }) | ||
|
|
||
| test('swipe left to open via touch', () => { | ||
| const spy = jest.spyOn(getRectModule, 'getRect').mockReturnValue({ | ||
| width: 80, | ||
| height: 40, | ||
| top: 0, | ||
| left: 0, | ||
| right: 80, | ||
| bottom: 40, | ||
| }) | ||
|
|
||
| const onOpen = jest.fn() | ||
| const { container } = render( | ||
| <Swipe | ||
| leftAction={ | ||
| <Button type="success" shape="square"> | ||
| 选择 | ||
| </Button> | ||
| } | ||
| onOpen={onOpen} | ||
| > | ||
| <Cell title="滑动" radius={0} /> | ||
| </Swipe> | ||
| ) | ||
|
|
||
| const wrapper = container.querySelector('.nut-swipe') as HTMLElement | ||
|
|
||
| act(() => { | ||
| fireEvent.touchStart(wrapper, { | ||
| touches: [{ clientX: 100, clientY: 0, pageX: 100, pageY: 0 }], | ||
| }) | ||
| }) | ||
| act(() => { | ||
| fireEvent.touchMove(wrapper, { | ||
| touches: [{ clientX: 200, clientY: 0, pageX: 200, pageY: 0 }], | ||
| }) | ||
| }) | ||
| act(() => { | ||
| fireEvent.touchEnd(wrapper, { | ||
| changedTouches: [{ clientX: 200, clientY: 0 }], | ||
| }) | ||
| }) | ||
|
|
||
| expect(onOpen).toHaveBeenCalled() | ||
| spy.mockRestore() | ||
| }) | ||
|
|
||
| test('swipe close after opened', () => { | ||
| const spy = jest.spyOn(getRectModule, 'getRect').mockReturnValue({ | ||
| width: 80, | ||
| height: 40, | ||
| top: 0, | ||
| left: 0, | ||
| right: 80, | ||
| bottom: 40, | ||
| }) | ||
|
|
||
| const onClose = jest.fn() | ||
| const { container } = render( | ||
| <Swipe | ||
| rightAction={ | ||
| <Button type="primary" shape="square"> | ||
| 删除 | ||
| </Button> | ||
| } | ||
| onClose={onClose} | ||
| > | ||
| <Cell title="滑动" radius={0} /> | ||
| </Swipe> | ||
| ) | ||
|
|
||
| const wrapper = container.querySelector('.nut-swipe') as HTMLElement | ||
|
|
||
| // Open first | ||
| act(() => { | ||
| fireEvent.touchStart(wrapper, { | ||
| touches: [{ clientX: 200, clientY: 0, pageX: 200, pageY: 0 }], | ||
| }) | ||
| }) | ||
| act(() => { | ||
| fireEvent.touchMove(wrapper, { | ||
| touches: [{ clientX: 100, clientY: 0, pageX: 100, pageY: 0 }], | ||
| }) | ||
| }) | ||
| act(() => { | ||
| fireEvent.touchEnd(wrapper, { | ||
| changedTouches: [{ clientX: 100, clientY: 0 }], | ||
| }) | ||
| }) | ||
|
|
||
| // Close by swiping back | ||
| act(() => { | ||
| fireEvent.touchStart(wrapper, { | ||
| touches: [{ clientX: 100, clientY: 0, pageX: 100, pageY: 0 }], | ||
| }) | ||
| }) | ||
| act(() => { | ||
| fireEvent.touchMove(wrapper, { | ||
| touches: [{ clientX: 200, clientY: 0, pageX: 200, pageY: 0 }], | ||
| }) | ||
| }) | ||
| act(() => { | ||
| fireEvent.touchEnd(wrapper, { | ||
| changedTouches: [{ clientX: 200, clientY: 0 }], | ||
| }) | ||
| }) | ||
|
|
||
| expect(onClose).toHaveBeenCalled() | ||
| spy.mockRestore() | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
补上阈值临界区间的断言。
这里三组手势都在 width = 80 时使用了 100px 位移,已经同时超过了关闭态 30% 和打开态 70% 两个阈值。这样即使 opened 仍然被误用为 ref 对象,这些用例也会继续通过,锁不住这次 opened.current 修复的回归。建议至少补一组边界值断言:关闭态拖到约 40% 应该打开;已打开态回拖约 40% 不应关闭,超过约 70% 才关闭。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/packages/swipe/__tests__/swipe.spec.tsx` around lines 101 - 255, Add
explicit boundary tests for the Swipe thresholds: when getRect (mocked via
getRectModule.getRect) returns width = 80, add one test that starts closed,
performs a touchStart at x=200 then touchMove to x=168 (≈40% drag / 32px) and
touchEnd and assert onOpen was called; add another test that starts opened
(simulate initial open by performing the >70% drag first as in existing "Open
first" steps) then touchStart at the opened position and touchMove back to x=168
(≈40%) and touchEnd and assert onClose was NOT called, then touchMove back
further to x=144 (≈70% / 56px) and touchEnd and assert onClose was called;
reference the existing tests' use of getRectModule.getRect, the Swipe component
and the wrapper queried via '.nut-swipe', and the onOpen/onClose mocks when
adding these boundary assertions.
fix(swipe): resolve infinite re-render loop causing page freeze
Closes #3433
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
问题现象
用户在 H5 环境下使用 Swipe 组件,进入页面即卡死(无限重渲染导致页面冻结)。
根因分析
修复涉及两类 bug:
Bug 1:
useCallback依赖不稳定导致无限循环(仅 H5 端swipe.tsx)leftRef/rightRef使用了useCallback+ ref callback 模式来测量左右操作区域宽度,但依赖数组写的是[props.leftAction]/[props.rightAction]。这两个 prop 是 React 元素(JSX),每次父组件渲染都会产生新的引用。导致:props.leftAction新引用 →leftRefcallback 重建leftRef变化 → React 调用 ref callback →setActionWidth触发 state 更新Bug 2:
openedref 误用为布尔值(H5 + Taro 双端)opened是useRef(false)创建的 ref 对象,代码中有两处直接使用了!opened和opened而非opened.current。ref 对象本身是一个永远存在的对象(truthy),所以!opened永远是false,opened永远是true,导致:onTouchMove中isEdge判断逻辑错误,滑动边界检测失效toggle中baseNum始终为1 - 0.3 = 0.7,开合阈值判断不随 open/close 状态变化修复方案
useCallback依赖改为[]swipe.tsxactionWidthRefswipe.tsx!opened→!opened.currentswipe.tsx+swipe.taro.tsxopened→opened.currentswipe.tsx+swipe.taro.tsx兼容性说明
opened状态判断恢复正确,滑动开合阈值(30% / 70%)会随组件状态正确切换。之前始终按已打开状态计算阈值(0.7),修复后未打开时用 30%、已打开时用 70%createSelectorQuery(不是 ref callback),不存在无限循环问题,但opened的 ref 误用同样存在,本次一并修复☑️ 请求合并前的自查清单
Summary by CodeRabbit
发布说明
Bug Fixes
Tests