Skip to content

Commit 49b9cdf

Browse files
committed
refactor(plexus): convert SvgEdges to functional component
Convert SvgEdges from class-based to functional component using React.memo with custom comparator to preserve the original shouldComponentUpdate behavior. Resolves #3397 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent 512ed41 commit 49b9cdf

File tree

2 files changed

+69
-40
lines changed

2 files changed

+69
-40
lines changed

packages/plexus/src/Digraph/SvgEdges.test.js

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,27 @@ import React from 'react';
55
import { render } from '@testing-library/react';
66
import SvgEdges from './SvgEdges';
77

8-
// mock SvgEdge 子組件,這樣測試才不會受到子組件的影響
8+
// Mock SvgEdge child component
9+
const mockSvgEdgeProps = [];
910
jest.mock('./SvgEdge', () => {
10-
const MockSvgEdge = props => (
11-
<g data-testid="svg-edge" data-from={props.layoutEdge.edge.from} data-to={props.layoutEdge.edge.to} />
12-
);
11+
const MockSvgEdge = props => {
12+
mockSvgEdgeProps.push(props);
13+
return (
14+
<g data-testid="svg-edge" data-from={props.layoutEdge.edge.from} data-to={props.layoutEdge.edge.to} />
15+
);
16+
};
1317
return MockSvgEdge;
1418
});
1519

1620
describe('SvgEdges', () => {
17-
// 準備測試用的 mock 資料
1821
const mockGetClassName = name => `plexus--${name}`;
1922

2023
const mockRenderUtils = {
2124
getGlobalId: id => `global-${id}`,
2225
getZoomTransform: () => ({ k: 1, x: 0, y: 0 }),
2326
};
2427

25-
// 建立幾條測試用的 edge
28+
// Create mock edges for testing
2629
const createMockEdge = (from, to) => ({
2730
edge: { from, to },
2831
pathPoints: [
@@ -33,7 +36,7 @@ describe('SvgEdges', () => {
3336

3437
const mockLayoutEdges = [createMockEdge('node-a', 'node-b'), createMockEdge('node-b', 'node-c')];
3538

36-
// 包一個 svg 容器,因為 SvgEdge 會渲染 SVG 元素
39+
// Wrap in svg container since SvgEdge renders SVG elements
3740
const renderInSvg = edges => {
3841
return render(
3942
<svg>
@@ -58,17 +61,16 @@ describe('SvgEdges', () => {
5861
const { getAllByTestId } = renderInSvg(mockLayoutEdges);
5962
const edges = getAllByTestId('svg-edge');
6063

61-
// 檢查第一條 edge
64+
// Check first edge
6265
expect(edges[0]).toHaveAttribute('data-from', 'node-a');
6366
expect(edges[0]).toHaveAttribute('data-to', 'node-b');
6467

65-
// 檢查第二條
68+
// Check second edge
6669
expect(edges[1]).toHaveAttribute('data-from', 'node-b');
6770
expect(edges[1]).toHaveAttribute('data-to', 'node-c');
6871
});
6972

7073
it('handles optional marker props', () => {
71-
// 這個測試確認有 marker 的情況也能正常 render
7274
const { getAllByTestId } = render(
7375
<svg>
7476
<SvgEdges
@@ -80,7 +82,27 @@ describe('SvgEdges', () => {
8082
/>
8183
</svg>
8284
);
83-
// 只要不 crash 就算過
8485
expect(getAllByTestId('svg-edge')).toHaveLength(2);
8586
});
87+
88+
it('passes setOnEdge to child components', () => {
89+
mockSvgEdgeProps.length = 0;
90+
const mockSetOnEdge = jest.fn();
91+
const { getAllByTestId } = render(
92+
<svg>
93+
<SvgEdges
94+
getClassName={mockGetClassName}
95+
layoutEdges={mockLayoutEdges}
96+
renderUtils={mockRenderUtils}
97+
setOnEdge={mockSetOnEdge}
98+
/>
99+
</svg>
100+
);
101+
expect(getAllByTestId('svg-edge')).toHaveLength(2);
102+
103+
// Verify setOnEdge is passed to each child component
104+
mockSvgEdgeProps.forEach(props => {
105+
expect(props.setOnEdge).toBe(mockSetOnEdge);
106+
});
107+
});
86108
});
Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2019 Uber Technologies, Inc.
1+
// Copyright (c) 2026 The Jaeger Authors.
22
// SPDX-License-Identifier: Apache-2.0
33

44
import * as React from 'react';
@@ -17,32 +17,39 @@ type TProps<T = {}> = {
1717
setOnEdge?: TSetProps<(edge: TLayoutEdge<T>, utils: TRendererUtils) => TAnyProps | null>;
1818
};
1919

20-
export default class SvgEdges<T = {}> extends React.Component<TProps<T>> {
21-
shouldComponentUpdate(np: TProps<T>) {
22-
const p = this.props;
23-
return (
24-
p.getClassName !== np.getClassName ||
25-
p.layoutEdges !== np.layoutEdges ||
26-
p.markerEndId !== np.markerEndId ||
27-
p.markerStartId !== np.markerStartId ||
28-
p.renderUtils !== np.renderUtils ||
29-
!isSamePropSetter(p.setOnEdge, np.setOnEdge)
30-
);
31-
}
32-
33-
render() {
34-
const { getClassName, layoutEdges, markerEndId, markerStartId, renderUtils, setOnEdge } = this.props;
35-
return layoutEdges.map(edge => (
36-
<SvgEdge
37-
key={`${edge.edge.from}\v${edge.edge.to}`}
38-
getClassName={getClassName}
39-
layoutEdge={edge}
40-
markerEndId={markerEndId}
41-
markerStartId={markerStartId}
42-
renderUtils={renderUtils}
43-
setOnEdge={setOnEdge}
44-
label={edge.edge.label}
45-
/>
46-
));
47-
}
20+
// Comparison function that mirrors the original shouldComponentUpdate logic
21+
// Returns true if props are equal (no re-render needed)
22+
function arePropsEqual<T>(prev: TProps<T>, next: TProps<T>): boolean {
23+
return (
24+
prev.getClassName === next.getClassName &&
25+
prev.layoutEdges === next.layoutEdges &&
26+
prev.markerEndId === next.markerEndId &&
27+
prev.markerStartId === next.markerStartId &&
28+
prev.renderUtils === next.renderUtils &&
29+
isSamePropSetter(prev.setOnEdge, next.setOnEdge)
30+
);
4831
}
32+
33+
const SvgEdges = <T = {},>({
34+
getClassName,
35+
layoutEdges,
36+
markerEndId,
37+
markerStartId,
38+
renderUtils,
39+
setOnEdge,
40+
}: TProps<T>) => {
41+
return layoutEdges.map(edge => (
42+
<SvgEdge
43+
key={`${edge.edge.from}\v${edge.edge.to}`}
44+
getClassName={getClassName}
45+
layoutEdge={edge}
46+
markerEndId={markerEndId}
47+
markerStartId={markerStartId}
48+
renderUtils={renderUtils}
49+
setOnEdge={setOnEdge}
50+
label={edge.edge.label}
51+
/>
52+
));
53+
};
54+
55+
export default React.memo(SvgEdges, arePropsEqual) as unknown as typeof SvgEdges;

0 commit comments

Comments
 (0)