Avoid rc package deep imports#164
Conversation
概览本 PR 升级了 Switch 组件的依赖版本（特别是 变更内容Enzyme 至 React Testing Library 完整迁移
相关 PR
建议审查人
审查工作量🎯 3 (中等难度) | ⏱️ ~25 分钟
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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 updates the project's dependencies, including an upgrade to React 18 and a migration of the test suite from Enzyme to React Testing Library. Additionally, it refactors imports from @rc-component/util to use named exports. The review feedback recommends improving the test implementation by explicitly providing the which property in keyboard events to align with component logic and utilizing standard React Testing Library queries like getByText for better testing practices.
|
|
||
| describe('rc-switch', () => { | ||
| function keyDownWithWhich(target, which) { | ||
| fireEvent.keyDown(target, { keyCode: which }); |
There was a problem hiding this comment.
建议在 fireEvent.keyDown 中显式传入 which 属性。虽然 keyCode 已被弃用，但由于组件内部逻辑（如 src/index.tsx 第 74 行）仍在使用 e.which，在测试中模拟该属性可以确保与组件逻辑的一致性并提高测试的健壮性。
| fireEvent.keyDown(target, { keyCode: which }); | |
| fireEvent.keyDown(target, { keyCode: which, which }); |
| const { container } = render(<Switch loadingIcon={<span className="extra">loading</span>} />); | ||
| expect(container.querySelector('.extra').textContent).toBe('loading'); |
There was a problem hiding this comment.
建议使用 React Testing Library 推荐的查询方式（如 getByText）来替代 container.querySelector。这不仅更符合 RTL 的最佳实践（从用户视角出发），而且在元素未找到时能提供更清晰的错误信息。
| const { container } = render(<Switch loadingIcon={<span className="extra">loading</span>} />); | |
| expect(container.querySelector('.extra').textContent).toBe('loading'); | |
| const { getByText } = render(<Switch loadingIcon={<span className="extra">loading</span>} />); | |
| expect(getByText('loading').classList.contains('extra')).toBe(true); |
![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
🤖 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 `@tests/index.spec.js`:
- Around line 140-147: The test "disabled should click not to change" defines an
onClick mock via onClick and fires a click on the switch but only asserts
onChange wasn't called; update the test that uses createSwitch({ disabled: true,
onChange, onClick, checked: true }) to also assert that the onClick mock was not
called after fireEvent.click(getByRole('switch')), i.e., add
expect(onClick).not.toHaveBeenCalled() to the test to verify disabled state
prevents click handlers from running.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 404ef499-24e5-4767-8f10-d488f8a14576
📒 Files selected for processing (3)
package.jsonsrc/index.tsxtests/index.spec.js
| it('disabled should click not to change', () => { | ||
| const onChange = jest.fn(); | ||
| const onClick = jest.fn(); | ||
| const wrapper = createSwitch({ disabled: true, onChange, onClick, checked: true }); | ||
| const { getByRole } = createSwitch({ disabled: true, onChange, onClick, checked: true }); | ||
|
|
||
| wrapper.simulate('click'); | ||
| fireEvent.click(getByRole('switch')); | ||
| expect(onChange).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
考虑验证 onClick 回调也未被调用。
测试中定义了 onClick 回调（第 142 行），但只断言了 onChange 未被调用。为了更完整的测试覆盖，建议也验证禁用状态下 onClick 的行为。
💡 建议添加 onClick 断言
fireEvent.click(getByRole('switch'));
expect(onChange).not.toHaveBeenCalled();
+expect(onClick).not.toHaveBeenCalled();📝 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('disabled should click not to change', () => { | |
| const onChange = jest.fn(); | |
| const onClick = jest.fn(); | |
| const wrapper = createSwitch({ disabled: true, onChange, onClick, checked: true }); | |
| const { getByRole } = createSwitch({ disabled: true, onChange, onClick, checked: true }); | |
| wrapper.simulate('click'); | |
| fireEvent.click(getByRole('switch')); | |
| expect(onChange).not.toHaveBeenCalled(); | |
| }); | |
| it('disabled should click not to change', () => { | |
| const onChange = jest.fn(); | |
| const onClick = jest.fn(); | |
| const { getByRole } = createSwitch({ disabled: true, onChange, onClick, checked: true }); | |
| fireEvent.click(getByRole('switch')); | |
| expect(onChange).not.toHaveBeenCalled(); | |
| expect(onClick).not.toHaveBeenCalled(); | |
| }); |
🤖 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 `@tests/index.spec.js` around lines 140 - 147, The test "disabled should click
not to change" defines an onClick mock via onClick and fires a click on the
switch but only asserts onChange wasn't called; update the test that uses
createSwitch({ disabled: true, onChange, onClick, checked: true }) to also
assert that the onClick mock was not called after
fireEvent.click(getByRole('switch')), i.e., add
expect(onClick).not.toHaveBeenCalled() to the test to verify disabled state
prevents click handlers from running.
1 participant
背景
antd 侧限制继续使用 rc 包的
lib/es深路径导入，需要将 switch 中对@rc-component/util的内部路径依赖迁移到包根入口。调整内容
@rc-component/father-plugin，使用插件统一拦截 rc 包lib/es深路径导入。@rc-component/util，从@rc-component/util根入口导入KeyCode与useControlledState。验证
npm test -- --runInBandnpm run lintnpm run compileSummary by CodeRabbit
发行说明