From b95779e9b89c24cf3e7464e76622806a7ca1725e Mon Sep 17 00:00:00 2001 From: Isaac Ezer Date: Wed, 19 Feb 2020 16:34:29 -0500 Subject: [PATCH] Remove deprecated Lifecycle hooks (#234) * remove unsafe calls from animated examples * remove unsafe calls from TreeMap * remove unsafe calls from KernelDensityEstimation * remove unsafe calls from LineChart * remove Unsafe calls from SankeyDiagram * remove deprecated lifecycle hooks from ZoomContainer --- .../PieChart/examples/PieChart.js.example | 2 +- .../examples/AnimatedTreeMap.js.example | 30 +++++--- src/KernelDensityEstimation.js | 14 ++-- src/LineChart.js | 27 +++---- src/SankeyDiagram.js | 73 ++++++++++--------- src/TreeMap.js | 57 ++++++++------- src/ZoomContainer.js | 51 ++++++------- tests/jsdom/spec/TreeMap.spec.js | 50 ++++++------- 8 files changed, 160 insertions(+), 144 deletions(-) diff --git a/docs/src/docs/PieChart/examples/PieChart.js.example b/docs/src/docs/PieChart/examples/PieChart.js.example index f8b6ed08..1f3cd9a2 100644 --- a/docs/src/docs/PieChart/examples/PieChart.js.example +++ b/docs/src/docs/PieChart/examples/PieChart.js.example @@ -11,7 +11,7 @@ class PieChartExample extends React.Component { this.setState({ sinVal }); }; - UNSAFE_componentWillMount() { + componentDidMount() { this._interval = setInterval(this._animateValue, 20); } componentWillUnmount() { diff --git a/docs/src/docs/TreeMap/examples/AnimatedTreeMap.js.example b/docs/src/docs/TreeMap/examples/AnimatedTreeMap.js.example index 1e2e54c1..a6b3512e 100644 --- a/docs/src/docs/TreeMap/examples/AnimatedTreeMap.js.example +++ b/docs/src/docs/TreeMap/examples/AnimatedTreeMap.js.example @@ -1,5 +1,18 @@ class AnimatedTreeMapExample extends React.Component { - state = {getValue: "size"}; + constructor(props) { + super(props); + + const data = { + children: _.range(1, 5).map(n => ({ + children: _.times(n * n, m => ({ + size: (n +1) * (m + 1) + (100 * Math.random()), + size2: (n +1) * (m + 1) + (100 * Math.random()) + })) + })) + }; + + this.state = { getValue: "size", data }; + } _animateValue = () => { if(this.state.getValue === "size") @@ -8,23 +21,16 @@ class AnimatedTreeMapExample extends React.Component { this.setState({getValue: "size"}); }; - UNSAFE_componentWillMount() { + componentDidMount() { this._interval = setInterval(this._animateValue, 1000); - this._data = { - children: _.range(1, 5).map(n => ({ - children: _.times(n * n, m => ({ - size: (n +1) * (m + 1) + (100 * Math.random()), - size2: (n +1) * (m + 1) + (100 * Math.random()) - })) - })) - }; } + componentWillUnmount() { clearInterval(this._interval); } render() { - const {getValue} = this.state; + const {getValue, data} = this.state; const colorScale = d3.scaleLinear() .domain([0, 65]) @@ -33,7 +39,7 @@ class AnimatedTreeMapExample extends React.Component { return
({ diff --git a/src/KernelDensityEstimation.js b/src/KernelDensityEstimation.js index be7588e4..20b3205d 100644 --- a/src/KernelDensityEstimation.js +++ b/src/KernelDensityEstimation.js @@ -71,18 +71,16 @@ class KernelDensityEstimation extends React.Component { return shouldUpdate; } - UNSAFE_componentWillMount() { - this.initKDE(this.props); + static getDerivedStateFromProps(nextProps, prevState) { + const kdeData = KernelDensityEstimation.getKdeData(nextProps); + return { kdeData }; } - UNSAFE_componentWillReceiveProps(newProps) { - this.initKDE(newProps); - } - initKDE(props) { + + static getKdeData(props) { const { data, bandwidth, sampleCount, xScale, width } = props; const kernel = epanechnikovKernel(bandwidth); const samples = xScale.ticks(sampleCount || Math.ceil(width / 2)); - - this.setState({ kdeData: kernelDensityEstimator(kernel, samples)(data) }); + return kernelDensityEstimator(kernel, samples)(data); } render() { diff --git a/src/LineChart.js b/src/LineChart.js index f70251d4..b8b06f51 100644 --- a/src/LineChart.js +++ b/src/LineChart.js @@ -50,24 +50,25 @@ export default class LineChart extends React.Component { curve: curveLinear, }; - shouldComponentUpdate(nextProps) { - return !xyPropsEqual(this.props, nextProps, ['lineStyle', 'lineClassName']); + static getBisectorState(props) { + const bisectX = bisector(d => getValue(props.x, d)).left; + return { bisectX }; } - /* eslint-disable-next-line camelcase */ - UNSAFE_componentWillMount() { - this.initBisector(this.props); - } + static getDerivedStateFromProps(nextProps) { + if (nextProps.x) { + return LineChart.getBisectorState(nextProps); + } - /* eslint-disable-next-line camelcase */ - UNSAFE_componentWillReceiveProps(nextProps) { - this.initBisector(nextProps); + return null; } - initBisector(props) { - this.setState({ - bisectX: bisector(d => getValue(props.x, d)).left, - }); + state = { + bisectX: null, + }; + + shouldComponentUpdate(nextProps) { + return !xyPropsEqual(this.props, nextProps, ['lineStyle', 'lineClassName']); } getHovered = x => { diff --git a/src/SankeyDiagram.js b/src/SankeyDiagram.js index 90944921..936f145e 100644 --- a/src/SankeyDiagram.js +++ b/src/SankeyDiagram.js @@ -951,38 +951,9 @@ export default class SankeyDiagram extends React.Component { linkTargetLabelStartOffset: '98%', }; - _makeSankeyGraph() { - const innerWidth = - this.props.width - (this.props.marginLeft + this.props.marginRight); - const innerHeight = - this.props.height - (this.props.marginTop + this.props.marginBottom); - const makeSankey = sankey() - .size([innerWidth, innerHeight]) - .nodeId(this.props.nodeId) - .nodeWidth(this.props.nodeWidth) - .nodePadding(this.props.nodePadding) - .nodeAlign( - nodeAlignmentsByName[this.props.nodeAlignment] || - nodeAlignmentsByName.justify, - ); + static getDerivedStateFromProps(nextProps, prevState) { + const { prevProps } = prevState; - const nodes = this.props.shouldClone - ? cloneDeep(this.props.nodes) - : this.props.nodes; - const links = this.props.shouldClone - ? cloneDeep(this.props.links) - : this.props.links; - const sankeyGraph = makeSankey({ nodes, links }); - this._graph = enhanceGraph(sankeyGraph); - } - - /* eslint-disable-next-line camelcase */ - UNSAFE_componentWillMount() { - this._makeSankeyGraph(); - } - - /* eslint-disable-next-line camelcase */ - UNSAFE_componentWillReceiveProps(nextProps) { // only update this._graph if a prop which affects the sankey layout has changed (most don't) const sankeyLayoutPropKeys = [ 'nodes', @@ -1000,9 +971,43 @@ export default class SankeyDiagram extends React.Component { ]; const hasChangedSankey = sankeyLayoutPropKeys.some(key => { - return nextProps[key] !== this.props[key]; + return nextProps[key] !== prevProps[key]; }); - if (hasChangedSankey) this._makeSankeyGraph(); + if (hasChangedSankey) { + const graph = SankeyDiagram.makeSankeyGraph(nextProps); + return { + graph, + prevProps: cloneDeep(nextProps), + }; + } + + return null; + } + + static makeSankeyGraph(props) { + const innerWidth = props.width - (props.marginLeft + props.marginRight); + const innerHeight = props.height - (props.marginTop + props.marginBottom); + const makeSankey = sankey() + .size([innerWidth, innerHeight]) + .nodeId(props.nodeId) + .nodeWidth(props.nodeWidth) + .nodePadding(props.nodePadding) + .nodeAlign( + nodeAlignmentsByName[props.nodeAlignment] || + nodeAlignmentsByName.justify, + ); + + const nodes = props.shouldClone ? cloneDeep(props.nodes) : props.nodes; + const links = props.shouldClone ? cloneDeep(props.links) : props.links; + const sankeyGraph = makeSankey({ nodes, links }); + return enhanceGraph(sankeyGraph); + } + + constructor(props) { + super(props); + const graph = SankeyDiagram.makeSankeyGraph(props); + const prevProps = cloneDeep(props); + this.state = { graph, prevProps }; } render() { @@ -1018,7 +1023,7 @@ export default class SankeyDiagram extends React.Component { marginRight, } = this.props; - const graph = this._graph; + const { graph } = this.state; const makeLinkPath = sankeyLinkHorizontal(); const className = `rct-sankey-diagram ${this.props.className}`; const innerWidth = width - (marginLeft + marginRight); diff --git a/src/TreeMap.js b/src/TreeMap.js index 812bb2f1..ae949ff2 100644 --- a/src/TreeMap.js +++ b/src/TreeMap.js @@ -107,18 +107,27 @@ class TreeMap extends React.Component { NodeLabelComponent: TreeMapNodeLabel, }; - /* eslint-disable-next-line camelcase */ - UNSAFE_componentWillMount() { - const { data } = this.props; - // initialize the layout function - this._tree = getTree(this.props); - // clone the data because d3 mutates it! - this._rootNode = getRootNode(cloneDeep(data), this.props); + static initTreemap(rootNode, tree, options) { + // create a d3 treemap layout function, + // and configure it with the given options + const { getValue, sort } = options; + const treeRoot = rootNode.sum(d => { + if (isFunction(getValue)) return getValue(d); + else if (isString(getValue)) return d[getValue]; + return 0; + }); + return tree(sort ? treeRoot.sort(sort) : treeRoot).descendants(); } - /* eslint-disable-next-line camelcase */ - UNSAFE_componentWillReceiveProps(newProps) { - const { width, height, data, sticky } = this.props; + static getStateFromProps(props) { + const tree = getTree(props); + const rootNode = getRootNode(cloneDeep(props.data), props); + const prevProps = cloneDeep(props); + return { tree, rootNode, prevProps }; + } + + static getDerivedStateFromProps(newProps, state) { + const { width, height, data, sticky } = state.prevProps; // if height, width, or the data changes, or if the treemap is not sticky, re-initialize the layout function // todo reevaluate this logic @@ -128,10 +137,18 @@ class TreeMap extends React.Component { height !== newProps.height || JSON.stringify(data) !== JSON.stringify(newProps.data) ) { - this._tree = getTree(newProps); - this._rootNode = getRootNode(cloneDeep(newProps.data), this.props); + return TreeMap.getStateFromProps(newProps); } + + return null; + } + + constructor(props) { + super(props); + + this.state = TreeMap.getStateFromProps(props); } + render() { const { width, @@ -149,7 +166,9 @@ class TreeMap extends React.Component { NodeLabelComponent, } = this.props; - const nodes = initTreemap(this._rootNode, this._tree, this.props); + const { rootNode, tree } = this.state; + + const nodes = TreeMap.initTreemap(rootNode, tree, this.props); const style = { position: 'relative', width, height }; @@ -199,16 +218,4 @@ function getTree(options) { return tree; } -function initTreemap(rootNode, tree, options) { - // create a d3 treemap layout function, - // and configure it with the given options - const { getValue, sort } = options; - const treeRoot = rootNode.sum(d => { - if (isFunction(getValue)) return getValue(d); - else if (isString(getValue)) return d[getValue]; - return 0; - }); - return tree(sort ? treeRoot.sort(sort) : treeRoot).descendants(); -} - export default TreeMap; diff --git a/src/ZoomContainer.js b/src/ZoomContainer.js index 7c78f1ea..d84532e9 100644 --- a/src/ZoomContainer.js +++ b/src/ZoomContainer.js @@ -146,6 +146,32 @@ export default class ZoomContainer extends React.Component { }); } + componentDidUpdate(prevProps) { + const nextProps = this.props; + if (prevProps.controlled) { + // if controlled component and zoom props have changed, apply the new zoom props to d3-zoom + // (unbind handler first so as not to create infinite callback loop) + const hasChangedZoom = + nextProps.zoomX !== prevProps.zoomX || + nextProps.zoomY !== prevProps.zoomY || + nextProps.zoomScale !== prevProps.zoomScale; + + if (hasChangedZoom) { + this.zoom.on('zoom', null); + const nextZoomTransform = zoomTransformFromProps(nextProps); + this.zoom.transform(this.state.selection, nextZoomTransform); + this.zoom.on('zoom', this.handleZoom); + + // update state.lastZoomTransform so we can revert d3-zoom to this next time it's changed internally + // eslint-disable-next-line react/no-did-update-set-state + this.setState({ + lastZoomTransform: nextZoomTransform, + }); + } + } + this._updateZoomProps(nextProps); + } + handleZoom = (...args) => { const nextZoomTransform = d3.event.transform; @@ -167,31 +193,6 @@ export default class ZoomContainer extends React.Component { if (this.props.onZoom) this.props.onZoom(nextZoomTransform, ...args); }; - /* eslint-disable-next-line camelcase */ - UNSAFE_componentWillReceiveProps(nextProps) { - if (this.props.controlled) { - // if controlled component and zoom props have changed, apply the new zoom props to d3-zoom - // (unbind handler first so as not to create infinite callback loop) - const hasChangedZoom = - nextProps.zoomX !== this.props.zoomX || - nextProps.zoomY !== this.props.zoomY || - nextProps.zoomScale !== this.props.zoomScale; - - if (hasChangedZoom) { - this.zoom.on('zoom', null); - const nextZoomTransform = zoomTransformFromProps(nextProps); - this.zoom.transform(this.state.selection, nextZoomTransform); - this.zoom.on('zoom', this.handleZoom); - - // update state.lastZoomTransform so we can revert d3-zoom to this next time it's changed internally - this.setState({ - lastZoomTransform: nextZoomTransform, - }); - } - } - this._updateZoomProps(nextProps); - } - _updateZoomProps(props) { let propsToUse = props; diff --git a/tests/jsdom/spec/TreeMap.spec.js b/tests/jsdom/spec/TreeMap.spec.js index 1da79f8e..327eee5b 100644 --- a/tests/jsdom/spec/TreeMap.spec.js +++ b/tests/jsdom/spec/TreeMap.spec.js @@ -1,25 +1,24 @@ -import React from "react"; -import * as d3 from "d3"; -import _ from "lodash"; -import { expect } from "chai"; -import sinon from "sinon"; -import { mount } from "enzyme"; +import React from 'react'; +import _ from 'lodash'; +import { expect } from 'chai'; +import sinon from 'sinon'; +import { mount } from 'enzyme'; -import { TreeMap } from "../../../src/index.js"; -import TreeMapNode from "../../../src/TreeMapNode.js"; -import TreeMapNodeLabel from "../../../src/TreeMapNodeLabel.js"; +import { TreeMap } from '../../../src/index.js'; +import TreeMapNode from '../../../src/TreeMapNode.js'; +import TreeMapNodeLabel from '../../../src/TreeMapNodeLabel.js'; -describe("TreeMap", () => { +describe('TreeMap', () => { const data = { children: _.range(1, 5).map(n => ({ size: n * 5, - name: `Name: ${n}` - })) + name: `Name: ${n}`, + })), }; const props = { data, - nodeStyle: { border: "1px solid #333" }, + nodeStyle: { border: '1px solid #333' }, getLabel: d => d.name, getValue: d => d.size, onClickNode: sinon.spy(), @@ -27,10 +26,10 @@ describe("TreeMap", () => { onMouseLeaveNode: sinon.spy(), onMouseMoveNode: sinon.spy(), width: 400, - height: 500 + height: 500, }; - it("passes props correctly to nodes", () => { + it('passes props correctly to nodes', () => { const chart = mount(); const nodes = chart.find(TreeMapNode); @@ -39,10 +38,9 @@ describe("TreeMap", () => { }); }); - it("renders correct amount of nodes and labels", () => { + it('renders correct amount of nodes and labels', () => { const chart = mount(); const nodes = chart.find(TreeMapNode); - const labels = chart.find(TreeMapNodeLabel); expect(nodes).to.have.lengthOf(5); @@ -51,31 +49,31 @@ describe("TreeMap", () => { }); }); - it("recreates tree when sticky is false, and keeps tree when true", () => { + it('recreates tree when sticky is false, and keeps tree when true', () => { let chart = mount(); - let tree = chart.instance()._tree; + let tree = chart.instance().state.tree; chart.setProps({ data }); - expect(tree).to.not.equal(chart.instance()._tree); + expect(tree).to.not.equal(chart.instance().state.tree); chart = mount(); - tree = chart.instance()._tree; + tree = chart.instance().state.tree; chart.setProps({ data }); - expect(tree).to.eql(chart.instance()._tree); + expect(tree).to.eql(chart.instance().state.tree); }); - it("triggers event handlers", () => { + it('triggers event handlers', () => { const chart = mount(); const nodes = chart.find(TreeMapNode); expect(props.onMouseMoveNode).not.to.have.been.called; - nodes.at(1).simulate("mousemove"); + nodes.at(1).simulate('mousemove'); expect(props.onMouseMoveNode).to.have.been.called; expect(props.onMouseEnterNode).not.to.have.been.called; - nodes.at(1).simulate("mouseenter"); + nodes.at(1).simulate('mouseenter'); expect(props.onMouseEnterNode).to.have.been.called; expect(props.onMouseLeaveNode).not.to.have.been.called; - nodes.at(1).simulate("mouseleave"); + nodes.at(1).simulate('mouseleave'); expect(props.onMouseLeaveNode).to.have.been.called; }); });