-
-
Notifications
You must be signed in to change notification settings - Fork 947
refactor(Sender): Slot sender refactor #1515
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
base: main
Are you sure you want to change the base?
Conversation
|
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. 📝 WalkthroughWalkthrough本 PR 在 Sender 子系统引入并集成三款新 Hook(useCursor、useSlotBuilder、useSlotConfigState),重构 SlotTextArea 的光标/插槽/插入逻辑并将 Changes
Sequence Diagram(s)%%{init: {"themeVariables":{"actorBorder":"#2b6cb0","actorBackground":"#e6f2ff","noteBackground":"#fff4e6"}}}%%
sequenceDiagram
participant UI as SlotTextArea
participant Config as useSlotConfigState
participant Builder as useSlotBuilder
participant Cursor as useCursor
participant DOM as Editor/DOM
UI->>Config: 初始化 / mergeSlotConfig(...)
UI->>Builder: 请求构建节点 (buildSlotSpan / buildEditSlotSpan / buildSpaceSpan)
Builder-->>DOM: 返回 HTMLSpanElement 并插入 DOM
Note over DOM,UI: 用户交互(输入/插入/删除/粘贴)
DOM->>UI: 事件触发 (input / keydown / paste / click)
UI->>Config: getNodeInfo(目标节点)
Config-->>UI: 返回 NodeInfo
UI->>Cursor: 调用光标 API (setCursorPosition / setSlotFocus / setAfterNodeFocus, 含 preventScroll)
Cursor->>DOM: 操作 Selection/Range 并 focus(尊重 preventScroll)
Cursor-->>UI: 返回位置/确认
Estimated code review effort🎯 4 (复杂) | ⏱️ ~60–90 分钟
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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 |
Summary of ChangesHello @kimteayon, 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 primarily focuses on a substantial refactoring of the 'Sender' component, specifically its 'SlotTextArea' functionality. The main objective is to enhance modularity and maintainability by extracting intricate cursor manipulation and DOM element construction into new, reusable React hooks. Additionally, the PR introduces a standardized warning utility across several components and includes minor code style optimizations to improve readability. 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
|
Deploying ant-design-x with
|
| Latest commit: |
ae4d14c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d4d5016a.ant-design-x.pages.dev |
| Branch Preview URL: | https://slot-sender-refactor.ant-design-x.pages.dev |
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 is a significant and well-executed refactoring of the Sender component's slot functionality. By extracting complex logic for cursor management, DOM element building, and state management into dedicated custom hooks (useCursor, useSlotBuilder, useSlotConfigState), you've greatly improved the structure, readability, and maintainability of the SlotTextArea component. My review identifies a few potential bugs introduced during this refactoring, including issues with cursor positioning, node insertion order, and incorrect data attribute access. Addressing these points will help ensure the new implementation is robust.
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/x/components/sender/hooks/use-input-height.ts (1)
11-18: 高度计算对lineHeight的 NaN/单位场景不健壮(normal、px字符串会导致错误)当前逻辑把
styles.lineHeight/token.lineHeight统一parseFloat后再* fontSize,在lineHeight: "20px"时会把20px当“倍数”导致高度被放大;在lineHeight: "normal"时可能产生NaN,进而让minHeight/maxHeight变成NaN。建议显式区分“px”与“无单位倍数”,并为normal/NaN提供兜底。- const lineHeight = parseFloat(`${styles.lineHeight || token.lineHeight}`); - const fontSize = parseFloat(`${computedStyle?.fontSize || styles.fontSize || token.fontSize}`); - const height = computedStyle?.lineHeight - ? parseFloat(`${computedStyle?.lineHeight}`) - : lineHeight * fontSize; + const fontSize = parseFloat(`${computedStyle?.fontSize || styles.fontSize || token.fontSize}`); + + const resolveLineHeightPx = (): number => { + const lh = computedStyle?.lineHeight; + // 1) 优先用浏览器计算后的 px(避免单位/normal 问题) + if (lh && lh !== 'normal') { + const n = parseFloat(String(lh)); + if (Number.isFinite(n)) return n; + } + + // 2) 回退到传入 styles/token:区分 px 字符串与无单位倍数(number) + const candidate = styles.lineHeight ?? token.lineHeight; + if (typeof candidate === 'number' && Number.isFinite(candidate)) { + return candidate * fontSize; // React: number lineHeight 语义为倍数 + } + if (typeof candidate === 'string') { + const trimmed = candidate.trim(); + if (trimmed.endsWith('px')) { + const n = parseFloat(trimmed); + if (Number.isFinite(n)) return n; + } + // 其它字符串(如 "normal")兜底 + } + + // 3) 最终兜底(接近浏览器 normal 的常见实现) + return 1.2 * fontSize; + }; + + const height = resolveLineHeightPx();Also applies to: 28-36
packages/x/components/sender/SlotTextArea.tsx (1)
173-181: content slot 写入 innerHTML:需要明确是否允许 HTML;否则是 XSS 面
slotSpan.innerHTML = value如果 value 来自用户输入/外部数据,会引入 XSS 风险。若不需要富文本,建议改textContent;若需要,至少做白名单/消毒(例如 DOMPurify)。case 'content': - slotSpan.innerHTML = value || ''; + slotSpan.textContent = value || ''; slotSpan.setAttribute('data-placeholder', node.props?.placeholder || ''); return null;
🧹 Nitpick comments (1)
packages/x/components/sender/hooks/use-cursor.ts (1)
15-44: 类型建议放宽:API 实际会对 span/HTMLElement 生效当前签名大量限定
HTMLDivElement(如setCursorPosition/setAfterNodeFocus),但调用方会传HTMLSpanElement(slot/skill)。建议把入参类型收敛到HTMLElement | Node,避免到处as HTMLDivElement。Also applies to: 189-249
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/x/components/attachments/FileList/index.tsx(1 hunks)packages/x/components/code-highlighter/CodeHighlighter.tsx(1 hunks)packages/x/components/file-card/List.tsx(1 hunks)packages/x/components/mermaid/Mermaid.tsx(3 hunks)packages/x/components/notification/index.tsx(2 hunks)packages/x/components/sender/SlotTextArea.tsx(9 hunks)packages/x/components/sender/hooks/use-cursor.ts(1 hunks)packages/x/components/sender/hooks/use-input-height.ts(1 hunks)packages/x/components/sender/hooks/use-slot-builder.ts(1 hunks)packages/x/components/sender/hooks/use-slot-config-state.ts(3 hunks)packages/x/components/suggestion/index.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/x/components/sender/SlotTextArea.tsx (2)
packages/x/components/sender/index.tsx (1)
SlotConfigType(36-36)packages/x/components/sender/interface.ts (4)
SlotConfigType(113-119)SkillType(54-65)InsertPosition(13-13)EventType(121-123)
🪛 ast-grep (0.40.0)
packages/x/components/sender/hooks/use-slot-builder.ts
[warning] 76-76: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: span.innerHTML = ' '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 76-76: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: span.innerHTML = ' '
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
⏰ 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). (4)
- GitHub Check: build preview
- GitHub Check: test
- GitHub Check: size
- GitHub Check: test
🔇 Additional comments (12)
packages/x/components/sender/hooks/use-input-height.ts (1)
5-10: 显式标注返回类型是加分项,但确认是否想把该 Hook 输出“钉死”为CSSProperties
): React.CSSProperties能让下游类型更稳定、也更符合“样式片段”的语义;如果未来可能返回undefined/null(例如交由上层合并策略处理),这里会变成破坏性约束,需要提前确认方向。packages/x/components/code-highlighter/CodeHighlighter.tsx (1)
69-69: LGTM!将
clsx调用格式化为单行,保持了与 PR 其他文件的一致性,且未改变任何功能行为。packages/x/components/file-card/List.tsx (1)
63-65: LGTM!
mergedCls的clsx调用重构为单行格式,参数与条件对象均保持不变,代码更简洁。packages/x/components/suggestion/index.tsx (2)
1-1: LGTM!将
useEvent和useMergedState从@rc-component/util统一导入,符合导入语句组织的最佳实践。
174-176: LGTM!
rootClassName的clsx调用重构为单行格式,与 PR 中其他组件保持一致的代码风格。packages/x/components/attachments/FileList/index.tsx (1)
157-157: LGTM!Button 的
className属性格式化为单行clsx调用,保持代码风格一致性。packages/x/components/mermaid/Mermaid.tsx (3)
9-9: LGTM!引入集中式的
warning工具函数,与 PR 中其他组件(如 notification)的错误处理方式保持一致。
104-106: 确认warning用于渲染错误的适用性
warning工具通常用于开发时的 API 弃用警告或使用不当提示。对于运行时的渲染失败错误,使用console.error可能更为合适,因为这是真正的运行时错误而非开发提示。建议确认此处使用
warning是否为项目的既定规范。如果是,则可以保留当前实现。
315-315: LGTM!
className的clsx调用重构为单行格式,与 PR 其他文件保持一致的代码风格。packages/x/components/notification/index.tsx (1)
2-16: warning 工具替换 console.warn:方向正确,但要确认条件语义一致这里传入
warning(XNotification.permissible, ...)依赖warning的实现是“条件为 false 才输出”。如果warning语义相反,会导致在支持 Notification 的环境也持续告警。建议快速对齐../_util/warning的签名语义(与历史 console.warn 触发条件一致)。packages/x/components/sender/hooks/use-slot-builder.ts (1)
17-35: slot/skill/span 构建逻辑抽离:可维护性提升builder API(skill / edit-slot / slot)拆分清晰,依赖项 useCallback 也合理。
Also applies to: 40-50, 55-65
packages/x/components/sender/SlotTextArea.tsx (1)
12-132: useCursor/useSlotBuilder/useSlotConfigState 的解耦改造:整体可读性更好编辑器内的 DOM 构造与光标管理抽到 hooks 后,SlotTextArea 复杂度明显下降;focus/insert 也把 preventScroll 贯穿进来了。
Also applies to: 258-272, 777-798
size-limit report 📦
|
Bundle ReportChanges will increase total bundle size by 1.51MB (49.5%) ⬆️
Affected Assets, Files, and Routes:view changes for bundle: x-markdown-array-pushAssets Changed:
view changes for bundle: antdx-array-pushAssets Changed:
|
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: 2
🧹 Nitpick comments (1)
packages/x/components/sender/__tests__/slot.test.tsx (1)
200-219: 建议验证关闭回调是否被调用测试定义了
mockClose回调函数但没有验证它是否被调用。建议添加对关闭按钮的点击测试,确认onClose回调被正确触发。可以添加如下验证:
await waitFor(() => { expect(container.querySelector('#ant-sender-slot-placeholders')).toBeInTheDocument(); }); + + const closeBtn = container.querySelector('[aria-label="Close"]') as HTMLElement; + if (closeBtn) { + fireEvent.click(closeBtn); + expect(mockClose).toHaveBeenCalled(); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/x/components/mermaid/__tests__/index.test.tsx(2 hunks)packages/x/components/sender/__tests__/slot.test.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-04T03:46:13.221Z
Learnt from: YumoImer
Repo: ant-design/x PR: 293
File: components/x-request/demo/custom-transformer.tsx:75-79
Timestamp: 2024-12-04T03:46:13.221Z
Learning: When reviewing code examples in `components/x-request/demo/custom-transformer.tsx` that use `mockFetch`, it's unnecessary to suggest enhancements to the `TransformStream` implementation, such as adding error handling and data validation, since the code is intended as a simple demonstration.
Applied to files:
packages/x/components/mermaid/__tests__/index.test.tsx
⏰ 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). (5)
- GitHub Check: build preview
- GitHub Check: test
- GitHub Check: size
- GitHub Check: test
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
packages/x/components/mermaid/__tests__/index.test.tsx (2)
85-85: 测试更新与实现变更一致!将测试从监听
console.warn改为console.error,并更新预期错误消息格式为[antdx: Mermaid] Render failed,这与 Mermaid.tsx 中使用warning工具函数的实现变更保持一致。Also applies to: 90-92
423-423: 错误处理测试正确对齐实现!将渲染错误测试从监听
console.warn改为console.error,并更新预期错误消息以包含完整的错误信息,这与使用warning工具函数的实现变更一致。Also applies to: 428-430
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 (2)
packages/x/components/sender/demo/slot-filling.tsx (1)
148-154: 建议调整插槽键名顺序和占位符文本以提高演示清晰度当前实现在插入操作中存在以下可改进之处:
键名顺序不一致:第一个插槽使用
partner_2,第二个使用partner_1,这可能会让阅读代码的开发者感到困惑。建议按照插入顺序命名(如partner_1在前,partner_2在后)。占位符文本相同:两个不同类型的插槽(
input和content)使用了相同的占位符文本 "Enter a name",这可能无法很好地展示它们的区别。建议为content类型的插槽使用不同的占位符,以便更清晰地演示各自的功能特点。演示意图确认:从单个插槽扩展到两个插槽的变更应该是有意为之,用于展示批量插入功能。如果这是预期行为,建议在注释中说明此演示的目的。
应用以下差异来改进演示代码的可读性:
onClick={() => { senderRef.current?.insert?.([ { type: 'input', - key: `partner_2_${Date.now()}`, + key: `partner_1_${Date.now()}`, props: { defaultValue: '1111', placeholder: 'Enter a name' }, }, { type: 'content', - key: `partner_1_${Date.now()}`, - props: { placeholder: 'Enter a name' }, + key: `partner_2_${Date.now()}`, + props: { placeholder: 'Enter content' }, }, ]); }}packages/x/components/sender/hooks/use-slot-config-state.ts (1)
86-89: 可选:明确 exhaustive-deps 意图useEffect 的依赖数组中未包含
setSlotValuesBySlotConfig,ESLint 的 exhaustive-deps 规则可能会发出警告。虽然该函数使用空依赖数组的 useCallback 包装因此是稳定的,但为了提高代码可读性,可以考虑添加注释说明或显式添加到依赖数组中。useEffect(() => { if (!slotConfig) return; setSlotValuesBySlotConfig(slotConfig); - }, [slotConfig]); + }, [slotConfig, setSlotValuesBySlotConfig]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/x/components/sender/SlotTextArea.tsx(9 hunks)packages/x/components/sender/demo/slot-filling.tsx(1 hunks)packages/x/components/sender/hooks/use-cursor.ts(1 hunks)packages/x/components/sender/hooks/use-slot-builder.ts(1 hunks)packages/x/components/sender/hooks/use-slot-config-state.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/x/components/sender/hooks/use-slot-builder.ts
- packages/x/components/sender/hooks/use-cursor.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/x/components/sender/SlotTextArea.tsx (1)
packages/x/components/sender/interface.ts (4)
SlotConfigType(113-119)SkillType(54-65)InsertPosition(13-13)EventType(121-123)
⏰ 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). (5)
- GitHub Check: build preview
- GitHub Check: size
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Cloudflare Pages
🔇 Additional comments (9)
packages/x/components/sender/hooks/use-slot-config-state.ts (2)
106-113: LGTM!重构逻辑清晰通过集中构建新的 slotConfigMap 和 slotValues,然后统一更新 ref 和 state,逻辑清晰且避免了中间状态不一致的问题。
115-134: LGTM!增量更新逻辑正确正确地将新的 slot 配置合并到现有配置中,同时保持 state 和 stateRef 的同步更新。
packages/x/components/sender/SlotTextArea.tsx (7)
108-109: LGTM!正确集成重构后的 hook正确解构了新的 API 方法,使得组件可以更方便地访问 slot 配置和节点信息。
112-125: LGTM!光标管理模块化将光标管理逻辑提取到 useCursor hook 中,提高了代码的可维护性和可测试性。传递 slotConfigMap 作为依赖是合理的。
293-340: LGTM!节点文本提取逻辑改进通过使用 getNodeInfo 统一获取节点元数据,代码更加一致和可维护。正确处理了 nbsp 节点、slot 值格式化(formatResult)以及 skill 节点的特殊逻辑。
342-410: LGTM!性能优化合理通过预分配数组和使用索引追踪,减少了数组动态增长的开销。配合 getNodeInfo 实现了更清晰的节点处理逻辑。
426-472: LGTM!插入位置逻辑增强函数现在返回更丰富的信息(包括 selection),并且通过早期返回和清晰的类型判断提高了代码可读性和健壮性。
699-775: LGTM!之前的问题已全部修复本次重构正确解决了之前评审中的所有问题:
- Line 707:使用
addSlotValuesBySlotConfig正确处理 slot 配置,避免了数组展开的错误- Line 751:
startOffset >= 0修复了偏移量为 0 时被误判为 false 的问题- Lines 760-762:反转 slotNode 数组确保了正确的视觉顺序
- preventScroll 参数正确传递到光标设置函数
777-798: LGTM!focus 实现更加清晰通过使用 useCursor hook 提供的方法,focus 函数的实现更加简洁和一致,每个分支都正确处理了 preventScroll 选项。
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: 1
♻️ Duplicate comments (2)
packages/x/components/sender/__tests__/slot.test.tsx (2)
239-247: 修正控制台警告的监听方式测试监听
console.error但实现中使用了warning()工具函数(来自@rc-component/util),该函数调用console.warn而非console.error。应改为监听console.warn以确保测试能正确捕获警告。应用此 diff 修正:
- const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); const invalidConfig = [{ type: 'input', props: { placeholder: 'No key' } } as SlotConfigType]; render(<Sender slotConfig={invalidConfig} />); expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Slot key is missing')); consoleSpy.mockRestore();
345-354: 测试逻辑不完整:缺少 submitDisabled 属性测试名称为"应该处理submitDisabled状态",但实际渲染的组件没有传入
submitDisabled属性。要正确测试此功能,应该传入submitDisabled={true}。应用此 diff 修正:
it('应该处理submitDisabled状态', () => { const onSubmit = jest.fn(); const { container } = render( - <Sender slotConfig={baseSlotConfig} onSubmit={onSubmit} submitType="enter" />, + <Sender slotConfig={baseSlotConfig} onSubmit={onSubmit} submitType="enter" submitDisabled />, ); const inputArea = container.querySelector('[role="textbox"]') as HTMLElement; fireEvent.keyDown(inputArea, { key: 'Enter' }); expect(onSubmit).not.toHaveBeenCalled(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/x/components/sender/SlotTextArea.tsx(11 hunks)packages/x/components/sender/__tests__/slot.test.tsx(3 hunks)packages/x/components/sender/demo/slot-filling.tsx(1 hunks)packages/x/components/sender/hooks/use-slot-config-state.ts(1 hunks)packages/x/components/sender/interface.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/x/components/sender/demo/slot-filling.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/x/components/sender/hooks/use-slot-config-state.ts (1)
packages/x/components/sender/interface.ts (1)
SlotConfigType(113-119)
packages/x/components/sender/SlotTextArea.tsx (1)
packages/x/components/sender/interface.ts (5)
SlotConfigType(113-119)SkillType(54-65)InsertPosition(13-13)SlotConfigBaseType(33-36)EventType(121-123)
⏰ 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). (5)
- GitHub Check: size
- GitHub Check: test
- GitHub Check: build preview
- GitHub Check: test
- GitHub Check: Cloudflare Pages
🔇 Additional comments (18)
packages/x/components/sender/interface.ts (1)
33-36: 导出接口符合重构目标将
SlotConfigBaseType公开导出是合理的,这允许其他模块(如 SlotTextArea 和相关 hooks)共享此类型定义,符合代码重构和模块化的最佳实践。packages/x/components/sender/__tests__/slot.test.tsx (3)
5-58: DOM API mock 设置完善全面模拟了
window.getSelection和document.createRangeAPI,为测试 contenteditable 编辑器提供了必要的环境。在beforeEach中设置确保了每个测试的独立性。
61-89: 测试 fixture 重构提升可维护性将 slot 配置整合为
baseSlotConfig避免了重复代码,统一的占位符文本和标签使测试更易于理解和维护。
91-344: 测试覆盖全面且组织良好测试套件按功能分组(核心功能、交互、技能、边界条件、高覆盖率),涵盖了各种场景包括 ref 接口、键盘事件、粘贴处理、disabled/readOnly 状态以及 autoSize 配置。测试结构清晰,易于维护。
packages/x/components/sender/hooks/use-slot-config-state.ts (4)
4-16: 类型定义清晰合理
NodeInfo接口和SlotValues类型定义明确,SUPPORTED_INPUT_TYPES常量正确识别了需要处理 defaultValue 的输入类型 slot。
21-33: 默认值构建逻辑正确
buildSlotValues函数正确处理不同 slot 类型的默认值提取,对输入类型使用defaultValue,对其他类型使用value或label,并提供空字符串作为兜底。
50-66: 节点信息提取逻辑正确
getNodeInfoBySlotConfigMap函数正确从 DOM 元素的 dataset 属性中提取节点信息,使用dataset.placeholder获取占位符(过去的审查已修正),并从 Map 中查找对应的 slot 配置。
68-124: Hook API 设计清晰
useSlotConfigState返回的 API 设计良好,提供了getSlotValues、setSlotValues、mergeSlotConfig和getNodeInfo方法,便于外部调用。状态管理使用 ref 避免了不必要的重渲染。packages/x/components/sender/SlotTextArea.tsx (10)
12-22: 依赖项导入符合重构目标新增的
useCursor和useSlotBuilderhooks 导入体现了代码职责分离,SlotConfigBaseType的导入与接口文件的公开导出相匹配。
28-33: insert 方法签名增强合理新增
preventScroll可选参数允许调用者控制插入时的滚动行为,提升了 API 的灵活性和用户体验控制能力。
119-137: Hook 集成实现关注点分离
useCursor和useSlotBuilderhooks 的使用将光标管理和 slot 构建逻辑从组件中抽离,提高了代码的可维护性和可测试性。
264-296: 警告信息标准化改进使用
warning工具函数替代直接的 console 调用,并在消息中包含组件名称 'Sender',提供了更好的调试信息。
299-346: 节点文本提取逻辑重构合理使用
getNodeInfo统一获取节点信息,避免了直接访问 dataset 的分散逻辑。正确处理了 slot 节点的formatResult、skill 节点的文本提取以及 nbsp 空格节点。
348-416: 编辑器值获取逻辑优化重构后的
getEditorValue使用预分配数组批量组装文本,减少了中间字符串拼接的性能开销。单次遍历子节点并使用getNodeInfo统一处理,逻辑更清晰高效。
432-479: 插入位置判断逻辑增强
getInsertPosition新增了对显式 position 参数的支持,并返回更丰富的上下文信息(插入类型、slot类型、selection快照),为调用者提供了更灵活的插入控制能力。
560-670: 键盘事件处理重构提升可读性将键盘事件处理分解为
shouldSkipKeyHandling、handleBackspaceKey、shouldSubmitForm和handleSkillAreaKeyEvent等独立函数,每个函数职责单一,显著提升了代码的可读性和可维护性。
740-818: insert 方法重构逻辑正确重构后的
insert方法正确使用了新的 cursor API(setCursorPosition、setAfterNodeFocus),startOffset >= 0检查修正了过去的边界条件问题,反转slotNode数组确保了正确的视觉顺序,preventScroll参数正确传递到底层 API。
820-841: focus 方法重构实现清晰
focus方法将光标定位逻辑委托给useCursorhook 提供的专用方法(setSlotFocus、setStartCursor、setAllSelectCursor、setEndCursor),每种光标类型都有明确的处理路径,preventScroll选项得到正确传递。
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1515 +/- ##
==========================================
- Coverage 94.69% 94.57% -0.13%
==========================================
Files 137 139 +2
Lines 3960 4163 +203
Branches 1123 1173 +50
==========================================
+ Hits 3750 3937 +187
- Misses 207 223 +16
Partials 3 3 ☔ 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
♻️ Duplicate comments (1)
packages/x/components/sender/hooks/use-slot-config-state.ts (1)
100-110: mergeSlotConfig 缺少 Map 清理,会残留过期配置
mergeSlotConfig函数在更新配置映射时没有先清空slotConfigMapRef.current,导致当新的slotConfig中移除了某些 key 时,这些 key 仍会保留在 Map 中。后续通过getNodeInfo查询时可能会命中这些过期的配置项,引发错误。虽然
useEffect(第 87 行)中正确地进行了清理,但mergeSlotConfig作为另一个更新入口点,同样需要清理逻辑。建议在更新前清空 Map:
const mergeSlotConfig = useCallback((newSlotConfig: SlotConfigType[]) => { const newValues = buildSlotValues(newSlotConfig); // 更新配置映射 + slotConfigMapRef.current.clear(); newSlotConfig.forEach((node) => { if (node.key) slotConfigMapRef.current.set(node.key, node); }); _setState((prev) => ({ ...prev, ...newValues })); stateRef.current = { ...stateRef.current, ...newValues }; }, []);
🧹 Nitpick comments (2)
packages/x/components/sender/hooks/use-slot-config-state.ts (2)
21-33: 默认值提取逻辑正确
buildSlotValues函数根据不同的 slot 类型正确提取默认值,逻辑清晰。第 26 行的(node as any).props类型断言虽然不够严格,但考虑到SlotConfigType是复杂的联合类型,这种处理方式可以接受。如果后续有时间,可以考虑为不同的 SlotConfigType 子类型定义更精确的类型守卫,以避免
any断言:const getPropsFromNode = (node: SlotConfigType): Record<string, any> => { if ('props' in node) return node.props || {}; return {}; };
117-157: 文本值提取逻辑全面,考虑了各种边界情况
getNodeTextValue函数正确处理了文本节点、元素节点、slot 节点等多种场景,支持formatResult自定义格式化,逻辑严谨。函数长度较长(约 40 行),如果后续维护时觉得复杂度较高,可以考虑拆分为更小的辅助函数以提升可读性,例如:
const getTextForTextNode = (node: Node): string => node.textContent || ''; const getTextForSlotNode = ( element: HTMLElement, nodeInfo: NodeInfo, stateRef: React.MutableRefObject<SlotValues> ): string => { const { slotKey, nodeType, slotConfig } = nodeInfo; if (nodeType === 'nbsp') return ' '; if (!slotConfig || slotConfig.type === 'content') { return element.innerText || ''; } const slotValue = stateRef.current[slotKey!] ?? ''; return slotConfig.formatResult?.(slotValue) ?? slotValue; };不过当前实现已足够清晰,这只是可选的重构建议。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/x/components/sender/__tests__/__snapshots__/demo-extend.test.ts.snapis excluded by!**/*.snappackages/x/components/sender/__tests__/__snapshots__/demo.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
packages/x/components/sender/hooks/use-slot-config-state.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
🧬 Code graph analysis (1)
packages/x/components/sender/hooks/use-slot-config-state.ts (1)
packages/x/components/sender/interface.ts (1)
SlotConfigType(113-119)
⏰ 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). (5)
- GitHub Check: build preview
- GitHub Check: size
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
packages/x/components/sender/hooks/use-slot-config-state.ts (5)
1-16: 类型定义和常量设计合理引入的
NodeInfo接口、SlotValues类型以及SUPPORTED_INPUT_TYPES常量设计清晰,使用Set进行类型检查是良好的实践。导入路径的调整也符合项目结构重构。
50-66: 节点信息提取实现正确
getNodeInfoBySlotConfigMap函数正确地从 DOM 元素的dataset中提取各项配置信息。第 60 行的placeholder: dataset.placeholder已修正(之前评审中指出的dataset.dataset拼写错误已解决)。
84-92: 初始化逻辑正确且完整
useEffect中正确地在重建配置映射前清空了 Map(第 87 行),避免残留过期数据。状态和 ref 的同步更新也处理得当。
112-115: getNodeInfo 回调实现正确使用
useCallback包装,依赖数组为空,确保回调稳定性的同时能访问最新的slotConfigMapRef.current。实现合理。
159-169: Hook API 设计合理返回的元组结构清晰,第一个元素提供配置映射的只读访问,第二个元素暴露了完整的操作 API。类型定义准确,接口设计符合 React Hook 最佳实践。
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
♻️ Duplicate comments (1)
packages/x/components/sender/SlotTextArea.tsx (1)
554-559: onKeyDown 可能被重复调用(与之前评论重复)
shouldSkipKeyHandling内部已经在第 472 行调用了onKeyDown?.(e),当其返回true时,第 557 行又再次调用onKeyDown。这会导致外部回调被执行两次。建议移除第 557 行的重复调用:
const onInternalKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => { // 检查是否应该跳过处理 if (shouldSkipKeyHandling(e)) { - onKeyDown?.(e as unknown as React.KeyboardEvent<HTMLTextAreaElement>); return; }注意:此问题在之前的评论中已被标记(提到在 commit d27223a 中已修复),但当前代码中似乎仍存在此问题。
🧹 Nitpick comments (2)
packages/x/components/sender/SlotTextArea.tsx (2)
136-136: 检查 getEditorValue 闭包是否需要 useCallback 包装第 136 行传递给
useCursor的getEditorValue是一个箭头函数() => getEditorValue()。每次渲染都会创建新的函数引用,这可能导致useCursor内部的依赖更新。如果useCursor内部将此函数作为依赖项,可能会触发不必要的重新计算或副作用。建议使用
useCallback包装此函数,或者直接传递getEditorValue引用(如果它本身是稳定的)。} = useCursor({ prefixCls, getSlotDom: (key: string) => slotDomMap.current.get(key), slotConfigMap, getNodeInfo, - getEditorValue: () => getEditorValue(), + getEditorValue, });
540-540: 修正错误消息格式错误消息中冒号后缺少空格,建议改为:
- warning(false, 'Sender', `Failed to handle skill area key event:${error}`); + warning(false, 'Sender', `Failed to handle skill area key event: ${error}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/x/components/sender/SlotTextArea.tsx(16 hunks)packages/x/components/sender/hooks/use-slot-builder.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/x/components/sender/hooks/use-slot-builder.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
⏰ 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). (5)
- GitHub Check: build preview
- GitHub Check: size
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
packages/x/components/sender/SlotTextArea.tsx (5)
313-381: getEditorValue 重构逻辑清晰,性能优化合理这段重构将值提取逻辑改为预分配数组并使用索引递增的方式,避免了频繁的数组操作。通过
slice(0, resultIndex)来处理实际使用的长度,逻辑正确且高效。空内容的处理也更加健壮。
476-514: 退格键处理逻辑提取得当将退格键的删除逻辑提取为独立方法
handleBackspaceKey,提高了代码可读性和可维护性。边界条件检查(如focusOffset === 1和focusOffset === 0)覆盖全面,对于插槽和技能节点的删除处理正确。
694-740: 插入上下文获取逻辑提取合理将插入位置判断逻辑提取为
getInsertContext方法,通过 switch 语句清晰地处理不同的插入类型(end、start、slot、box),代码结构更加清晰,便于维护和测试。
817-838: focus 方法重构提升了可读性将焦点管理逻辑重构为基于
cursor类型的 switch 语句,清晰地处理不同的聚焦场景(slot、start、all、end)。选项合并逻辑(mergeOptions)确保了默认值的正确应用。
12-153: 整体重构架构合理,hook 集成提升了代码组织性本次重构引入了三个专用 hooks(
useCursor、useSlotBuilder、useSlotConfigState),将光标管理、DOM 构建和配置状态管理逻辑从组件中分离出来,符合 React 最佳实践。代码的可测试性和可维护性得到显著提升。主要改进:
- 光标操作集中管理,API 更加清晰
- 插槽构建逻辑模块化
- 配置驱动的渲染方式更易扩展
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: 6
♻️ Duplicate comments (2)
packages/x/components/sender/__tests__/slot.test.tsx (1)
426-432: 修正控制台警告的监听方式(未修复的问题)测试监听
console.error但实现使用了warning工具函数,该函数内部调用console.warn而非console.error。此问题在之前的评审中已被标记但尚未修复。应用此修复:
- const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); const invalidConfig = [{ type: 'input', props: { placeholder: 'No key' } } as SlotConfigType]; render(<Sender slotConfig={invalidConfig} />); expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Slot key is missing')); consoleSpy.mockRestore();packages/x/components/sender/hooks/use-cursor.ts (1)
200-227: 需要区分节点类型来正确设置偏移量当前代码在 Line 211 使用
targetNode.childNodes.length来限制 offset。这个实现假设targetNode始终是 Element 节点。但是:
- 对于 Element 节点:
Range.setStart(node, offset)的 offset 表示子节点索引- 对于 Text 节点:offset 表示字符位置
如果
targetNode可能是 Text 节点,应该根据节点类型选择不同的 offset 计算方式。建议应用以下修复:
focus(editableNode, preventScroll); const { range, selection } = getRange(); if (range && selection) { try { - const maxPosition = Math.min(position, targetNode.childNodes.length); + const maxPosition = + targetNode.nodeType === Node.TEXT_NODE + ? Math.min(position, targetNode.textContent?.length || 0) + : Math.min(position, targetNode.childNodes.length); range.setStart(targetNode, maxPosition); range.setEnd(targetNode, maxPosition); range.collapse(false); setRange(range, selection); } catch (error) { - warning(false, 'Sender', `Failed to set cursor position:: ${error}`); + warning(false, 'Sender', `Failed to set cursor position: ${error}`); } }同时修正了 Line 217 的拼写错误(双冒号)。
🧹 Nitpick comments (1)
packages/x/components/sender/hooks/use-cursor.ts (1)
291-301: 过往问题已修复,建议统一可选链使用根据过往评审意见,此函数之前存在"创建 newRange 但添加原 range"的问题。当前代码已修复:直接修改并使用
range,逻辑正确。依赖项
[focus]也是正确的。轻微的代码风格建议:Line 295 使用了可选链
range?.setStartAfter,但 Line 296range.collapse没有使用。虽然 Line 293 已经做了守卫,但为保持一致性,可以考虑:if (!range || !selection) return; focus(editableNode, preventScroll); - range?.setStartAfter(targetNode); + range.setStartAfter(targetNode); range.collapse(false); selection.removeAllRanges(); selection.addRange(range);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/x/components/sender/__tests__/slot.test.tsx(3 hunks)packages/x/components/sender/hooks/use-cursor.ts(1 hunks)packages/x/components/sender/hooks/use-slot-builder.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/x/components/sender/hooks/use-slot-builder.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
🧬 Code graph analysis (2)
packages/x/components/sender/hooks/use-cursor.ts (1)
packages/x/components/sender/interface.ts (2)
InsertPosition(13-13)SlotConfigBaseType(33-36)
packages/x/components/sender/__tests__/slot.test.tsx (1)
packages/x/components/sender/interface.ts (1)
SlotConfigType(113-119)
🪛 GitHub Actions: ✅ test
packages/x/components/sender/__tests__/slot.test.tsx
[error] 552-552: TS6133: 'onChange' is declared but its value is never read.
[error] 557-557: TS6133: 'value' is declared but its value is never read.
[error] 782-782: TS6133: 'value' is declared but its value is never read.
[error] 816-816: TS6133: 'onChange' is declared but its value is never read.
[error] 831-831: TS2353: Object literal may only specify known properties, and 'description' does not exist in type 'SkillType'.
[error] 917-917: TS6133: 'onChange' is declared but its value is never read.
[error] 983-983: TS2353: Object literal may only specify known properties, and 'description' does not exist in type 'SkillType'.
⏰ 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). (3)
- GitHub Check: size
- GitHub Check: build preview
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
packages/x/components/sender/__tests__/slot.test.tsx (1)
5-58: DOM 模拟设置完善全局 DOM API 模拟配置完整,正确模拟了
document.createRange和window.getSelection,涵盖了测试 contenteditable 和文本选择功能所需的所有必要方法。packages/x/components/sender/hooks/use-cursor.ts (5)
1-67: 类型定义和接口设计合理导入和类型定义结构清晰,UseCursorReturn 接口涵盖了光标管理的各种场景。使用统一的
warning工具函数替代console.warn也符合重构目标。
70-129: 基础工具函数实现稳健所有工具函数都具备:
- SSR 安全检查(window 存在性)
- Try-catch 错误处理
- 统一的 warning 日志
- 正确的 useCallback 依赖项
131-198: 光标设置函数实现正确
setEndCursor、setStartCursor和setAllSelectCursor三个函数:
- 统一的错误处理模式
- 正确使用
collapse参数setAllSelectCursor对skillDom的特殊处理(跳过第一个子节点)符合技能槽的 UI 逻辑
229-289: 插槽聚焦逻辑处理周全
setSlotFocus函数通过嵌套辅助函数实现了:
- 区分
input和content类型插槽的聚焦策略- 支持指定 key 或自动查找第一个可聚焦插槽
- 对 INPUT 元素和 contentEditable 元素采用不同的聚焦方式
- 正确排除占位符节点(Line 249)
303-433: 文本和位置工具函数实现细致剩余的工具函数都展现了良好的边界情况处理:
getTextBeforeCursor(Line 303-341):
- 快速路径优化(空节点早返回)
- 验证光标在目标节点内
- 清理零宽空格
\u200B
getInsertPosition(Line 348-404):
- 多层次回退逻辑
- 与
getNodeInfo集成以识别插槽类型
getEndRange和getStartRange(Line 409-433):
- 正确处理尾部换行符边界情况
- 考虑 skill 节点偏移
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
♻️ Duplicate comments (6)
packages/x/components/sender/__tests__/slot.test.tsx (6)
551-568: 移除未使用的变量声明TypeScript 报告未使用的变量:第 552 行的
onChange和第 557 行的value参数。应用此修复:
it('should handle custom slot with disabled state', () => { - const onChange = jest.fn(); const customConfig: SlotConfigType[] = [ { type: 'custom', key: 'custom-disabled', - customRender: (value: any, onChange: (value: any) => void, { disabled }: any) => ( + customRender: (_value: any, _onChange: (value: any) => void, { disabled }: any) => ( <button type="button" disabled={disabled} onClick={() => _onChange('test')}> {disabled ? 'Disabled' : 'Enabled'} </button> ), }, ]; const { getByText } = render(<Sender slotConfig={customConfig} disabled />); expect(getByText('Disabled')).toBeDisabled(); });
776-796: 移除未使用的参数第 782 行的
value参数未使用。应用此修复:
const customConfig: SlotConfigType[] = [ { type: 'custom', key: 'custom-format', - customRender: (value: any, onChange: (value: any) => void) => ( + customRender: (_value: any, onChange: (value: any) => void) => ( <button type="button" onClick={() => onChange('test-value')}> Click </button> ), formatResult: (v: any) => `[${v}]`, }, ];
799-826: 移除未使用的参数第 819 行的
onChange参数未使用。应用此修复:
{ type: 'custom', key: 'custom', - customRender: (value, onChange) => <span>Custom: {value}</span>, + customRender: (value) => <span>Custom: {value}</span>, formatResult: (v) => `[${v}]`, },
828-844: 移除不存在的属性第 835 行的
description属性不存在于SkillType类型中。应用此修复:
const { container } = render( <Sender slotConfig={[]} skill={{ value: 'test-skill', title: 'Test Skill', - description: 'This is a test skill', closable: { closeIcon: '×', onClose: jest.fn(), }, }} />, );
901-928: 移除未使用的参数第 921 行的
onChange参数未使用。应用此修复:
{ type: 'custom', key: 'custom', - customRender: (value, onChange) => <span>Custom: {value}</span>, + customRender: (value) => <span>Custom: {value}</span>, formatResult: (v) => `[${v}]`, },
930-946: 移除不存在的属性第 937 行的
description属性不存在于SkillType类型中。应用此修复:
const { container } = render( <Sender slotConfig={[]} skill={{ value: 'test-skill', title: 'Test Skill', - description: 'This is a test skill', closable: { closeIcon: '×', onClose: jest.fn(), }, }} />, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/x/components/sender/__tests__/slot.test.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
📚 Learning: 2024-12-04T03:46:13.221Z
Learnt from: YumoImer
Repo: ant-design/x PR: 293
File: components/x-request/demo/custom-transformer.tsx:75-79
Timestamp: 2024-12-04T03:46:13.221Z
Learning: When reviewing code examples in `components/x-request/demo/custom-transformer.tsx` that use `mockFetch`, it's unnecessary to suggest enhancements to the `TransformStream` implementation, such as adding error handling and data validation, since the code is intended as a simple demonstration.
Applied to files:
packages/x/components/sender/__tests__/slot.test.tsx
🧬 Code graph analysis (1)
packages/x/components/sender/__tests__/slot.test.tsx (2)
packages/x/components/sender/index.tsx (1)
SlotConfigType(36-36)packages/x/components/sender/interface.ts (1)
SlotConfigType(113-119)
🪛 GitHub Actions: ✅ test
packages/x/components/sender/__tests__/slot.test.tsx
[error] 552-552: TypeScript error TS6133: 'onChange' is declared but its value is never read.
[error] 557-557: TypeScript error TS6133: 'value' is declared but its value is never read.
[error] 782-782: TypeScript error TS6133: 'value' is declared but its value is never read.
[error] 819-819: TypeScript error TS6133: 'onChange' is declared but its value is never read.
[error] 835-835: TypeScript error TS2353: Object literal may only specify known properties, and 'description' does not exist in type 'SkillType'.
[error] 921-921: TypeScript error TS6133: 'onChange' is declared but its value is never read.
[error] 937-937: TypeScript error TS2353: Object literal may only specify known properties, and 'description' does not exist in type 'SkillType'.
[error] 1045-1045: TypeScript error TS2322: Type '({ type: string; value: string; key?: undefined; props?: undefined; } | { type: string; key: string; props: { defaultValue: string; }; value?: undefined; })[]' is not assignable to type 'readonly SlotConfigType[]'.
[error] 1083-1083: TypeScript error TS2741: Property 'options' is missing in type '{ defaultValue: string; }' but required in type '{ defaultValue?: string | undefined; options: string[]; placeholder?: string | undefined; }'.
⏰ 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). (3)
- GitHub Check: size
- GitHub Check: build preview
- GitHub Check: Cloudflare Pages
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: 1
♻️ Duplicate comments (3)
packages/x/components/sender/__tests__/slot.test.tsx (3)
426-433: 修正控制台警告的监听方式测试监听了
console.error,但根据之前的代码分析,组件实际使用warning工具函数(来自@rc-component/util),该函数内部调用的是console.warn而非console.error。应改为监听console.warn以确保测试能正确捕获警告。应用以下修复:
- const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); const invalidConfig = [{ type: 'input', props: { placeholder: 'No key' } } as SlotConfigType]; render(<Sender slotConfig={invalidConfig} />); expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Slot key is missing')); consoleSpy.mockRestore();基于之前的代码分析结果。
1016-1026: 修正控制台警告的监听方式与第 426-433 行的问题相同,此处也应监听
console.warn而非console.error。应用以下修复:
- const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); const invalidConfig = [ { type: 'input', props: { placeholder: 'No key' } } as SlotConfigType, { type: 'select', props: { options: ['A', 'B'] } } as SlotConfigType, ]; render(<Sender slotConfig={invalidConfig} />); expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Slot key is missing')); consoleSpy.mockRestore();
899-1005: 删除重复的测试块"Comprehensive coverage tests" 测试块(第 899-1005 行)与 "Final coverage tests" 测试块(第 798-897 行)包含几乎完全相同的测试用例,导致不必要的代码重复。重复的测试包括:
- "should handle all slot types with various/complete configurations"
- "should handle skill with all properties"
- "should handle all event handlers"
- "should handle all props combinations"
- "should handle empty and null values gracefully"
建议删除整个 "Comprehensive coverage tests" 描述块,或将两个块中的唯一差异(如第 973-991 行中额外的
prefix、suffix、footer、headerprops)合并到 "Final coverage tests" 块中。应用以下修复:
}); - describe('Comprehensive coverage tests', () => { - it('should handle all slot types with complete configurations', () => { - // ... 与 Final coverage tests 中的测试重复 - }); - - it('should handle skill with all properties', () => { - // ... 与 Final coverage tests 中的测试重复 - }); - - it('should handle all event handlers', () => { - // ... 与 Final coverage tests 中的测试重复 - }); - - it('should handle all props combinations', () => { - // ... 如需保留额外的 props,请合并到 Final coverage tests 中 - }); - - it('should handle empty and null values gracefully', () => { - // ... 与 Final coverage tests 中的测试重复 - }); - }); - describe('Edge case and boundary tests', () => {
🧹 Nitpick comments (1)
packages/x/components/suggestion/demo/trigger.tsx (1)
5-5: 未使用的变量_uuid。变量
_uuid在 line 15 被递增,但在整个代码中从未被读取或使用。tag 的命名是基于value状态而非_uuid。建议移除此未使用的变量以提高代码清晰度。应用此 diff 来移除未使用的变量:
-let _uuid = 0; - const Demo: React.FC = () => { const [tags, setTags] = React.useState<string[]>([]); const [value, setValue] = React.useState(''); return ( <Suggestion<string> items={(info) => [{ label: `Trigger by '${info}'`, value: String(info) }]} onSelect={() => { - _uuid += 1; setTags([...tags, `Cell_${value.replace('/g', '')}`]); setValue(''); }}Also applies to: 15-15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/x/.dumi/theme/slots/Header/Navigation.tsx(1 hunks)packages/x/components/sender/__tests__/slot.test.tsx(3 hunks)packages/x/components/suggestion/demo/trigger.tsx(2 hunks)packages/x/docs/x-sdk/demos/chat-providers/custom-provider-width-ui.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/x/docs/x-sdk/demos/chat-providers/custom-provider-width-ui.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
📚 Learning: 2024-12-04T03:46:13.221Z
Learnt from: YumoImer
Repo: ant-design/x PR: 293
File: components/x-request/demo/custom-transformer.tsx:75-79
Timestamp: 2024-12-04T03:46:13.221Z
Learning: When reviewing code examples in `components/x-request/demo/custom-transformer.tsx` that use `mockFetch`, it's unnecessary to suggest enhancements to the `TransformStream` implementation, such as adding error handling and data validation, since the code is intended as a simple demonstration.
Applied to files:
packages/x/components/sender/__tests__/slot.test.tsx
🧬 Code graph analysis (2)
packages/x/.dumi/theme/slots/Header/Navigation.tsx (2)
packages/x/docs/playground/_utils/local.ts (1)
isZhCN(149-150)packages/x/.dumi/theme/utils.ts (1)
isZhCN(119-121)
packages/x/components/sender/__tests__/slot.test.tsx (2)
packages/x/components/sender/index.tsx (1)
SlotConfigType(36-36)packages/x/components/sender/interface.ts (1)
SlotConfigType(113-119)
⏰ 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). (5)
- GitHub Check: test
- GitHub Check: build preview
- GitHub Check: size
- GitHub Check: test
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
packages/x/.dumi/theme/slots/Header/Navigation.tsx (1)
197-197: 语法简化改进正确。将
locale['zhUrl']改为locale.zhUrl是一个很好的代码风格优化,点表示法更简洁易读。逻辑功能完全一致,类型安全。不过需要注意:此更改似乎与 PR 标题所述的"Sender 插槽模式重构"无直接关联,可能是随本次 PR 一并提交的附带优化。
packages/x/components/sender/__tests__/slot.test.tsx (6)
5-58: 全局模拟设置完善DOM API 模拟设置全面覆盖了 Range 和 Selection API 所需的方法,为 contentEditable 相关功能的测试提供了充分的支持。
61-89: 测试数据配置完整
baseSlotConfig覆盖了所有插槽类型(text、input、content、select、tag、custom),为后续测试提供了完整的基准配置。
91-134: 核心功能测试完善核心功能测试覆盖了组件的基本渲染、空配置处理、ref 接口完整性以及值的获取和清除功能,测试结构清晰合理。
136-296: 交互测试覆盖全面交互测试完整覆盖了输入变化、下拉选择、内容编辑、键盘事件(Enter、Shift+Enter、Ctrl+A、Backspace)以及粘贴事件(文件和文本),使用了适当的异步等待和事件触发。
298-796: 技能功能、边界条件和扩展覆盖测试完善这些测试块全面覆盖了技能(skill)功能的各种场景、边界条件处理、配置更新以及各种 edge cases。测试结构清晰,断言准确,特别是第 720-738 行通过自定义 footer 正确测试了 submitDisabled 状态。
1007-1107: 边界和边缘情况测试全面边缘情况测试覆盖了 null/undefined 配置、超长文本、特殊字符(包括潜在的 XSS 字符串)、快速配置变更、并发 ref 方法调用以及嵌套插槽交互等场景,第 1029 行的注释表明对测试性能进行了合理考虑。
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
🧹 Nitpick comments (3)
packages/x/components/sender/__tests__/slot-error.test.tsx (2)
87-102: 建议增强测试断言。当前测试仅验证"不抛出异常",但没有断言实际行为。建议添加对 DOM 状态、ref 方法返回值或副作用的具体断言,以提高测试的有效性。
例如:
ref.current?.insert([{ type: 'text', value: 'test' }]); expect(ref.current?.getValue().value).toContain('test'); ref.current?.focus(); expect(document.activeElement).toBe(container.querySelector('[role="textbox"]')); ref.current?.clear(); expect(ref.current?.getValue().value).toBe('');
122-145: 建议验证onPaste回调行为。当前测试传入了
onPaste回调但没有验证它是否被调用。建议添加断言以确保事件处理正确触发。expect(() => { fireEvent.paste(inputArea, { /* ... */ }); }).not.toThrow(); expect(onPaste).toHaveBeenCalled();packages/x/components/sender/__tests__/slot-boundary.test.tsx (1)
351-381: 发现重复的测试用例Lines 352-365 和 lines 367-380 的两个测试用例完全相同,都测试了空 clipboardData 的场景。建议合并为一个测试,或者调整其中一个测试以覆盖不同的场景(例如测试有文件的粘贴)。
合并重复测试的示例:
- it('should handle paste with null clipboardData', () => { - const { container } = render(<Sender slotConfig={[{ type: 'text', value: 'Test' }]} />); - - const inputArea = container.querySelector('[role="textbox"]') as HTMLElement; - - expect(() => { - fireEvent.paste(inputArea, { - clipboardData: { - getData: () => '', - files: [], - }, - }); - }).not.toThrow(); - }); - - it('should handle paste with empty text and no files', () => { + it('should handle paste with empty clipboardData', () => { const { container } = render(<Sender slotConfig={[{ type: 'text', value: 'Test' }]} />); const inputArea = container.querySelector('[role="textbox"]') as HTMLElement; expect(() => { fireEvent.paste(inputArea, { clipboardData: { getData: () => '', files: [], }, }); }).not.toThrow(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/x/components/sender/__tests__/slot-boundary.test.tsx(1 hunks)packages/x/components/sender/__tests__/slot-error.test.tsx(1 hunks)packages/x/components/sender/__tests__/use-cursor.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
⏰ 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). (5)
- GitHub Check: test
- GitHub Check: build preview
- GitHub Check: size
- GitHub Check: test
- GitHub Check: Cloudflare Pages
🔇 Additional comments (27)
packages/x/components/sender/__tests__/slot-error.test.tsx (7)
105-120: 测试正确处理了 console mock。该测试正确地 mock 了
console.error并在测试后恢复,避免了副作用。这是良好的测试实践。
179-181: 验证fireEvent.input的 target 属性。使用
innerText作为fireEvent.input的 target 属性可能无法正确模拟输入事件。对于 contenteditable 元素,通常使用textContent。请验证这是否能正确触发 auto-resize 逻辑。如果需要测试 auto-resize 行为,建议添加对元素高度变化的断言,或验证相关的 resize 回调是否被触发。
184-213: 正确使用了rerender测试 prop 变化。该测试正确地使用
rerender来测试 skill prop 的变化和移除场景,测试结构合理。
215-236: 良好的 API 覆盖。测试覆盖了
insert方法的所有位置参数变体(start、end、all、slot),确保了 API 的完整性。
238-263: 优秀的测试断言实践。与其他测试不同,该测试不仅检查"不抛异常",还对
getValue()返回值进行了具体的结构断言,验证了实际功能。这是良好的测试实践。
265-276: 测试覆盖了新增的preventScroll功能。该测试验证了
focus方法的preventScroll选项与不同光标位置的组合,与 PR 中提到的新功能对应。
297-313: 良好的边界条件测试。测试覆盖了部分配置的边界情况(空文本、无默认值、空选项),并验证了
clear后的状态,确保了健壮性。packages/x/components/sender/__tests__/use-cursor.test.tsx (11)
1-72: 测试 Mock 配置完善Mock 设置覆盖了 DOM Selection 和 Range API 的主要方法,为测试提供了良好的隔离环境。beforeEach 和 afterEach 的使用确保了测试间的状态清理。
73-94: getSelection 测试覆盖完整测试覆盖了 window 未定义和正常获取 selection 的场景,边界条件处理得当。
96-137: getRange 错误处理测试充分测试涵盖了空 selection、正常 range 获取以及 getRangeAt 抛出异常的场景,确保了健壮性。
139-314: 光标定位方法测试全面各个光标定位方法都进行了充分的测试,包括边界条件、空值处理和错误场景。使用
act()包裹状态更新是正确的做法。
316-429: getTextBeforeCursor 测试覆盖完整测试包含了多种边界情况,特别是 lines 404-428 的零宽空格处理测试,这对于 contenteditable 元素非常重要。错误处理和边界条件都得到了验证。
431-471: removeAllRanges 测试合理覆盖了空 selection、正常清除和错误处理场景,测试策略得当。
473-585: setSlotFocus 测试全面测试涵盖了不同的 slot 类型(input/content)以及自动查找第一个可聚焦 slot 的逻辑(lines 555-584),确保了功能的健壮性。
587-770: getInsertPosition 测试细致该测试组覆盖了插入位置计算的各种场景,包括 start/end/cursor/box 等不同的位置类型,以及 selection、range、editableRef 为空等边界情况,测试非常充分。
772-848: Range 计算测试覆盖基本场景getEndRange 和 getStartRange 的测试包含了有/无 skill 的情况,lines 790-805 的换行符处理测试也很重要。
850-895: setAfterNodeFocus 测试合理测试覆盖了空 range 和正常设置焦点的场景,验证了预期的方法调用。
897-921: 焦点错误处理测试得当测试验证了在 focus 方法抛出异常或元素不可聚焦时,代码能够优雅处理而不抛出错误。
packages/x/components/sender/__tests__/slot-boundary.test.tsx (9)
5-70: Mock 设置详尽且实用测试 Mock 配置非常全面,包括了 Range 和 Selection 的所有必要方法。Line 23-25 对 setStartAfter 的错误模拟特别有用。
72-156: 错误处理测试覆盖充分测试组涵盖了 insert 方法的各种错误场景,包括空 ref、空配置、null selection 等。Lines 103-124 正确地恢复了 Mock 的 window.getSelection,避免影响后续测试。
158-182: Focus 方法边界测试完整测试覆盖了 focus 方法的所有光标位置选项(start/end/all/slot)以及 preventScroll 参数,边界条件处理得当。
184-242: Blur 和 Select 事件测试合理测试验证了 blur 和 select 事件在不同场景下的行为,包括有 skill 和无 skill 的情况,确保了事件处理的健壮性。
244-300: Clear 和 GetValue 测试覆盖完整测试包含了简单和复杂 slot 配置的场景,验证了返回值的结构。Lines 256-260 的复杂配置测试很好地模拟了实际使用场景。
302-349: 输入法和键盘事件测试充分测试覆盖了 composition 事件以及各种键盘组合键(包括 Enter + 修饰键,Ctrl/Cmd+A 等),Lines 332-348 还测试了复杂 DOM 结构下的 Backspace 行为。
383-401: Skill 交互测试合理测试覆盖了 skill 的动态变化场景,包括从无到有的转换以及删除不存在的 skill,确保了组件对 skill 状态变化的正确响应。
403-426: Slot 渲染边界测试完善测试涵盖了无效配置的场景(缺少 key 或未知类型),Lines 405-413 正确地 Mock 了 console.error 以避免测试输出污染,确保了组件在异常配置下的容错性。
428-441: AutoSize 测试覆盖基本场景测试验证了 autoSize 属性的基本功能,包括对象配置和 false 值。测试已足够,但如果要进一步增强,可以考虑添加 minRows > maxRows 等边界条件的测试。
可以考虑添加以下边界测试以增强覆盖率:
it('should handle autoSize with invalid minRows/maxRows', () => { render( <Sender slotConfig={[{ type: 'text', value: 'Test' }]} autoSize={{ minRows: 10, maxRows: 1 }} />, ); }); it('should handle autoSize with zero or negative values', () => { render( <Sender slotConfig={[{ type: 'text', value: 'Test' }]} autoSize={{ minRows: 0, maxRows: 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.
Actionable comments posted: 5
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/x/components/attachments/__tests__/util.test.ts(1 hunks)packages/x/components/sender/__tests__/slot-error.test.tsx(1 hunks)packages/x/components/sender/__tests__/slot.test.tsx(3 hunks)packages/x/components/suggestion/demo/trigger.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/x/components/suggestion/demo/trigger.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
📚 Learning: 2024-12-04T03:46:13.221Z
Learnt from: YumoImer
Repo: ant-design/x PR: 293
File: components/x-request/demo/custom-transformer.tsx:75-79
Timestamp: 2024-12-04T03:46:13.221Z
Learning: When reviewing code examples in `components/x-request/demo/custom-transformer.tsx` that use `mockFetch`, it's unnecessary to suggest enhancements to the `TransformStream` implementation, such as adding error handling and data validation, since the code is intended as a simple demonstration.
Applied to files:
packages/x/components/sender/__tests__/slot.test.tsxpackages/x/components/sender/__tests__/slot-error.test.tsx
🧬 Code graph analysis (2)
packages/x/components/attachments/__tests__/util.test.ts (1)
packages/x/components/attachments/util.ts (2)
isImageFileType(3-3)previewImage(7-56)
packages/x/components/sender/__tests__/slot.test.tsx (3)
packages/x/components/sender/index.tsx (1)
SlotConfigType(36-36)packages/x/components/sender/interface.ts (1)
SlotConfigType(113-119)packages/x/scripts/check-cssinjs.tsx (2)
render(33-41)render(48-56)
⏰ 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). (5)
- GitHub Check: test
- GitHub Check: size
- GitHub Check: build preview
- GitHub Check: test
- GitHub Check: Cloudflare Pages
🔇 Additional comments (9)
packages/x/components/sender/__tests__/slot-error.test.tsx (1)
117-325: 测试覆盖全面且结构合理。剩余测试用例涵盖了插槽验证、粘贴事件、键盘交互、自动调整大小、技能交互、插槽插入、复杂配置以及焦点管理等场景。测试逻辑清晰,使用
expect().not.toThrow()验证错误处理路径是恰当的。packages/x/components/attachments/__tests__/util.test.ts (2)
1-25: 测试用例覆盖全面!
isImageFileType的测试覆盖了正向用例、负向用例和大小写敏感性,与实现逻辑完全一致。
153-170: LGTM!常规图片和边界情况(null/undefined)的测试逻辑正确,期望值与实现行为一致。
packages/x/components/sender/__tests__/slot.test.tsx (6)
5-58: LGTM!全面的 DOM API 模拟设置全局 DOM API 模拟(
document.createRange和window.getSelection)设置完善,为测试涉及光标定位、文本选择和插入操作的组件提供了必要的支持。模拟实现包含了所有必需的方法和属性。
60-89: LGTM!全面的插槽配置测试基线
baseSlotConfig定义了涵盖所有插槽类型(text、input、content、select、tag、custom)的综合测试配置,为跨测试重用提供了良好的基础。
91-134: LGTM!核心功能测试覆盖全面核心功能测试涵盖了所有插槽类型的渲染、空配置处理、ref 接口暴露以及值的获取和清除操作,测试结构清晰合理。
136-296: LGTM!交互测试覆盖全面交互测试涵盖了输入、选择、内容编辑、自定义插槽、键盘事件和粘贴操作等场景,正确使用了
waitFor处理异步操作,测试逻辑清晰完整。
298-422: LGTM!技能功能测试完整技能功能测试涵盖了技能的渲染、关闭、键盘移除、更改以及生命周期操作,测试场景全面合理。
498-1000: LGTM!测试覆盖率极高且场景全面测试套件覆盖了大量场景,包括:
- 正常功能路径
- 边界条件(空值、长文本、特殊字符)
- 错误处理(缺失的键、无效配置)
- 性能场景(快速配置变更、并发操作)
- 复杂交互(嵌套插槽)
测试结构清晰,分组合理,为组件提供了可靠的质量保障。
…into slot-sender-refactor
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 (3)
packages/x/components/sender/__tests__/slot.test.tsx (3)
239-260: 建议增强断言这些测试用例仅包含注释 "Should not throw" 或 "Should handle ... without error",但实际上测试框架在抛出异常时会自动失败。建议添加更有意义的断言来验证预期行为,或者如果确实只是验证不抛错,可以使用
expect().not.toThrow()包装。it('should handle Ctrl+A selection', () => { const { container } = render(<Sender slotConfig={baseSlotConfig} />); const inputArea = container.querySelector('[role="textbox"]') as HTMLElement; - fireEvent.keyDown(inputArea, { key: 'a', ctrlKey: true }); - // Should not throw + expect(() => { + fireEvent.keyDown(inputArea, { key: 'a', ctrlKey: true }); + }).not.toThrow(); });
845-868: 事件处理器断言不够充分测试触发了
focus、blur、keyDown、keyUp事件,但断言只验证了 handler 是否为函数类型,而没有验证它们是否被实际调用。建议使用toHaveBeenCalled()断言。fireEvent.focus(inputArea); fireEvent.blur(inputArea); fireEvent.keyDown(inputArea, { key: 'a' }); fireEvent.keyUp(inputArea, { key: 'a' }); - expect(typeof handlers.onFocus).toBe('function'); - expect(typeof handlers.onBlur).toBe('function'); + expect(handlers.onFocus).toHaveBeenCalled(); + expect(handlers.onBlur).toHaveBeenCalled(); + expect(handlers.onKeyDown).toHaveBeenCalled(); + expect(handlers.onKeyUp).toHaveBeenCalled(); });
954-967: 测试名称具有误导性测试名称为"concurrent ref method calls",但
insert、clear、getValue是同步方法,使用Promise.all并不能实现真正的并发调用。建议将测试名称改为更准确的描述,如"连续调用 ref 方法"。- it('should handle concurrent ref method calls', () => { + it('should handle sequential ref method calls', () => { const ref = React.createRef<any>(); render(<Sender slotConfig={[{ type: 'text', value: 'Test' }]} ref={ref} />); - // Concurrent operations + // Sequential operations expect(() => { - Promise.all([ - ref.current?.insert([{ type: 'text', value: '1' }]), - ref.current?.insert([{ type: 'text', value: '2' }]), - ref.current?.clear(), - ref.current?.getValue(), - ]); + ref.current?.insert([{ type: 'text', value: '1' }]); + ref.current?.insert([{ type: 'text', value: '2' }]); + ref.current?.clear(); + ref.current?.getValue(); }).not.toThrow(); });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/x/components/sender/__tests__/slot.test.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
📚 Learning: 2024-12-04T03:46:13.221Z
Learnt from: YumoImer
Repo: ant-design/x PR: 293
File: components/x-request/demo/custom-transformer.tsx:75-79
Timestamp: 2024-12-04T03:46:13.221Z
Learning: When reviewing code examples in `components/x-request/demo/custom-transformer.tsx` that use `mockFetch`, it's unnecessary to suggest enhancements to the `TransformStream` implementation, such as adding error handling and data validation, since the code is intended as a simple demonstration.
Applied to files:
packages/x/components/sender/__tests__/slot.test.tsx
⏰ 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). (4)
- GitHub Check: test
- GitHub Check: build preview
- GitHub Check: size
- GitHub Check: test
🔇 Additional comments (8)
packages/x/components/sender/__tests__/slot.test.tsx (8)
5-58: DOM API mock 设置合理全局 DOM API mock 设置完整,涵盖了
createRange和getSelection所需的方法。getSelectionmock 在每个测试前重新定义,确保测试隔离性。
61-89: 基础配置覆盖全面
baseSlotConfig包含所有插槽类型(text、input、content、select、tag、custom),为后续测试提供了良好的基础数据。
91-134: 核心功能测试完备测试覆盖了渲染、空配置、ref 接口(focus/blur/clear/getValue/insert)以及值的获取和清除,验证逻辑清晰。
298-422: 技能功能测试完善覆盖了技能的关闭按钮、不可关闭状态、键盘删除、技能切换、移除与重新添加等场景,测试结构清晰。
424-496: 边界条件测试正确正确使用
console.warn捕获警告信息,并覆盖了 disabled、readOnly、autoSize 配置以及各类事件回调。
498-649: 附加覆盖测试全面涵盖配置更新、默认值处理、readOnly/disabled 状态、多次插入操作、复杂配置等场景,测试覆盖率较高。
799-898: 最终覆盖测试完整覆盖了所有插槽类型的完整配置、技能属性、事件处理器组合以及 props 组合,为组件提供了全面的测试保障。
969-1000: 边缘用例测试完善空字符串值处理和嵌套插槽交互测试逻辑正确,
select类型已正确包含options属性。
|
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: 0
♻️ Duplicate comments (2)
packages/x/components/sender/__tests__/slot.test.tsx (2)
908-919: 修正控制台监听方式与第 425-433 行的测试相同,此处监听了
console.error但应该监听console.warn。应用此修复:
it('should handle slot with missing key warnings', () => { - const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); const invalidConfig = [ { type: 'input', props: { placeholder: 'No key' } } as SlotConfigType, { type: 'select', props: { options: ['A', 'B'] } } as SlotConfigType, ]; render(<Sender slotConfig={invalidConfig} />); expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Slot key is missing')); consoleSpy.mockRestore(); });注意:此测试与第 425-433 行的测试功能重复,可以考虑合并或移除其中一个。
基于代码审查记录,该问题在之前的评审中已被标记但似乎未正确修复。
425-433: 修正控制台监听方式测试监听了
console.error,但根据之前的审查意见和代码实现,组件使用的warning工具函数实际调用的是console.warn。这会导致测试无法正确捕获警告信息,造成测试失效。应用此修复:
it('should handle missing key warning', () => { - const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); const invalidConfig = [{ type: 'input', props: { placeholder: 'No key' } } as SlotConfigType]; render(<Sender slotConfig={invalidConfig} />); expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Slot key is missing')); consoleSpy.mockRestore(); });基于代码审查记录,该问题在之前的评审中已被标记但似乎未正确修复。
🧹 Nitpick comments (1)
packages/x/components/sender/__tests__/slot.test.tsx (1)
60-1001: 测试组织良好,覆盖全面测试文件结构清晰,涵盖了 Sender 组件插槽功能的各个方面:
- 核心功能(渲染、引用接口、值管理)
- 交互行为(输入、选择、键盘事件、粘贴)
- 技能功能(关闭按钮、移除、变更)
- 边界条件(禁用状态、只读状态、事件回调)
- 边缘情况(空值、特殊字符、并发操作)
之前审查中标记的问题(未使用变量、类型注解、重复测试块)已得到修复。
可选改进:测试文件较大(1000+ 行),可以考虑按功能模块拆分为多个文件(如
slot.basic.test.tsx、slot.interaction.test.tsx、slot.edge-cases.test.tsx),以提升可维护性。
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/x/components/mermaid/__tests__/index.test.tsx(2 hunks)packages/x/components/sender/__tests__/slot.test.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
Learnt from: afc163
Repo: ant-design/x PR: 0
File: :0-0
Timestamp: 2025-04-11T14:47:09.527Z
Learning: 当评审 ant-design/x 仓库中的 PR 时,需要用中文回复中文评论。该项目的文档支持中英双语。
📚 Learning: 2024-12-04T03:46:13.221Z
Learnt from: YumoImer
Repo: ant-design/x PR: 293
File: components/x-request/demo/custom-transformer.tsx:75-79
Timestamp: 2024-12-04T03:46:13.221Z
Learning: When reviewing code examples in `components/x-request/demo/custom-transformer.tsx` that use `mockFetch`, it's unnecessary to suggest enhancements to the `TransformStream` implementation, such as adding error handling and data validation, since the code is intended as a simple demonstration.
Applied to files:
packages/x/components/sender/__tests__/slot.test.tsxpackages/x/components/mermaid/__tests__/index.test.tsx
🧬 Code graph analysis (1)
packages/x/components/sender/__tests__/slot.test.tsx (2)
packages/x/components/sender/interface.ts (1)
SlotConfigType(113-119)packages/x/components/sender/index.tsx (1)
SlotConfigType(36-36)
⏰ 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). (4)
- GitHub Check: build preview
- GitHub Check: size
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (2)
packages/x/components/mermaid/__tests__/index.test.tsx (1)
85-96: 测试更新与生产代码一致错误处理测试已正确更新以验证新的统一警告格式
[antdx: Mermaid] Render failed。测试使用console.error监听器和expect.stringContaining进行灵活断言,与生产代码中使用集中式warning工具的行为一致。监听器的设置和恢复也处理得当。Also applies to: 423-434
packages/x/components/sender/__tests__/slot.test.tsx (1)
5-58: 测试环境模拟配置完善DOM API 的模拟实现非常全面,包括
document.createRange和window.getSelection的完整接口,能够支持光标操作和选区管理的测试场景。

中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
💡 Background and Solution
📝 Change Log
Summary by CodeRabbit
发行说明
重构
行为变更
新特性
样式
测试
杂项
✏️ Tip: You can customize this high-level summary in your review settings.