fix(x-markdown): support animation inside custom components - #1974
fix(x-markdown): support animation inside custom components#1974wryyyds7 wants to merge 2 commits into
Conversation
- Add option to StreamingOption interface - Modify Renderer.ts shouldReplaceText condition to allow AnimationText inside custom components when animateInsideComponents is enabled - Add 5 unit tests in Renderer.test.ts Close ant-design#1950
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough新增 Changes组件内文字动画
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Code Review
This pull request introduces a new animateInsideComponents configuration option to allow text fade-in animations inside custom components. It updates the Renderer logic to respect this option and adds comprehensive unit tests to verify its behavior. The review feedback suggests optimizing the text replacement check in Renderer.ts by lazily evaluating the parent tag check only when animations are enabled, and using Object.prototype.hasOwnProperty.call to prevent potential prototype pollution issues.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const parentTagName = (domNode.parent as Element)?.name; | ||
| const isParentCustomComponent = parentTagName && this.options.components?.[parentTagName]; | ||
| const shouldReplaceText = enableAnimation && isValidTextNode && !isParentCustomComponent; | ||
| const shouldReplaceText = enableAnimation && isValidTextNode && (!isParentCustomComponent || animateInsideComponents); |
There was a problem hiding this comment.
性能与安全性优化
问题分析:
- 性能开销: 目前的实现中,对于每个解析的 DOM 节点,无论
enableAnimation是否开启,或者当前节点是否为有效的文本节点 (isValidTextNode),都会执行parentTagName的获取以及在this.options.components中的查找操作。由于大部分用户默认不开启动画,这会带来不必要的性能开销。 - 原型链污染/安全隐患: 直接使用
this.options.components?.[parentTagName]进行查找时,如果parentTagName刚好是Object.prototype上的属性(例如toString、valueOf、hasOwnProperty等),会导致非预期的匹配。
改进建议:
建议将这部分逻辑放入 if (enableAnimation && isValidTextNode) 条件分支中进行惰性求值(Short-circuiting),并使用 Object.prototype.hasOwnProperty.call 安全地检查自定义组件,从而同时提升性能和安全性。
| const parentTagName = (domNode.parent as Element)?.name; | |
| const isParentCustomComponent = parentTagName && this.options.components?.[parentTagName]; | |
| const shouldReplaceText = enableAnimation && isValidTextNode && !isParentCustomComponent; | |
| const shouldReplaceText = enableAnimation && isValidTextNode && (!isParentCustomComponent || animateInsideComponents); | |
| let shouldReplaceText = false; | |
| if (enableAnimation && isValidTextNode) { | |
| const parentTagName = (domNode.parent as Element)?.name; | |
| const isParentCustomComponent = | |
| parentTagName && | |
| Object.prototype.hasOwnProperty.call(this.options.components || {}, parentTagName); | |
| shouldReplaceText = !isParentCustomComponent || !!animateInsideComponents; | |
| } |
Address gemini-code-assist review feedback: - Move parentTagName/isParentCustomComponent into if block for lazy evaluation - Use Object.prototype.hasOwnProperty.call for safe property lookup
Close #1950
问题
XMarkdown 的
streaming.enableAnimation开启后,当文字节点的父节点是自定义组件(通过components属性传入)时,动画会被跳过,文字不出现淡入效果。修改内容
interface.ts:在 StreamingOption 中新增 animateInsideComponents?: boolean 开关,默认 false(向后兼容)
Renderer.ts:修改 shouldReplaceText 判断条件为 (!isParentCustomComponent || animateInsideComponents)
Renderer.test.ts:新增 5 个单测
使用方式
<XMarkdown
,content={content}
streaming={{
enableAnimation: true,
animateInsideComponents: true,
animationConfig: { fadeDuration: 600, easing: 'ease-in-out' },
}}
components={{
p: ({ children, ...rest }) => <p {...rest}>{children}
}}
/>
根因在
Renderer.ts中: