Refactor SpanGraph.UNSAFE_componentWillReceiveProps (jaegertracing#613)

* add test to ensure canvas graph rendering is limited

This test codifies the sentiments on SpanGraph/index.tsx#L35, confirming that canvas graph is not re-rendered unless trace is changed

Signed-off-by: Tim Klever <[email protected]>

* refactor SpanGraph to not use internal state for memoization

Using component state in this manner is considered a react anti-pattern. By removing it, not only is the size and complexity of the component reduced, but it enables the removal of the legacy method UNSAFE_componentWillReceiveProps

Signed-off-by: Tim Klever <[email protected]>
Signed-off-by: vvvprabhakar <[email protected]>

packages/jaeger-ui/src/components/TracePage/TracePageHeader/SpanGraph/index.test.js

@@ -73,4 +73,16 @@ describe('<SpanGraph>', () => {
     }));
     expect(canvasGraph.prop('items')).toEqual(items);
   });
+
+  it('does not regenerate CanvasSpanGraph without new trace', () => {
+    const canvasGraph = wrapper.find(CanvasSpanGraph).first();
+    const items = canvasGraph.prop('items');
+
+    wrapper.instance().forceUpdate();
+
+    const newCanvasGraph = wrapper.find(CanvasSpanGraph).first();
+    const newItems = newCanvasGraph.prop('items');
+
+    expect(newItems).toBe(items);
+  });
 });

packages/jaeger-ui/src/components/TracePage/TracePageHeader/SpanGraph/index.tsx

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 import * as React from 'react';
+import memoizeOne from 'memoize-one';
 
 import CanvasSpanGraph from './CanvasSpanGraph';
 import TickLabels from './TickLabels';
@@ -31,57 +32,38 @@ type SpanGraphProps = {
   updateNextViewRangeTime: (nextUpdate: ViewRangeTimeUpdate) => void;
 };
 
-/**
- * Store `items` in state so they are not regenerated every render. Otherwise,
- * the canvas graph will re-render itself every time.
- */
-type SpanGraphState = {
-  items: {
-    valueOffset: number;
-    valueWidth: number;
-    serviceName: string;
-  }[];
+type SpanItem = {
+  valueOffset: number;
+  valueWidth: number;
+  serviceName: string;
 };
 
-function getItem(span: Span) {
+function getItem(span: Span): SpanItem {
   return {
     valueOffset: span.relativeStartTime,
     valueWidth: span.duration,
     serviceName: span.process.serviceName,
   };
 }
 
-export default class SpanGraph extends React.PureComponent<SpanGraphProps, SpanGraphState> {
-  state: SpanGraphState;
+function getItems(trace: Trace): SpanItem[] {
+  return trace.spans.map(getItem);
+}
+
+const memoizedGetItems = memoizeOne(getItems);
 
+export default class SpanGraph extends React.PureComponent<SpanGraphProps> {
   static defaultProps = {
     height: DEFAULT_HEIGHT,
   };
 
-  constructor(props: SpanGraphProps) {
-    super(props);
-    const { trace } = props;
-    this.state = {
-      items: trace ? trace.spans.map(getItem) : [],
-    };
-  }
-
-  // eslint-disable-next-line camelcase
-  UNSAFE_componentWillReceiveProps(nextProps: SpanGraphProps) {
-    const { trace } = nextProps;
-    if (this.props.trace !== trace) {
-      this.setState({
-        items: trace ? trace.spans.map(getItem) : [],
-      });
-    }
-  }
-
   render() {
     const { height, trace, viewRange, updateNextViewRangeTime, updateViewRangeTime } = this.props;
     if (!trace) {
       return <div />;
     }
-    const { items } = this.state;
+
+    const items = memoizedGetItems(trace);
     return (
       <div className="ub-pb2 ub-px2">
         <TickLabels numTicks={TIMELINE_TICK_INTERVAL} duration={trace.duration} />