-
-
Notifications
You must be signed in to change notification settings - Fork 144
chore: bump upstream version #635
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @yoyo837, 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 focuses on updating several key dependencies and raising the minimum required React version to 18.0.0. The primary goal is to ensure the project remains compatible with the latest versions of its underlying components and to align with the current React ecosystem, potentially benefiting from performance improvements and new features introduced in React 18. 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)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request bumps several dependencies, most notably updating the peerDependencies for react and react-dom to require version 18 or higher. This is a major breaking change for consumers of the library.
My main concern, as detailed in the comment on package.json, is that while the peerDependencies are updated, the project's devDependencies still point to React 16. This means the library is not being tested with the new minimum required version of React. This is a critical issue that must be addressed by updating devDependencies and the testing setup to use React 18.
Additionally, given this is a breaking change, it should be accompanied by:
- A major version bump for this package (e.g., from
1.8.0to2.0.0). - A corresponding entry in the
CHANGELOG.mdto inform users about this important update.
| "react": ">=18.0.0", | ||
| "react-dom": ">=18.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumping the peerDependencies for react and react-dom to >=18.0.0 is a significant breaking change. However, the devDependencies in this project are still using React 16 ("react": "^16.0.0", "react-dom": "^16.0.0"). This means the project's tests are running against a version of React that no longer satisfies the new peer dependency requirement.
To ensure compatibility, the library must be tested against the versions of React it claims to support. Please update the devDependencies to use React 18 and adjust the testing infrastructure accordingly. For example, enzyme-adapter-react-16 is not compatible with React 18 and will need to be replaced or updated.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (1)
69-71: Enzyme 相关依赖与 React 18 不兼容。
enzyme-adapter-react-16不支持 React 18,但 devDependencies 中的 React 已升级到 18.x。由于测试已迁移到 React Testing Library,建议移除这些遗留的 Enzyme 依赖:
enzymeenzyme-adapter-react-16enzyme-to-json@types/enzyme"@rc-component/father-plugin": "^2.0.2", "@rc-component/form": "^1.4.0", "@rc-component/np": "^1.0.3", "@rc-component/trigger": "^3.0.0", "@testing-library/react": "^12.1.5", - "@types/enzyme": "^3.1.15", "@types/jest": "^29.4.0", ... - "enzyme": "^3.3.0", - "enzyme-adapter-react-16": "^1.15.6", - "enzyme-to-json": "^3.2.1",tests/checkable.spec.tsx (1)
195-247: 禁用复选框用例中需在第二次点击后重新查询选中节点当前在“should be correct expression with disableCheckbox”用例里,第一次点击后通过
const checkedCheckboxes = container.querySelectorAll('.rc-cascader-checkbox-checked'); expect(checkedCheckboxes).toHaveLength(0);完成断言;第二次点击禁用 menuItem 后直接复用同一个
checkedCheckboxes变量再次expect,由于NodeList是静态快照,即便 DOM 状态变化,这里也始终为 0,无法真正覆盖第二次点击场景。建议第二次点击后重新查询 DOM:
- fireEvent.click(disabledCheckboxes[0]); - expect(checkedCheckboxes).toHaveLength(0); + fireEvent.click(disabledCheckboxes[0]); + const checkedCheckboxesAfterDisabledClick = container.querySelectorAll( + '.rc-cascader-checkbox-checked', + ); + expect(checkedCheckboxesAfterDisabledClick).toHaveLength(0);这样可以确保两次交互都被正确校验。
🧹 Nitpick comments (5)
tests/util.ts (1)
14-39:selectOption和clickOption功能重复。这两个函数实现了几乎相同的功能(定位菜单项并触发事件),但存在细微差异:
selectOption使用.rc-cascader-menu选择器,找不到元素时抛出错误clickOption使用ul.rc-cascader-menu选择器,找不到元素时静默返回建议统一使用其中一个函数,或让
clickOption内部调用selectOption以避免重复代码。export function clickOption( container: HTMLElement, menuIndex: number, itemIndex: number, type: 'click' | 'doubleClick' | 'mouseEnter' = 'click', ): void { - const option = findOption(container, menuIndex, itemIndex); - if (!option) return; - - if (type === 'doubleClick') { - fireEvent.doubleClick(option); - } else if (type === 'mouseEnter') { - fireEvent.mouseEnter(option); - } else { - fireEvent.click(option); - } + selectOption(container, menuIndex, itemIndex, type); }Also applies to: 61-77
tests/search.limit.spec.tsx (2)
6-13: 建议从tests/util.ts导入doSearch而非本地重复定义。
tests/util.ts已经导出了相同功能的doSearch函数,建议复用以保持一致性并减少重复代码。import React from 'react'; import { fireEvent, render } from '@testing-library/react'; import Cascader from '../src'; +import { doSearch } from './util'; describe('Cascader.Search', () => { - function doSearch(container: HTMLElement, search: string) { - const input = container.querySelector('input'); - fireEvent.change(input!, { - target: { - value: search, - }, - }); - } const options = [
29-29: 三个测试用例名称重复,建议使用描述性名称。三个
it('limit', ...)测试用例名称相同,在测试失败时难以定位问题。建议根据测试场景命名:- it('limit', () => { + it('limit: false should show all results', () => { // ... limit: false }); - it('limit', () => { + it('limit: 0 should show all results', () => { // ... limit: 0 }); - it('limit', () => { + it('limit: 20 should only show 20 results', () => { // ... limit: 20 });Also applies to: 45-45, 61-61
tests/checkable.spec.tsx (1)
5-5: 移除未使用的selectOption导入以消除静态检查告警本文件只使用了
clickOption,selectOption未被引用,建议删除以保持代码整洁并避免 CodeQL/TS 的 unused import 告警。-import { clickOption, selectOption } from './util'; +import { clickOption } from './util';tests/keyboard.spec.tsx (1)
164-165: 删除未使用的input变量以精简测试并消除静态分析告警这些用例中声明的
const input = container.querySelector('input')!;没有被后续代码使用,键盘行为全部通过keyDown(container, ...)间接在 input 上触发。可以直接移除这些变量声明,既不会影响测试行为,又能消除 CodeQL 的 unused variable 提示:- const input = container.querySelector('input')!; - @@ - const input = container.querySelector('input')!; - @@ - const input = container.querySelector('input')!; @@ - const input = container.querySelector('input')!; - @@ - const input = container.querySelector('input')!; - @@ - const input = container.querySelector('input')!; - @@ - const input = container.querySelector('input')!; -其余键盘流程(打开/关闭、搜索、RTL、全部禁用等)逻辑看起来是正确的。
Also applies to: 200-201, 238-239, 253-254, 293-294, 314-315, 343-344
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/__snapshots__/search.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (12)
examples/rc-form.tsx(1 hunks)package.json(2 hunks)src/OptionList/useKeyboard.ts(1 hunks)tests/checkable.spec.tsx(7 hunks)tests/fieldNames.spec.tsx(7 hunks)tests/index.spec.tsx(34 hunks)tests/keyboard.spec.tsx(4 hunks)tests/loadData.spec.tsx(7 hunks)tests/search.limit.spec.tsx(5 hunks)tests/search.spec.tsx(9 hunks)tests/selector.spec.tsx(4 hunks)tests/util.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/OptionList/useKeyboard.ts
🧰 Additional context used
🧬 Code graph analysis (5)
tests/fieldNames.spec.tsx (1)
tests/util.ts (2)
expectOpen(3-12)clickOption(61-77)
tests/index.spec.tsx (5)
tests/util.ts (2)
clickOption(61-77)isOpen(42-46)tests/demoOptions.ts (2)
addressOptions(38-99)optionsForActiveMenuItems(3-36)src/hooks/useEntities.ts (1)
options(15-51)src/Cascader.tsx (1)
CascaderRef(204-204)src/index.tsx (1)
CascaderRef(10-10)
tests/selector.spec.tsx (3)
examples/panel.tsx (1)
value(59-109)tests/demoOptions.ts (1)
addressOptions(38-99)tests/util.ts (1)
clickOption(61-77)
tests/loadData.spec.tsx (2)
tests/util.ts (1)
clickOption(61-77)src/hooks/useEntities.ts (1)
options(15-51)
tests/search.spec.tsx (1)
tests/util.ts (3)
doSearch(80-89)keyDown(91-100)expectOpen(3-12)
🪛 GitHub Actions: ✅ test
tests/index.spec.tsx
[error] 1115-1115: TypeScript error TS2339: Property 'style' does not exist on type 'Element' (tests/index.spec.tsx:1115,22).
🪛 GitHub Check: CodeQL
tests/checkable.spec.tsx
[notice] 5-5: Unused variable, import, function or class
Unused import selectOption.
tests/keyboard.spec.tsx
[notice] 164-164: Unused variable, import, function or class
Unused variable input.
[notice] 200-200: Unused variable, import, function or class
Unused variable input.
[notice] 238-238: Unused variable, import, function or class
Unused variable input.
[notice] 253-253: Unused variable, import, function or class
Unused variable input.
[notice] 293-293: Unused variable, import, function or class
Unused variable input.
[notice] 314-314: Unused variable, import, function or class
Unused variable input.
[notice] 343-343: Unused variable, import, function or class
Unused variable input.
🔇 Additional comments (7)
examples/rc-form.tsx (1)
2-2: LGTM!导入路径变更与 package.json 中的依赖更新保持一致(移除
rc-field-form,添加@rc-component/form)。tests/loadData.spec.tsx (1)
1-5: 测试迁移到 RTL 的实现正确。测试从 Enzyme 成功迁移到 React Testing Library,使用了正确的模式:
- 使用
render和container进行 DOM 查询- 使用
rerender更新组件- 使用
fireEvent触发事件tests/fieldNames.spec.tsx (1)
36-72: 基于 RTL 的 fieldNames 测试迁移看起来正确这里一组用例(自定义 fieldNames、defaultValue、displayRender 以及 label/value 相同场景)使用
render+ DOM 查询和clickOption/expectOpen驱动交互,验证的 value 路径与展示文本都与组件语义一致,目前看不到行为回归风险。Also applies to: 74-145
tests/search.spec.tsx (1)
47-237: 搜索相关用例迁移到 RTL 的方式合理且覆盖完整各个搜索用例统一通过
doSearch和keyDown工具驱动输入与键盘行为,使用container.querySelector(All)直接断言搜索结果文本及onSearch/onChange回调参数,包含默认搜索、changeOnSelect、排序、limit、多选等路径,整体逻辑与预期行为一致。tests/index.spec.tsx (1)
29-1106: 整体 RTL 迁移与测试工具封装保持了原有行为本文件大量用例已统一改用
render+container.querySelector(All),并通过selectOption/clickOption/isOpen/expectOpen等工具驱动交互与断言,包括默认值、高亮项、hover 展开、滚动行为、popupRender、自定义 aria/data 属性等场景,覆盖面与原 Enzyme 用例基本一致,迁移方式合理。tests/selector.spec.tsx (2)
86-136:avoid reuse用 TestComponent + 受控值重写后更贴近真实场景通过内部
TestComponent管理多选value和onChange,并用自定义tagRender渲染Tag按钮,循环点击首个按钮并断言.reuse类不存在,可以很好地覆盖“删除后不复用旧节点”的场景;同时完全使用 RTL 容器查询和事件,整体实现简洁且行为正确。
139-148: “when selected modify options” 用例的 RTL 迁移保持了原本的防回归意图该用例现在通过
clickOption(container, 0, 0)/clickOption(container, 1, 1)完成两级选择,然后使用rerender替换为只含单个isLeaf: false选项的配置,主要目的是验证已选中状态下变更options不会导致异常或崩溃;逻辑上与原 Enzyme 版本一致,没有发现问题。
| const { container } = render(<Cascader popupStyle={{ zIndex: 999 }} open />); | ||
| const dropdown = container.querySelector('.rc-cascader-dropdown'); | ||
| expect(dropdown?.style.zIndex).toBe('999'); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
修复 TS 报错:Element 上不存在属性 style
这里 querySelector 返回类型为 Element | null,直接访问 dropdown?.style.zIndex 会触发 TS2339(也是当前 pipeline 报错的原因)。可以通过泛型或断言将其收窄为 HTMLElement:
- const dropdown = container.querySelector('.rc-cascader-dropdown');
- expect(dropdown?.style.zIndex).toBe('999');
+ const dropdown = container.querySelector<HTMLElement>('.rc-cascader-dropdown');
+ expect(dropdown?.style.zIndex).toBe('999');这样既满足类型检查,又不影响现有断言逻辑。
🧰 Tools
🪛 GitHub Actions: ✅ test
[error] 1115-1115: TypeScript error TS2339: Property 'style' does not exist on type 'Element' (tests/index.spec.tsx:1115,22).
🤖 Prompt for AI Agents
In tests/index.spec.tsx around lines 1113 to 1116, TypeScript errors because
querySelector returns Element | null and Element has no typed style property;
narrow the type to HTMLElement (either by using a generic on
querySelector<HTMLDivElement> or asserting as HTMLElement after the null check)
before accessing style.zIndex, ensuring you still handle the possible null
(e.g., optional chaining or an explicit expect(dropdown).not.toBeNull() before
casting) so the assertion can remain the same.
| export function keyDown(container: HTMLElement, keyCode: number) { | ||
| const input = container.querySelector('input'); | ||
|
|
||
| const keyEvent = createEvent.keyDown(input!, { | ||
| which: keyCode, | ||
| keyCode, | ||
| }); | ||
|
|
||
| fireEvent(input!, keyEvent); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keyDown 中存在潜在的空指针风险。
doSearch 在找不到 input 时会静默返回,但 keyDown 使用了非空断言 input!,如果 input 为 null 会导致运行时错误。建议保持一致的错误处理方式。
export function keyDown(container: HTMLElement, keyCode: number) {
const input = container.querySelector('input');
+ if (!input) return;
- const keyEvent = createEvent.keyDown(input!, {
+ const keyEvent = createEvent.keyDown(input, {
which: keyCode,
keyCode,
});
- fireEvent(input!, keyEvent);
+ fireEvent(input, keyEvent);
}📝 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.
| export function keyDown(container: HTMLElement, keyCode: number) { | |
| const input = container.querySelector('input'); | |
| const keyEvent = createEvent.keyDown(input!, { | |
| which: keyCode, | |
| keyCode, | |
| }); | |
| fireEvent(input!, keyEvent); | |
| } | |
| export function keyDown(container: HTMLElement, keyCode: number) { | |
| const input = container.querySelector('input'); | |
| if (!input) return; | |
| const keyEvent = createEvent.keyDown(input, { | |
| which: keyCode, | |
| keyCode, | |
| }); | |
| fireEvent(input, keyEvent); | |
| } |
🤖 Prompt for AI Agents
In tests/util.ts around lines 91 to 100, keyDown uses the non-null assertion
input! which can throw if querySelector returns null; change it to mirror
doSearch's behavior by guarding for a missing input: if there is no input,
return early (silently) and do not call createEvent.keyDown or fireEvent,
otherwise proceed using the existing input variable without non-null assertions.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #635 +/- ##
==========================================
- Coverage 99.52% 99.39% -0.14%
==========================================
Files 24 23 -1
Lines 635 662 +27
Branches 194 193 -1
==========================================
+ Hits 632 658 +26
- Misses 3 4 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
69-71: Remove Enzyme packages—test migration to React Testing Library is complete but cleanup is incompleteEnzyme and its packages are installed and initialized in
tests/setup.js(lines 7-10), but the actual test files have already been migrated to React Testing Library. Theenzyme-adapter-react-16adapter is incompatible with React 18 (which is yourpeerDependencyrequirement). Since all test files now use@testing-library/react, these packages should be removed fromdevDependencies:
enzymeenzyme-adapter-react-16enzyme-to-jsonAlso remove the associated setup files (
tests/setup.jsEnzyme configuration andtests/enzyme.tswrapper) and the@types/enzymepackage.
♻️ Duplicate comments (2)
package.json (1)
56-56: @testing-library/react v12 is incompatible with React 18 and must be upgraded@testing-library/react v12 does not support React 18. React 18 compatibility requires v13 or later. Upgrade to at least v13, or preferably to v16.3.0 (latest stable version):
- "@testing-library/react": "^12.1.5", + "@testing-library/react": "^16.3.0",Also verify that related testing-library packages (@testing-library/dom, @testing-library/user-event) are updated to compatible versions.
tests/index.spec.tsx (1)
1113-1116: 修复 TypeScript 错误:Element类型上不存在style属性
querySelector返回Element | null,而style属性仅存在于HTMLElement上。需要使用泛型指定返回类型。- const dropdown = container.querySelector('.rc-cascader-dropdown'); - expect(dropdown).toHaveStyle({ zIndex: '999' }); + const dropdown = container.querySelector<HTMLElement>('.rc-cascader-dropdown'); + expect(dropdown).toHaveStyle({ zIndex: '999' });或者使用 RTL 推荐的
toHaveStylematcher(如上所示),它可以更好地处理样式断言。
🧹 Nitpick comments (3)
tests/checkable.spec.tsx (1)
278-280: 建议使用 container 作用域查询此处使用
document.querySelector而非container.querySelector,虽然功能上可行,但与文件中其他测试的 RTL 模式不一致。如果下拉菜单是 portal 渲染到 body,这种用法是合理的;否则建议统一使用 container 作用域查询。- fireEvent.click( - document.querySelector('[data-path-key="China"] .rc-cascader-checkbox') as HTMLElement, - ); + fireEvent.click( + container.querySelector('[data-path-key="China"] .rc-cascader-checkbox') as HTMLElement, + );tests/keyboard.spec.tsx (2)
40-47: 定时器推进值过大
jest.advanceTimersByTime(100000)推进了 100 秒,这远超过典型的动画或防抖延迟(通常 < 1000ms)。虽然功能上正确,但建议使用jest.runAllTimers()来替代,代码更简洁且意图更明确。- jest.advanceTimersByTime(100000); - expect(isOpen(container)).toBeTruthy(); + jest.runAllTimers(); + expect(isOpen(container)).toBeTruthy();
368-379: 跳过的测试需要跟进此测试被标记为
it.skip并带有 TODO 注释。建议创建 issue 跟踪此项,以免遗忘。是否需要我帮助创建一个 issue 来跟踪这个被跳过的测试用例?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.json(2 hunks)tests/checkable.spec.tsx(7 hunks)tests/index.spec.tsx(34 hunks)tests/keyboard.spec.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/checkable.spec.tsx (1)
tests/demoOptions.ts (1)
addressOptions(38-99)
tests/keyboard.spec.tsx (2)
tests/demoOptions.ts (1)
addressOptions(38-99)tests/util.ts (2)
keyDown(91-100)isOpen(42-46)
🔇 Additional comments (7)
package.json (2)
81-82: 验证:React 18 升级与测试依赖项的兼容性React 版本已升级至 18.0.0,但测试框架和相关依赖项需要全面审查以确保兼容性。请确认:
- 所有测试已从 Enzyme 迁移到 React Testing Library
@testing-library/react已更新至与 React 18 兼容的版本(v13+)package-lock.json或yarn.lock已通过npm install或yarn install重新生成- 完整的测试套件在本地和 CI 中执行通过
38-38: 批准:其他依赖项更新lint 脚本、RC 组件库版本和
@rc-component/form的添加看起来是合理的。确认这些更新与测试框架迁移工作一致。Also applies to: 46-48, 53-53
tests/index.spec.tsx (1)
1-10: 测试迁移到 RTL 模式完成良好导入结构清晰,正确引入了 RTL 工具(
render、fireEvent、act)和测试辅助函数(expectOpen、selectOption、isOpen、clickOption)。迁移遵循了 RTL 最佳实践。tests/checkable.spec.tsx (2)
1-5: 导入清理完成导入语句干净整洁,仅引入了实际使用的工具函数
clickOption。RTL 迁移正确。
35-108: 测试逻辑正确,RTL 迁移完善
customize测试正确地混合使用了两种交互方式:
fireEvent.click(checkboxes[0])直接点击复选框元素clickOption(container, menuIndex, itemIndex)点击菜单项这两种方式在 checkable 模式下都能触发选中状态变化,测试覆盖了不同的用户交互路径。
tests/keyboard.spec.tsx (2)
1-7: 导入和测试结构正确RTL 导入和测试工具函数(
keyDown、isOpen)导入正确。测试结构遵循了标准的 RTL 模式。
51-109: 键盘导航测试覆盖全面此测试完整覆盖了键盘导航的各种场景:向下/向左/向右箭头键、无效按键处理、以及最终选择确认。RTL 迁移正确,断言逻辑清晰。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/keyboard.spec.tsx (3)
18-25: 统一封装定时器推进逻辑可以减重复、去除魔法数本文件多处使用:
act(() => { jest.advanceTimersByTime(100000); });建议在
tests/util.ts抽一个类似flushCascaderTimers()的工具(内部用act(() => jest.runAllTimers())或封装统一的延时),这里直接调用,可读性更好,也避免对具体延时值的硬编码。Also applies to: 41-45
119-166: 搜索相关用例目前依赖同步实现,后续如改为 debounce 需补充定时器/异步断言在开启 fake timers 的前提下,这几组搜索用例通过
fireEvent.change+keyDown直接断言选中项或下拉是否打开,没有调用advanceTimersByTime或 RTL 的异步查询 API。只要搜索逻辑是完全同步的现在就没问题,但如果后面在搜索里引入 debounce / setTimeout,这些用例会变得脆弱甚至假阳性。可以考虑:
- 在触发搜索后显式
act(() => jest.runAllTimers()),或- 使用
await findByText/waitFor等异步断言方式,使测试对实现细节不那么敏感。另外,“should not switch column when press left/right key in search input” 语义上是“在输入框里按方向键”,请确认
keyDown工具内部确实是对 input 触发事件(而不是容器),否则可以直接把input传给keyDown以让测试语义更直观。Also applies to: 381-403
405-419: 针对 children 自行处理 onKeyDown 的 skip 用例建议后续用 issue/TODO 跟进用例已迁到 RTL,但仍然是
it.skip,上面的 TODO 也标明语义有些“奇怪”。短期保持 skip 可以接受,不过建议:
- 要么在 issue 中明确记录预期行为与当前实现的差异,
- 要么在确定需求后补全断言并恢复为正常用例。
如果需要,我可以帮忙基于当前实现设计一个更明确的断言集合。
package.json (1)
38-38: lint 脚本中的扩展名列表有一个重复项
"--ext .tsx,.ts,.jsx,.jsx"中.jsx出现了两次,功能上没有问题,但可以顺手去重为一次,让脚本更干净一些。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
jest.config.js(0 hunks)package.json(2 hunks)tests/enzyme.ts(0 hunks)tests/keyboard.spec.tsx(4 hunks)tests/setup.js(0 hunks)tests/util.ts(2 hunks)
💤 Files with no reviewable changes (3)
- jest.config.js
- tests/enzyme.ts
- tests/setup.js
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/util.ts
🔇 Additional comments (6)
tests/keyboard.spec.tsx (4)
27-53: space/enter 打开/关闭用例迁移到 RTL 的方式合理参数化
['space', KeyCode.SPACE]/['enter', KeyCode.ENTER],每个用例独立render,用keyDown + isOpen断言打开/关闭,下游行为与组件语义一致,看不出问题。
55-307: 核心键盘导航与菜单开合路径覆盖充分这里一组用例覆盖了:
- 普通键盘导航(DOWN/RIGHT/LEFT/UP)、非处理按键(QUESTION_MARK)、
- 选中路径、changeOnSelect、忽略 disabled 项、
- 以及 LEFT/BACKSPACE/ESC 等关闭菜单行为和 RTL 场景,
并通过选中值与
addressOptions的对比来验证逻辑,与组件预期交互一致,迁移到基于 DOM 的断言后整体看起来是等价且更贴近真实使用的。
309-333: onKeyDown 回调用例很好地防止内部逻辑吞掉事件这里通过 DOWN / ESC / ENTER 各触发一次
keyDown并断言onKeyDown被调用 3 次,既验证了组件内部键盘处理逻辑,又保证不会阻断用户传入的onKeyDown,用例设计合理。
363-379: “全部 disabled 不应崩溃” 用例有效保护极端配置场景将所有选项都打上
disabled,反复按 DOWN 并断言没有 active item,可以很好地防止实现对这种极端配置处理不当(如越界索引或死循环),这个回归用例保留得很好。package.json (2)
46-49: 运行时依赖与开发依赖整体对齐到最新 rc-component 与 React 19 生态,看起来合理
@rc-component/select/tree/util小版本升级,同时 devDependencies 中 React/ReactDOM 升到^19.0.0,并配套更新@testing-library/react与 React 类型定义,测试和声明的支持范围基本一致,这部分改动本身没有明显问题,交给 CI 验证即可。Also applies to: 53-61, 77-79
3-3: React 18 peerDependencies require version alignment and documentationCurrent peerDependencies require React >=18.0.0, which represents a significant compatibility change from prior React versions. If this reflects an upgrade from React 16/17 support, it constitutes a breaking change that should be addressed:
- Version should be bumped to 2.0.0+ to reflect semantic versioning for breaking changes, or
- Ensure this is clearly documented in CHANGELOG and release notes explaining the React 16/17 support removal
Currently, there is no CHANGELOG entry documenting this requirement change for version 1.8.0, and the version number does not reflect a major version increment appropriate for dropping support for older React versions.
Summary by CodeRabbit
Chores
Tests
Examples
✏️ Tip: You can customize this high-level summary in your review settings.