Skip to content

Commit

Permalink
fix: replace PureComponent with shouldComponentUpdate (#534)
Browse files Browse the repository at this point in the history
This commit replaces React.PureComponent with React.Component with custom shouldComponentUpdate that deeply compares props.
Also, it fixes shouldComponentUpdate logic in ChartContainerComponent that caused the component to re-render every time the props have changed.
  • Loading branch information
patrykkopycinski authored Feb 5, 2020
1 parent 178afc9 commit 5043725
Show file tree
Hide file tree
Showing 16 changed files with 99 additions and 30 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@
"d3-color": "^1.4.0",
"d3-scale": "^1.0.7",
"d3-shape": "^1.3.4",
"fast-deep-equal": "^3.1.1",
"konva": "^4.0.18",
"newtype-ts": "^0.2.4",
"prop-types": "^15.7.2",
Expand Down
15 changes: 8 additions & 7 deletions src/chart_types/xy_chart/renderer/canvas/area_geometries.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import { Group as KonvaGroup } from 'konva/types/Group';
import { PathConfig } from 'konva/types/shapes/Path';
import { Circle, Group, Path } from 'react-konva';
import deepEqual from 'fast-deep-equal/es6/react';
import {
buildAreaRenderProps,
buildPointStyleProps,
Expand All @@ -24,21 +25,21 @@ interface AreaGeometriesDataProps {
highlightedLegendItem: LegendItem | null;
clippings: Clippings;
}
interface AreaGeometriesDataState {
overPoint?: PointGeometry;
}
export class AreaGeometries extends React.PureComponent<AreaGeometriesDataProps, AreaGeometriesDataState> {

export class AreaGeometries extends React.Component<AreaGeometriesDataProps> {
static defaultProps: Partial<AreaGeometriesDataProps> = {
animated: false,
};
private readonly barSeriesRef: React.RefObject<KonvaGroup> = React.createRef();
constructor(props: AreaGeometriesDataProps) {
super(props);
this.barSeriesRef = React.createRef();
this.state = {
overPoint: undefined,
};
}

shouldComponentUpdate(nextProps: AreaGeometriesDataProps) {
return !deepEqual(this.props, nextProps);
}

render() {
return (
<Group ref={this.barSeriesRef} key={'bar_series'}>
Expand Down
13 changes: 11 additions & 2 deletions src/chart_types/xy_chart/renderer/canvas/axis.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { Group, Line, Rect, Text } from 'react-konva';
import deepEqual from 'fast-deep-equal/es6/react';
import {
AxisTick,
AxisTicksDimensions,
Expand Down Expand Up @@ -36,7 +37,11 @@ interface AxisProps {
chartDimensions: Dimensions;
}

export class Axis extends React.PureComponent<AxisProps> {
export class Axis extends React.Component<AxisProps> {
shouldComponentUpdate(nextProps: AxisProps) {
return !deepEqual(this.props, nextProps);
}

private renderTickLabel = (tick: AxisTick, i: number) => {
/**
* padding is already computed through width
Expand Down Expand Up @@ -249,7 +254,11 @@ interface AxesProps {
debug: boolean;
chartDimensions: Dimensions;
}
class AxesComponent extends React.PureComponent<AxesProps> {
class AxesComponent extends React.Component<AxesProps> {
shouldComponentUpdate(nextProps: AxesProps) {
return !deepEqual(this.props, nextProps);
}

render() {
const {
axesVisibleTicks,
Expand Down
8 changes: 7 additions & 1 deletion src/chart_types/xy_chart/renderer/canvas/bar_geometries.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Group as KonvaGroup } from 'konva/types/Group';
import React from 'react';
import { Group, Rect } from 'react-konva';
import { animated, Spring } from 'react-spring/renderprops-konva.cjs';
import deepEqual from 'fast-deep-equal/es6/react';
import { buildBarRenderProps, buildBarBorderRenderProps } from './utils/rendering_props_utils';
import { BarGeometry } from '../../../../utils/geometry';
import { LegendItem } from '../../../../chart_types/xy_chart/legend/legend';
Expand All @@ -19,7 +20,7 @@ interface BarGeometriesDataProps {
interface BarGeometriesDataState {
overBar?: BarGeometry;
}
export class BarGeometries extends React.PureComponent<BarGeometriesDataProps, BarGeometriesDataState> {
export class BarGeometries extends React.Component<BarGeometriesDataProps, BarGeometriesDataState> {
static defaultProps: Partial<BarGeometriesDataProps> = {
animated: false,
};
Expand All @@ -31,6 +32,11 @@ export class BarGeometries extends React.PureComponent<BarGeometriesDataProps, B
overBar: undefined,
};
}

shouldComponentUpdate(nextProps: BarGeometriesDataProps, nextState: BarGeometriesDataState) {
return !deepEqual(this.props, nextProps) || !deepEqual(this.state, nextState);
}

render() {
const { bars, clippings } = this.props;
return (
Expand Down
7 changes: 6 additions & 1 deletion src/chart_types/xy_chart/renderer/canvas/bar_values.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { Group, Rect, Text } from 'react-konva';
import deepEqual from 'fast-deep-equal/es6/react';
import { Rotation } from '../../utils/specs';
import { Theme } from '../../../../utils/themes/theme';
import { Dimensions } from '../../../../utils/dimensions';
Expand All @@ -23,7 +24,11 @@ interface BarValuesProps {
bars: BarGeometry[];
}

export class BarValuesComponent extends React.PureComponent<BarValuesProps> {
export class BarValuesComponent extends React.Component<BarValuesProps> {
shouldComponentUpdate(nextProps: BarValuesProps) {
return !deepEqual(this.props, nextProps);
}

render() {
const { chartDimensions, bars } = this.props;
if (!bars) {
Expand Down
7 changes: 6 additions & 1 deletion src/chart_types/xy_chart/renderer/canvas/grid.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { Group, Line } from 'react-konva';
import { connect } from 'react-redux';
import deepEqual from 'fast-deep-equal/es6/react';
import { AxisLinePosition, isVerticalGrid } from '../../utils/axis_utils';
import { GridLineConfig, mergeGridLineConfigs, Theme } from '../../../../utils/themes/theme';
import { Dimensions } from '../../../../utils/dimensions';
Expand All @@ -22,7 +23,11 @@ interface GridProps {
chartDimensions: Dimensions;
}

class GridComponent extends React.PureComponent<GridProps> {
class GridComponent extends React.Component<GridProps> {
shouldComponentUpdate(nextProps: GridProps) {
return !deepEqual(this.props, nextProps);
}

render() {
const { axesGridLinesPositions, axesSpecs, chartDimensions, chartTheme } = this.props;
const gridComponents: JSX.Element[] = [];
Expand Down
7 changes: 6 additions & 1 deletion src/chart_types/xy_chart/renderer/canvas/line_annotation.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { Group, Line } from 'react-konva';
import deepEqual from 'fast-deep-equal/es6/react';
import { LineAnnotationStyle } from '../../../../utils/themes/theme';
import { AnnotationLineProps } from '../../annotations/line_annotation_tooltip';

Expand All @@ -8,7 +9,11 @@ interface LineAnnotationProps {
lineStyle: LineAnnotationStyle;
}

export class LineAnnotation extends React.PureComponent<LineAnnotationProps> {
export class LineAnnotation extends React.Component<LineAnnotationProps> {
shouldComponentUpdate(nextProps: LineAnnotationProps) {
return !deepEqual(this.props, nextProps);
}

render() {
const { lines } = this.props;

Expand Down
11 changes: 7 additions & 4 deletions src/chart_types/xy_chart/renderer/canvas/line_geometries.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { Group as KonvaGroup } from 'konva/types/Group';
import { Circle, Group, Path } from 'react-konva';
import deepEqual from 'fast-deep-equal/es6/react';
import {
buildLineRenderProps,
buildPointStyleProps,
Expand All @@ -21,10 +22,8 @@ interface LineGeometriesDataProps {
highlightedLegendItem: LegendItem | null;
clippings: Clippings;
}
interface LineGeometriesDataState {
overPoint?: PointGeometry;
}
export class LineGeometries extends React.PureComponent<LineGeometriesDataProps, LineGeometriesDataState> {

export class LineGeometries extends React.Component<LineGeometriesDataProps> {
static defaultProps: Partial<LineGeometriesDataProps> = {
animated: false,
};
Expand All @@ -37,6 +36,10 @@ export class LineGeometries extends React.PureComponent<LineGeometriesDataProps,
};
}

shouldComponentUpdate(nextProps: LineGeometriesDataProps) {
return !deepEqual(this.props, nextProps);
}

render() {
return (
<Group ref={this.barSeriesRef} key={'bar_series'}>
Expand Down
7 changes: 6 additions & 1 deletion src/chart_types/xy_chart/renderer/canvas/rect_annotation.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { Group, Rect } from 'react-konva';
import deepEqual from 'fast-deep-equal/es6';
import { RectAnnotationStyle } from '../../../../utils/themes/theme';
import { AnnotationRectProps } from '../../annotations/rect_annotation_tooltip';

Expand All @@ -8,7 +9,11 @@ interface Props {
rectStyle: RectAnnotationStyle;
}

export class RectAnnotation extends React.PureComponent<Props> {
export class RectAnnotation extends React.Component<Props> {
shouldComponentUpdate(nextProps: Props) {
return !deepEqual(this.props, nextProps);
}

render() {
const { rects } = this.props;
return <Group>{rects.map(this.renderAnnotationRect)}</Group>;
Expand Down
10 changes: 8 additions & 2 deletions src/chart_types/xy_chart/specs/line_annotation.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { createRef, CSSProperties, PureComponent } from 'react';
import React, { createRef, CSSProperties, Component } from 'react';
import deepEqual from 'fast-deep-equal/es6/react';
import { LineAnnotationSpec, DEFAULT_GLOBAL_ID, AnnotationTypes } from '../utils/specs';
import { DEFAULT_ANNOTATION_LINE_STYLE } from '../../../utils/themes/theme';
import { bindActionCreators, Dispatch } from 'redux';
Expand All @@ -12,7 +13,7 @@ type InjectedProps = LineAnnotationSpec &
Readonly<{
children?: React.ReactNode;
}>;
export class LineAnnotationSpecComponent extends PureComponent<LineAnnotationSpec> {
export class LineAnnotationSpecComponent extends Component<LineAnnotationSpec> {
static defaultProps: Partial<LineAnnotationSpec> = {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Annotation,
Expand All @@ -38,6 +39,11 @@ export class LineAnnotationSpecComponent extends PureComponent<LineAnnotationSpe
}
upsertSpec({ ...config });
}

shouldComponentUpdate(nextProps: LineAnnotationSpec) {
return !deepEqual(this.props, nextProps);
}

componentDidUpdate() {
const { upsertSpec, removeSpec, children, ...config } = this.props as InjectedProps;
if (this.markerRef.current) {
Expand Down
6 changes: 4 additions & 2 deletions src/components/chart_container.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { bindActionCreators, Dispatch } from 'redux';
import { connect } from 'react-redux';
import deepEqual from 'fast-deep-equal/es6/react';
import { GlobalChartState, BackwardRef } from '../state/chart_state';
import { onMouseUp, onMouseDown, onPointerMove } from '../state/actions/mouse';
import { getInternalChartRendererSelector } from '../state/selectors/get_chart_type_components';
Expand Down Expand Up @@ -38,9 +39,10 @@ type ReactiveChartProps = ReactiveChartStateProps & ReactiveChartDispatchProps &
class ChartContainerComponent extends React.Component<ReactiveChartProps> {
static displayName = 'ChartContainer';

shouldComponentUpdate(props: ReactiveChartProps) {
return props.initialized;
shouldComponentUpdate(nextProps: ReactiveChartProps) {
return !deepEqual(this.props, nextProps);
}

handleMouseMove = ({
nativeEvent: { offsetX, offsetY, timeStamp },
}: React.MouseEvent<HTMLDivElement, MouseEvent>) => {
Expand Down
7 changes: 6 additions & 1 deletion src/components/icons/assets/dot.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import React from 'react';
import deepEqual from 'fast-deep-equal/es6/react';
import { Props } from '../icon';

export class DotIcon extends React.PureComponent<Props> {
export class DotIcon extends React.Component<Props> {
shouldComponentUpdate(nextProps: Props) {
return !deepEqual(this.props, nextProps);
}

render() {
return (
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" {...this.props}>
Expand Down
7 changes: 6 additions & 1 deletion src/components/icons/icon.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import classNames from 'classnames';
import React, { SVGAttributes } from 'react';
import deepEqual from 'fast-deep-equal/es6/react';
import { AlertIcon } from './assets/alert';
import { DotIcon } from './assets/dot';
import { EmptyIcon } from './assets/empty';
Expand Down Expand Up @@ -31,7 +32,11 @@ export interface IconProps {

export type Props = Omit<SVGAttributes<SVGElement>, 'color' | 'type'> & IconProps;

export class Icon extends React.PureComponent<Props> {
export class Icon extends React.Component<Props> {
shouldComponentUpdate(nextProps: Props) {
return !deepEqual(this.props, nextProps);
}

render() {
const { type, color, className, tabIndex, ...rest } = this.props;
let optionalCustomStyles = null;
Expand Down
11 changes: 6 additions & 5 deletions src/components/legend/legend_item.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import classNames from 'classnames';
import React from 'react';
import deepEqual from 'fast-deep-equal/es6/react';
import { Icon } from '../icons/icon';
import { LegendItemListener, BasicListener } from '../../specs/settings';
import { LegendItem } from '../../chart_types/xy_chart/legend/legend';
Expand All @@ -22,10 +23,6 @@ interface LegendItemProps {
toggleDeselectSeriesAction: (legendItemId: SeriesIdentifier) => void;
}

interface LegendItemState {
isColorPickerOpen: boolean;
}

/**
* Create a div for the the displayed value
* @param displayValue
Expand Down Expand Up @@ -92,9 +89,13 @@ function renderColor(color: string, isSeriesVisible = true) {
);
}

export class LegendListItem extends React.PureComponent<LegendItemProps, LegendItemState> {
export class LegendListItem extends React.Component<LegendItemProps> {
static displayName = 'LegendItem';

shouldComponentUpdate(nextProps: LegendItemProps) {
return !deepEqual(this.props, nextProps);
}

render() {
const { displayValue, legendItem, legendPosition, label } = this.props;
const { color, isSeriesVisible, seriesIdentifier, isLegendItemVisible } = legendItem;
Expand Down
7 changes: 6 additions & 1 deletion src/components/react_canvas/grid.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { Group, Line } from 'react-konva';
import deepEqual from 'fast-deep-equal/es6/react';
import { AxisLinePosition } from '../../chart_types/xy_chart/utils/axis_utils';
import { GridLineConfig } from '../../utils/themes/theme';
import { Dimensions } from '../../utils/dimensions';
Expand All @@ -11,7 +12,11 @@ interface GridProps {
linesPositions: AxisLinePosition[];
}

export class Grid extends React.PureComponent<GridProps> {
export class Grid extends React.Component<GridProps> {
shouldComponentUpdate(nextProps: GridProps) {
return !deepEqual(this.props, nextProps);
}

render() {
return this.renderGrid();
}
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7535,6 +7535,11 @@ fast-deep-equal@^2.0.1:
resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-2.0.1.tgz#7b05218ddf9667bf7f370bf7fdb2cb15fdd0aa49"
integrity sha1-ewUhjd+WZ79/Nwv3/bLLFf3Qqkk=

fast-deep-equal@^3.1.1:
version "3.1.1"
resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-3.1.1.tgz#545145077c501491e33b15ec408c294376e94ae4"
integrity sha512-8UEa58QDLauDNfpbrX55Q9jrGHThw2ZMdOky5Gl1CDtVeJDPVrG4Jxx1N8jw2gkWaff5UUuX1KJd+9zGe2B+ZA==

fast-diff@^1.1.2:
version "1.2.0"
resolved "https://registry.yarnpkg.com/fast-diff/-/fast-diff-1.2.0.tgz#73ee11982d86caaf7959828d519cfe927fac5f03"
Expand Down

0 comments on commit 5043725

Please sign in to comment.