Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove componentWillReceiveProps #1239

Merged
merged 11 commits into from
Jan 26, 2019
Merged
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: 13 additions & 10 deletions demo/components/performance.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React from "react";
import { range } from "lodash";
import { VictorySelectionContainer } from "../../packages/victory-selection-container/src/index";
import { VictoryScatter } from "../../packages/victory-scatter/src/index";
import { VictoryChart } from "../../packages/victory-chart/src/index";

const scatterData = range(4000).map(() => ({ x: Math.random(), y: Math.random() }));

Expand Down Expand Up @@ -53,13 +54,7 @@ class App extends React.Component {
return (
<div className="demo">
<div style={containerStyle}>
<VictoryScatter
style={{
parent: chartStyle.parent,
data: {
fill: (datum, active) => (active ? "tomato" : "black")
}
}}
<VictoryChart
containerComponent={
<VictorySelectionContainer
selectionStyle={{
Expand All @@ -70,9 +65,17 @@ class App extends React.Component {
}}
/>
}
size={(datum, active) => (active ? 5 : 3)}
data={scatterData}
/>
style={{ parent: chartStyle.parent }}
>
<VictoryScatter
style={{
data: {
fill: (datum, active) => (active ? "tomato" : "black")
}
}}
data={scatterData}
/>
</VictoryChart>
</div>
</div>
);
Expand Down
10 changes: 9 additions & 1 deletion packages/victory-area/src/victory-area.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ const fallbackProps = {
interpolation: "linear"
};

const options = {
components: [
{ name: "parent", index: "parent" },
{ name: "data", index: "all" },
{ name: "labels" }
]
};

class VictoryArea extends React.Component {
static animationWhitelist = ["data", "domain", "height", "padding", "style", "width"];

Expand Down Expand Up @@ -93,4 +101,4 @@ class VictoryArea extends React.Component {
}
}

export default addEvents(VictoryArea);
export default addEvents(VictoryArea, options);
13 changes: 12 additions & 1 deletion packages/victory-axis/src/victory-axis.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ const fallbackProps = {
padding: 50
};

const options = {
components: [
{ name: "axis", index: 0 },
{ name: "axisLabel", index: 0 },
{ name: "grid" },
{ name: "parent", index: "parent" },
{ name: "ticks" },
{ name: "tickLabels" }
]
};

class VictoryAxis extends React.Component {
static animationWhitelist = [
"style",
Expand Down Expand Up @@ -230,4 +241,4 @@ class VictoryAxis extends React.Component {
}
}

export default addEvents(VictoryAxis);
export default addEvents(VictoryAxis, options);
18 changes: 17 additions & 1 deletion packages/victory-box-plot/src/victory-box-plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,22 @@ const defaultData = [
{ x: 2, min: 2, q1: 5, median: 8, q3: 12, max: 15 }
];

const options = {
components: [
{ name: "min" },
{ name: "minLabels" },
{ name: "max" },
{ name: "maxLabels" },
{ name: "median" },
{ name: "medianLabels" },
{ name: "q1" },
{ name: "q1Labels" },
{ name: "q3" },
{ name: "q3Labels" },
{ name: "parent", index: "parent" }
]
};

class VictoryBoxPlot extends React.Component {
static animationWhitelist = ["data", "domain", "height", "padding", "style", "width"];

Expand Down Expand Up @@ -237,4 +253,4 @@ class VictoryBoxPlot extends React.Component {
}
}

export default addEvents(VictoryBoxPlot);
export default addEvents(VictoryBoxPlot, options);
19 changes: 11 additions & 8 deletions packages/victory-chart/src/victory-chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { VictorySharedEvents } from "victory-shared-events";
import { VictoryAxis } from "victory-axis";
import { VictoryPolarAxis } from "victory-polar-axis";
import { getChildComponents, getCalculatedProps, getChildren } from "./helper-methods";
import isEqual from "react-fast-compare";

const fallbackProps = {
width: 450,
Expand Down Expand Up @@ -65,18 +66,20 @@ export default class VictoryChart extends React.Component {
nodesDoneLoad: false,
animating: true
};
this.setAnimationState = Wrapper.setAnimationState.bind(this);
}
this.setAnimationState = Wrapper.setAnimationState.bind(this);
}

componentWillReceiveProps(nextProps) {
shouldComponentUpdate(nextProps) {
if (this.props.animate) {
this.setAnimationState(this.props, nextProps);
if (!isEqual(this.props, nextProps)) {
this.setAnimationState(this.props, nextProps);
return false;
}
}
this.events = Wrapper.getAllEvents(nextProps);
return true;
}

// the old ones were bad
getNewChildren(props, childComponents, calculatedProps) {
const children = getChildren(props, childComponents, calculatedProps);
const getAnimationProps = Wrapper.getAnimationProps.bind(this);
Expand Down Expand Up @@ -129,13 +132,13 @@ export default class VictoryChart extends React.Component {
const container = standalone
? this.renderContainer(containerComponent, containerProps)
: groupComponent;
this.events = this.events || Wrapper.getAllEvents(props);
if (!isEmpty(this.events)) {
const events = Wrapper.getAllEvents(props);
if (!isEmpty(events)) {
return (
<VictorySharedEvents
container={container}
eventKey={eventKey}
events={this.events}
events={events}
externalEventMutations={externalEventMutations}
>
{newChildren}
Expand Down
36 changes: 20 additions & 16 deletions packages/victory-core/src/victory-animation/victory-animation.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import PropTypes from "prop-types";
import * as d3Ease from "d3-ease";
import { victoryInterpolator } from "./util";
import Timer from "../victory-util/timer";
import isEqual from "react-fast-compare";

export default class VictoryAnimation extends React.Component {
static displayName = "VictoryAnimation";
Expand Down Expand Up @@ -98,24 +99,27 @@ export default class VictoryAnimation extends React.Component {
}
}

/* lifecycle */
componentWillReceiveProps(nextProps) {
/* cancel existing loop if it exists */
this.getTimer().unsubscribe(this.loopID);
shouldComponentUpdate(nextProps, nextState) {
const equalProps = isEqual(this.props, nextProps);
if (!equalProps) {
/* cancel existing loop if it exists */
this.getTimer().unsubscribe(this.loopID);

/* If an object was supplied */
if (!Array.isArray(nextProps.data)) {
// Replace the tween queue. Could set `this.queue = [nextProps.data]`,
// but let's reuse the same array.
this.queue.length = 0;
this.queue.push(nextProps.data);
/* If an array was supplied */
} else {
/* Extend the tween queue */
this.queue.push(...nextProps.data);
/* If an object was supplied */
if (!Array.isArray(nextProps.data)) {
// Replace the tween queue. Could set `this.queue = [nextProps.data]`,
// but let's reuse the same array.
this.queue.length = 0;
this.queue.push(nextProps.data);
/* If an array was supplied */
} else {
/* Extend the tween queue */
this.queue.push(...nextProps.data);
}
/* Start traversing the tween queue */
this.traverseQueue();
}
/* Start traversing the tween queue */
this.traverseQueue();
return nextState.animationInfo.animating || !equalProps;
}

componentWillUnmount() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import Helpers from "../victory-util/helpers";
import Timer from "../victory-util/timer";
import Transitions from "../victory-util/transitions";
import { defaults, isFunction, pick, isObject } from "lodash";
import isEqual from "react-fast-compare";

export default class VictoryTransition extends React.Component {
static displayName = "VictoryTransition";
Expand Down Expand Up @@ -33,11 +34,14 @@ export default class VictoryTransition extends React.Component {
this.setState({ nodesShouldLoad: true }); //eslint-disable-line react/no-did-mount-set-state
}

componentWillReceiveProps(nextProps) {
this.getTimer().bypassAnimation();
this.setState(this.getTransitionState(this.props, nextProps), () =>
this.getTimer().resumeAnimation()
);
shouldComponentUpdate(nextProps) {
if (!isEqual(this.props, nextProps)) {
this.getTimer().bypassAnimation();
this.setState(this.getTransitionState(this.props, nextProps), () =>
this.getTimer().resumeAnimation()
);
}
return true;
}

componentWillUnmount() {
Expand Down
66 changes: 59 additions & 7 deletions packages/victory-core/src/victory-util/add-events.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,14 @@ const datumHasXandY = (datum) => {
return !isNil(datum._x) && !isNil(datum._y);
};

export default (WrappedComponent) => {
// used for checking state changes. Expected components can be passed in via options
const defaultComponents = [
{ name: "parent", index: "parent" },
{ name: "data" },
{ name: "labels" }
];

export default (WrappedComponent, options) => {
return class addEvents extends WrappedComponent {
constructor(props) {
super(props);
Expand All @@ -22,18 +29,63 @@ export default (WrappedComponent) => {
const calculatedValues = this.getCalculatedValues(props);
this.cacheValues(calculatedValues);
this.externalMutations = this.getExternalMutations(props);
this.calculatedState = this.getStateChanges(props, calculatedValues);
}

componentDidUpdate() {
const externalMutations = this.getExternalMutations(this.props);
if (!isEqual(this.externalMutations, externalMutations)) {
shouldComponentUpdate(nextProps) {
const calculatedValues = this.getCalculatedValues(nextProps);
const externalMutations = this.getExternalMutations(nextProps);
const animating = this.props.animating || this.props.animate;
const newMutation = !isEqual(externalMutations, this.externalMutations);
if (animating || newMutation) {
this.cacheValues(calculatedValues);
this.externalMutations = externalMutations;
this.applyExternalMutations(this.props, externalMutations);
this.applyExternalMutations(nextProps, externalMutations);
return true;
}
const calculatedState = this.getStateChanges(nextProps, calculatedValues);
if (!isEqual(this.calculatedState, calculatedState)) {
this.calculatedState = calculatedState;
this.cacheValues(calculatedValues);
return true;
}
if (!isEqual(this.props, nextProps)) {
this.cacheValues(calculatedValues);
return true;
}
return false;
}

componentWillReceiveProps(nextProps) {
this.cacheValues(this.getCalculatedValues(nextProps));
// compile all state changes from own and parent state. Order doesn't matter, as any state
// state change should trigger a re-render
getStateChanges(props, calculatedValues) {
const { hasEvents, getSharedEventState } = calculatedValues;
if (!hasEvents) {
return {};
}

const getState = (key, type) => {
const result = defaults({}, this.getEventState(key, type), getSharedEventState(key, type));
return isEmpty(result) ? undefined : result;
};

options = options || {};
const components = options.components || defaultComponents;
const stateChanges = components
.map((component) => {
if (!props.standalone && component.name === "parent") {
// don't check for changes on parent props for non-standalone components
return undefined;
} else {
return component.index !== undefined
? getState(component.index, component.name)
: calculatedValues.dataKeys
.map((key) => getState(key, component.name))
.filter(Boolean);
}
})
.filter(Boolean);
return stateChanges;
}

applyExternalMutations(props, externalMutations) {
Expand Down
1 change: 1 addition & 0 deletions packages/victory-group/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"dependencies": {
"lodash": "^4.17.5",
"prop-types": "^15.5.8",
"react-fast-compare": "^2.0.0",
"victory-core": "^31.1.0"
},
"scripts": {
Expand Down
17 changes: 10 additions & 7 deletions packages/victory-group/src/victory-group.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React from "react";
import { Helpers, VictoryContainer, VictoryTheme, CommonProps, Wrapper } from "victory-core";
import { VictorySharedEvents } from "victory-shared-events";
import { getChildren, getCalculatedProps } from "./helper-methods";
import isEqual from "react-fast-compare";

const fallbackProps = {
width: 450,
Expand Down Expand Up @@ -65,14 +66,16 @@ export default class VictoryGroup extends React.Component {
}
}

componentWillReceiveProps(nextProps) {
shouldComponentUpdate(nextProps) {
if (this.props.animate) {
this.setAnimationState(this.props, nextProps);
if (!isEqual(this.props, nextProps)) {
this.setAnimationState(this.props, nextProps);
return false;
}
}
this.events = Wrapper.getAllEvents(nextProps);
return true;
}

// the old ones were bad
getNewChildren(props, childComponents, calculatedProps) {
const children = getChildren(props, childComponents, calculatedProps);
const getAnimationProps = Wrapper.getAnimationProps.bind(this);
Expand Down Expand Up @@ -124,13 +127,13 @@ export default class VictoryGroup extends React.Component {
const container = standalone
? this.renderContainer(containerComponent, containerProps)
: groupComponent;
this.events = this.events || Wrapper.getAllEvents(props);
if (!isEmpty(this.events)) {
const events = Wrapper.getAllEvents(props);
if (!isEmpty(events)) {
return (
<VictorySharedEvents
container={container}
eventKey={eventKey}
events={this.events}
events={events}
externalEventMutations={externalEventMutations}
>
{newChildren}
Expand Down
Loading