Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 149 additions & 0 deletions packages/x-markdown/src/XMarkdown/__tests__/Renderer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import DOMPurify from 'dompurify';
import React from 'react';
import Renderer from '../core/Renderer';

import AnimationText from '../AnimationText';
// Mock React components for testing
const MockComponent: React.FC<any> = (props) => {
return React.createElement('div', props);
Expand Down Expand Up @@ -1518,6 +1519,154 @@ describe('Renderer', () => {
});
});

describe('animateInsideComponents', () => {
it('should skip AnimationText inside custom components by default', () => {
const CustomComponent: React.FC<any> = (props) => React.createElement('div', props);

const renderer = new Renderer({
components: { 'custom-wrapper': CustomComponent },
streaming: {
enableAnimation: true,
animationConfig: { fadeDuration: 100, easing: 'ease-in' },
},
});

const createElementSpy = jest.spyOn(React, 'createElement');

const html = '<custom-wrapper>text inside custom</custom-wrapper>';
renderer.processHtml(html);

// AnimationText should NOT be called for text inside custom components
const animationCalls = createElementSpy.mock.calls.filter(
(call) => call[0] === AnimationText,
);
expect(animationCalls).toHaveLength(0);

createElementSpy.mockRestore();
});

it('should use AnimationText inside custom components when animateInsideComponents is true', () => {
const CustomComponent: React.FC<any> = (props) => React.createElement('div', props);

const renderer = new Renderer({
components: { 'custom-wrapper': CustomComponent },
streaming: {
enableAnimation: true,
animationConfig: { fadeDuration: 100, easing: 'ease-in' },
animateInsideComponents: true,
},
});

const createElementSpy = jest.spyOn(React, 'createElement');

const html = '<custom-wrapper>text inside custom</custom-wrapper>';
renderer.processHtml(html);

// AnimationText SHOULD be called for text inside custom components
const animationCalls = createElementSpy.mock.calls.filter(
(call) => call[0] === AnimationText,
);
expect(animationCalls).toHaveLength(1);
expect(animationCalls[0][1]).toEqual(
expect.objectContaining({
text: 'text inside custom',
}),
);

createElementSpy.mockRestore();
});

it('should not affect text nodes outside custom components', () => {
const CustomComponent: React.FC<any> = (props) => React.createElement('div', props);

const renderer = new Renderer({
components: { 'custom-wrapper': CustomComponent },
streaming: {
enableAnimation: true,
animationConfig: { fadeDuration: 100, easing: 'ease-in' },
animateInsideComponents: true,
},
});

const createElementSpy = jest.spyOn(React, 'createElement');

// Text both inside and outside custom components
const html = '<p>outer text</p><custom-wrapper>inner text</custom-wrapper>';
renderer.processHtml(html);

const animationCalls = createElementSpy.mock.calls.filter(
(call) => call[0] === AnimationText,
);

// Both text nodes should be wrapped with AnimationText
expect(animationCalls).toHaveLength(2);
expect(animationCalls[0][1]).toEqual(
expect.objectContaining({ text: 'outer text' }),
);
expect(animationCalls[1][1]).toEqual(
expect.objectContaining({ text: 'inner text' }),
);

createElementSpy.mockRestore();
});

it('should work with nested custom components', () => {
const OuterComponent: React.FC<any> = (props) => React.createElement('div', props);
const InnerComponent: React.FC<any> = (props) => React.createElement('span', props);

const renderer = new Renderer({
components: {
'outer-comp': OuterComponent,
'inner-comp': InnerComponent,
},
streaming: {
enableAnimation: true,
animationConfig: { fadeDuration: 100, easing: 'ease-in' },
animateInsideComponents: true,
},
});

const createElementSpy = jest.spyOn(React, 'createElement');

const html = '<outer-comp><inner-comp>nested text</inner-comp></outer-comp>';
renderer.processHtml(html);

const animationCalls = createElementSpy.mock.calls.filter(
(call) => call[0] === AnimationText,
);
expect(animationCalls).toHaveLength(1);
expect(animationCalls[0][1]).toEqual(
expect.objectContaining({ text: 'nested text' }),
);

createElementSpy.mockRestore();
});

it('should not enable animation when enableAnimation is false even with animateInsideComponents', () => {
const CustomComponent: React.FC<any> = (props) => React.createElement('div', props);

const renderer = new Renderer({
components: { 'custom-wrapper': CustomComponent },
streaming: {
enableAnimation: false,
animateInsideComponents: true,
},
});

const createElementSpy = jest.spyOn(React, 'createElement');

const html = '<custom-wrapper>text inside custom</custom-wrapper>';
renderer.processHtml(html);

const animationCalls = createElementSpy.mock.calls.filter(
(call) => call[0] === AnimationText,
);
expect(animationCalls).toHaveLength(0);

createElementSpy.mockRestore();
});
});

describe('SSR / no DOM safety', () => {
it('returns null from processHtml when DOMPurify.sanitize is unavailable', () => {
const renderer = new Renderer({ components: { 'custom-tag': MockComponent } });
Expand Down
4 changes: 2 additions & 2 deletions packages/x-markdown/src/XMarkdown/core/Renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class Renderer {
unclosedTags: Set<string> | undefined,
cidRef: { current: number; tagIndexes: Record<string, number> },
) {
const { enableAnimation, animationConfig } = this.options.streaming || {};
const { enableAnimation, animationConfig, animateInsideComponents } = this.options.streaming || {};
return (domNode: DOMNode) => {
const key = `x-markdown-component-${cidRef.current++}`;

Expand All @@ -182,7 +182,7 @@ class Renderer {
// Skip animation for text nodes inside custom components to preserve their internal structure
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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

性能与安全性优化

问题分析:

  1. 性能开销: 目前的实现中,对于每个解析的 DOM 节点,无论 enableAnimation 是否开启,或者当前节点是否为有效的文本节点 (isValidTextNode),都会执行 parentTagName 的获取以及在 this.options.components 中的查找操作。由于大部分用户默认不开启动画,这会带来不必要的性能开销。
  2. 原型链污染/安全隐患: 直接使用 this.options.components?.[parentTagName] 进行查找时,如果 parentTagName 刚好是 Object.prototype 上的属性(例如 toStringvalueOfhasOwnProperty 等),会导致非预期的匹配。

改进建议:
建议将这部分逻辑放入 if (enableAnimation && isValidTextNode) 条件分支中进行惰性求值(Short-circuiting),并使用 Object.prototype.hasOwnProperty.call 安全地检查自定义组件,从而同时提升性能和安全性。

Suggested change
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;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (shouldReplaceText) {
return React.createElement(AnimationText, { text: domNode.data, key, animationConfig });
}
Expand Down
6 changes: 6 additions & 0 deletions packages/x-markdown/src/XMarkdown/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ interface StreamingOption {
string
>
>;
/**
* @description 是否在自定义组件内部也启用文字淡入动画
* @description Whether to enable text fade-in animation inside custom components
* @default false
*/
animateInsideComponents?: boolean;
}

type StreamStatus = 'loading' | 'done';
Expand Down