Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
23 changes: 14 additions & 9 deletions packages/x/components/mermaid/Mermaid.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { DownloadOutlined, ZoomInOutlined, ZoomOutOutlined } from '@ant-design/icons';
import { Button, Segmented, Tooltip } from 'antd';
import { clsx } from 'clsx';
import throttle from 'lodash.throttle';
import { debounce } from 'lodash';
import mermaid, { type MermaidConfig } from 'mermaid';
import React, { useEffect, useRef, useState } from 'react';
import React, { useEffect, useId, useRef, useState } from 'react';
import useXComponentConfig from '../_util/hooks/use-x-component-config';
import warning from '../_util/warning';
import Actions from '../actions';
Expand Down Expand Up @@ -42,8 +42,6 @@ enum RenderType {
Image = 'image',
}

let uuid = 0;

const Mermaid: React.FC<MermaidProps> = React.memo((props) => {
const {
prefixCls: customizePrefixCls,
Expand All @@ -64,7 +62,8 @@ const Mermaid: React.FC<MermaidProps> = React.memo((props) => {
const [isDragging, setIsDragging] = useState(false);
const [lastMousePos, setLastMousePos] = useState({ x: 0, y: 0 });
const containerRef = useRef<HTMLDivElement>(null);
const id = `mermaid-${uuid++}-${children?.length || 0}`;
const reactId = useId();
const id = `mermaid-${reactId.replace(/:/g, '')}`;

// ============================ locale ============================
const [contextLocale] = useLocale('Mermaid', locale_EN.Mermaid);
Expand Down Expand Up @@ -103,19 +102,22 @@ const Mermaid: React.FC<MermaidProps> = React.memo((props) => {
}, [config]);

// ============================ render mermaid ============================
const renderDiagram = throttle(async () => {
const renderDiagram = debounce(async () => {
if (!children || !containerRef.current || renderType === RenderType.Code) return;

try {
const isValid = await mermaid.parse(children, { suppressErrors: true });
if (!isValid) throw new Error('Invalid Mermaid syntax');
if (!isValid) return; // 流式渲染过程中语法不完整时静默跳过,不报错

const { svg } = await mermaid.render(id, children);
containerRef.current.innerHTML = svg;
// 确保渲染完成后容器仍然存在(防止异步过程中组件已卸载)
if (containerRef.current) {
containerRef.current.innerHTML = svg;
}
} catch (error) {
warning(false, 'Mermaid', `Render failed: ${error}`);
}
}, 100);
}, 200);
Comment on lines +105 to +120

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.

high

在当前的实现中,renderDiagram 函数在每次组件渲染时都会被重新创建(即每次都会调用 debounce(...) 生成一个新的防抖函数实例)。

由于 Mermaid 组件在拖拽(isDraggingposition 改变)或缩放(scale 改变)时会频繁触发重新渲染,这会导致在拖拽/缩放过程中频繁、重复地创建新的 debounce 函数,造成不必要的内存开销和垃圾回收压力。

此外,由于每次渲染都是全新的 debounce 实例,防抖功能实际上完全依赖于 useEffect 的清理函数来手动调用 .cancel()。这不仅不符合 React 的声明式设计,也容易在复杂的生命周期中引入潜在的竞态问题。

建议:
使用 React.useMemo 配合 useRef 来缓存最新的 childrenrenderType,从而保持 renderDiagram 的引用在多次渲染之间保持稳定。

  const latestPropsRef = useRef({ children, renderType });
  latestPropsRef.current = { children, renderType };

  const renderDiagram = React.useMemo(() => {
    return debounce(async () => {
      const { children: currentChildren, renderType: currentRenderType } = latestPropsRef.current;
      if (!currentChildren || !containerRef.current || currentRenderType === RenderType.Code) return;

      try {
        const isValid = await mermaid.parse(currentChildren, { suppressErrors: true });
        if (!isValid) return; // 流式渲染过程中语法不完整时静默跳过,不报错

        const { svg } = await mermaid.render(id, currentChildren);
        // 确保渲染完成后容器仍然存在(防止异步过程中组件已卸载)
        if (containerRef.current) {
          containerRef.current.innerHTML = svg;
        }
      } catch (error) {
        warning(false, 'Mermaid', 'Render failed: ' + error);
      }
    }, 200);
  }, [id]);

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.

@Somtry cc


useEffect(() => {
if (renderType === RenderType.Code && containerRef.current) {
Expand All @@ -124,6 +126,9 @@ const Mermaid: React.FC<MermaidProps> = React.memo((props) => {
} else {
renderDiagram();
}
return () => {
renderDiagram.cancel();
};
}, [children, renderType, config]);

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

为了遵循 React Hooks 的依赖规范(react-hooks/exhaustive-deps),建议将稳定后的 renderDiagram 函数也加入到 useEffect 的依赖数组中。

Suggested change
}, [children, renderType, config]);
}, [children, renderType, config, renderDiagram]);


useEffect(() => {
Expand Down
105 changes: 103 additions & 2 deletions packages/x/components/mermaid/__tests__/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,7 @@ describe('Mermaid Component', () => {
});

describe('Performance Tests', () => {
it('should throttle render calls', async () => {
it('should debounce render calls', async () => {
const { rerender } = render(<Mermaid>{mermaidContent}</Mermaid>);

// 快速连续改变内容
Expand All @@ -908,7 +908,7 @@ describe('Mermaid Component', () => {
}

await waitFor(() => {
// 由于节流,render调用次数应该少于内容变化次数
// 由于防抖,连续快速更新后最终只触发一次渲染
expect(mockRender).toHaveBeenCalled();
});
});
Expand Down Expand Up @@ -1206,4 +1206,105 @@ describe('Mermaid Component', () => {
expect(element).toBeInTheDocument();
});
});

// Regression tests for issue #1947: Mermaid streaming render jitter
describe('Streaming Render Stability (issue #1947)', () => {
it('should keep stable id across re-renders', async () => {
const { rerender } = render(<Mermaid>{mermaidContent}</Mermaid>);

await waitFor(() => {
expect(mockRender).toHaveBeenCalled();
});

const firstCallId = mockRender.mock.calls[0][0];

// Simulate streaming content change
rerender(<Mermaid>{`${mermaidContent}\n B-->C;`}</Mermaid>);

await waitFor(() => {
expect(mockRender).toHaveBeenCalledTimes(2);
});

const secondCallId = mockRender.mock.calls[1][0];

// id should be the same across re-renders
expect(secondCallId).toBe(firstCallId);
});

it('should not use uuid++ that changes every render', async () => {
const { rerender } = render(<Mermaid>{mermaidContent}</Mermaid>);

await waitFor(() => {
expect(mockRender).toHaveBeenCalled();
});

// Record id from first render
const ids: string[] = [mockRender.mock.calls[0][0]];

// Multiple re-renders simulating streaming
for (let i = 0; i < 5; i++) {
rerender(<Mermaid>{`${mermaidContent} ${i}`}</Mermaid>);
await waitFor(() => {
expect(mockRender).toHaveBeenCalled();
});
const lastCall = mockRender.mock.calls[mockRender.mock.calls.length - 1];
ids.push(lastCall[0]);
}

// All ids should be identical
const uniqueIds = new Set(ids);
expect(uniqueIds.size).toBe(1);
});
Comment on lines +1234 to +1257

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

循环内 toHaveBeenCalled() 断言无法确认新渲染真正发生。

mockRender 在循环开始前已被调用过一次,expect(mockRender).toHaveBeenCalled() 在后续每次迭代中都会立即为真(无需等待本轮 200ms 防抖真正触发),因此 waitFor 可能在新渲染尚未发生前就返回,lastCall 可能重复取到之前的调用。即便实现存在回归(例如后续渲染完全没有触发),该测试仍可能因为 uniqueIds.size === 1 恒成立而“通过”,无法真正验证多次流式渲染都保持相同 id。

💚 建议的修复
       for (let i = 0; i < 5; i++) {
         rerender(<Mermaid>{`${mermaidContent} ${i}`}</Mermaid>);
         await waitFor(() => {
-          expect(mockRender).toHaveBeenCalled();
+          expect(mockRender).toHaveBeenCalledTimes(i + 2);
         });
         const lastCall = mockRender.mock.calls[mockRender.mock.calls.length - 1];
         ids.push(lastCall[0]);
       }
📝 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.

Suggested change
it('should not use uuid++ that changes every render', async () => {
const { rerender } = render(<Mermaid>{mermaidContent}</Mermaid>);
await waitFor(() => {
expect(mockRender).toHaveBeenCalled();
});
// Record id from first render
const ids: string[] = [mockRender.mock.calls[0][0]];
// Multiple re-renders simulating streaming
for (let i = 0; i < 5; i++) {
rerender(<Mermaid>{`${mermaidContent} ${i}`}</Mermaid>);
await waitFor(() => {
expect(mockRender).toHaveBeenCalled();
});
const lastCall = mockRender.mock.calls[mockRender.mock.calls.length - 1];
ids.push(lastCall[0]);
}
// All ids should be identical
const uniqueIds = new Set(ids);
expect(uniqueIds.size).toBe(1);
});
it('should not use uuid++ that changes every render', async () => {
const { rerender } = render(<Mermaid>{mermaidContent}</Mermaid>);
await waitFor(() => {
expect(mockRender).toHaveBeenCalled();
});
// Record id from first render
const ids: string[] = [mockRender.mock.calls[0][0]];
// Multiple re-renders simulating streaming
for (let i = 0; i < 5; i++) {
rerender(<Mermaid>{`${mermaidContent} ${i}`}</Mermaid>);
await waitFor(() => {
expect(mockRender).toHaveBeenCalledTimes(i + 2);
});
const lastCall = mockRender.mock.calls[mockRender.mock.calls.length - 1];
ids.push(lastCall[0]);
}
// All ids should be identical
const uniqueIds = new Set(ids);
expect(uniqueIds.size).toBe(1);
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/x/components/mermaid/__tests__/index.test.tsx` around lines 1234 -
1257, Update the re-render loop in the “should not use uuid++ that changes every
render” test to wait for mockRender to receive a new call after each rerender,
rather than merely asserting it has ever been called. Track the call count
before rerender and require it to increase before reading the latest call, so
the test verifies every debounced streaming render occurred and retained the
same id.


it('should have different ids for different component instances', async () => {
render(
<div>
<Mermaid key="1">{mermaidContent}</Mermaid>
<Mermaid key="2">{mermaidContent}</Mermaid>
</div>,
);

await waitFor(() => {
expect(mockRender).toHaveBeenCalledTimes(2);
});

const id1 = mockRender.mock.calls[0][0];
const id2 = mockRender.mock.calls[1][0];

// Different instances should have different ids
expect(id1).not.toBe(id2);
});

it('should skip render when mermaid syntax is incomplete (streaming)', async () => {
// Simulate incomplete syntax during streaming
mockParse.mockResolvedValue(false);

const incompleteCode = 'graph TD; A--' + '>';
const { rerender } = render(<Mermaid>{incompleteCode}</Mermaid>);

// Wait for debounce
await waitFor(() => {
expect(mockParse).toHaveBeenCalled();
});

// mockRender should NOT be called because syntax is invalid
expect(mockRender).not.toHaveBeenCalled();

// Now syntax becomes complete
mockParse.mockResolvedValue(true);
const completeCode = 'graph TD; A--' + '>B;';
rerender(<Mermaid>{completeCode}</Mermaid>);

await waitFor(() => {
expect(mockRender).toHaveBeenCalled();
});
});

it('should cancel pending debounced render on unmount', async () => {
const { unmount } = render(<Mermaid>{mermaidContent}</Mermaid>);

// Unmount before debounce fires
expect(() => unmount()).not.toThrow();
});
});
});