From 4bedf331e8c1015aecf766fc4a0ad8d8d3d611e9 Mon Sep 17 00:00:00 2001 From: dmt0 Date: Thu, 21 Feb 2019 13:08:18 -0500 Subject: [PATCH 1/2] Remove deprecated react methods; refactor --- src/__tests__/react-plotly.test.js | 10 +-- src/factory.js | 108 ++++++++++------------------- 2 files changed, 43 insertions(+), 75 deletions(-) diff --git a/src/__tests__/react-plotly.test.js b/src/__tests__/react-plotly.test.js index e43056e..104e6f4 100644 --- a/src/__tests__/react-plotly.test.js +++ b/src/__tests__/react-plotly.test.js @@ -44,10 +44,10 @@ describe('', () => { }); describe('initialization', function() { - test('calls Plotly.newPlot on instantiation', done => { + test('calls Plotly.react on instantiation', done => { createPlot({}) .then(() => { - expect(Plotly.newPlot).toHaveBeenCalled(); + expect(Plotly.react).toHaveBeenCalled(); }) .catch(err => { done.fail(err); @@ -61,7 +61,7 @@ describe('', () => { layout: {title: 'foo'}, }) .then(() => { - expectPlotlyAPICall(Plotly.newPlot, { + expectPlotlyAPICall(Plotly.react, { data: [{x: [1, 2, 3]}], layout: {title: 'foo'}, }); @@ -75,7 +75,7 @@ describe('', () => { layout: {width: 320, height: 240}, }) .then(() => { - expectPlotlyAPICall(Plotly.newPlot, { + expectPlotlyAPICall(Plotly.react, { layout: {width: 320, height: 240}, }); }) @@ -129,7 +129,7 @@ describe('', () => { // we've checked the third and fourth calls: if (callCnt === 2) { setTimeout(() => { - expect(Plotly.newPlot).not.toHaveBeenCalledTimes(2); + expect(Plotly.react).not.toHaveBeenCalledTimes(2); done(); }, 100); } diff --git a/src/factory.js b/src/factory.js index 92af98a..ac51840 100644 --- a/src/factory.js +++ b/src/factory.js @@ -61,11 +61,10 @@ export default function plotComponentFactory(Plotly) { this.getRef = this.getRef.bind(this); this.handleUpdate = this.handleUpdate.bind(this); this.figureCallback = this.figureCallback.bind(this); + this.updatePlotly = this.updatePlotly.bind(this); } - componentDidMount() { - this.unmounting = false; - + updatePlotly(shouldInvokeResizeHandler, figureCallbackFunction, shouldAttachUpdateEvents) { this.p = this.p .then(() => { if (!this.el) { @@ -78,17 +77,17 @@ export default function plotComponentFactory(Plotly) { } throw error; } - return Plotly.newPlot(this.el, { + return Plotly.react(this.el, { data: this.props.data, layout: this.props.layout, config: this.props.config, frames: this.props.frames, }); }) - .then(() => this.syncWindowResize(null, true)) + .then(() => this.syncWindowResize(shouldInvokeResizeHandler)) .then(this.syncEventHandlers) - .then(this.attachUpdateEvents) - .then(() => this.figureCallback(this.props.onInitialized)) + .then(() => this.figureCallback(figureCallbackFunction)) + .then(shouldAttachUpdateEvents ? this.attachUpdateEvents : () => {}) .catch(err => { if (err.reason === 'unmounting') { return; @@ -100,59 +99,35 @@ export default function plotComponentFactory(Plotly) { }); } - UNSAFE_componentWillUpdate(nextProps) { + componentDidMount() { + this.unmounting = false; + + this.updatePlotly(true, this.props.onInitialized, true); + } + + componentDidUpdate(prevProps) { this.unmounting = false; // frames *always* changes identity so fall back to check length only :( const numPrevFrames = - this.props.frames && this.props.frames.length ? this.props.frames.length : 0; + prevProps.frames && prevProps.frames.length ? prevProps.frames.length : 0; const numNextFrames = - nextProps.frames && nextProps.frames.length ? nextProps.frames.length : 0; + this.props.frames && this.props.frames.length ? this.props.frames.length : 0; const figureChanged = !( - nextProps.layout === this.props.layout && - nextProps.data === this.props.data && - nextProps.config === this.props.config && + prevProps.layout === this.props.layout && + prevProps.data === this.props.data && + prevProps.config === this.props.config && numNextFrames === numPrevFrames ); - const revisionDefined = nextProps.revision !== void 0; - const revisionChanged = nextProps.revision !== this.props.revision; + const revisionDefined = prevProps.revision !== void 0; + const revisionChanged = prevProps.revision !== this.props.revision; if (!figureChanged && (!revisionDefined || (revisionDefined && !revisionChanged))) { return; } - this.p = this.p - .then(() => { - if (!this.el) { - let error; - if (this.unmounting) { - error = new Error('Component is unmounting'); - error.reason = 'unmounting'; - } else { - error = new Error('Missing element reference'); - } - throw error; - } - return Plotly.react(this.el, { - data: nextProps.data, - layout: nextProps.layout, - config: nextProps.config, - frames: nextProps.frames, - }); - }) - .then(() => this.syncEventHandlers(nextProps)) - .then(() => this.syncWindowResize(nextProps)) - .then(() => this.figureCallback(nextProps.onUpdate)) - .catch(err => { - if (err.reason === 'unmounting') { - return; - } - console.error('Error while plotting:', err); // eslint-disable-line no-console - if (this.props.onError) { - this.props.onError(err); - } - }); + this.updatePlotly(false, this.props.onUpdate, false); } componentWillUnmount() { @@ -175,9 +150,9 @@ export default function plotComponentFactory(Plotly) { return; } - for (let i = 0; i < updateEvents.length; i++) { - this.el.on(updateEvents[i], this.handleUpdate); - } + updateEvents.forEach(updateEvent => { + this.el.on(updateEvent, this.handleUpdate); + }); } removeUpdateEvents() { @@ -185,9 +160,9 @@ export default function plotComponentFactory(Plotly) { return; } - for (let i = 0; i < updateEvents.length; i++) { - this.el.removeListener(updateEvents[i], this.handleUpdate); - } + updateEvents.forEach(updateEvent => { + this.el.removeListener(updateEvent, this.handleUpdate); + }); } handleUpdate() { @@ -203,21 +178,18 @@ export default function plotComponentFactory(Plotly) { } } - syncWindowResize(propsIn, invoke) { - const props = propsIn || this.props; + syncWindowResize(invoke) { if (!isBrowser) { return; } - if (props.useResizeHandler && !this.resizeHandler) { - this.resizeHandler = () => { - return Plotly.Plots.resize(this.el); - }; + if (this.props.useResizeHandler && !this.resizeHandler) { + this.resizeHandler = () => Plotly.Plots.resize(this.el); window.addEventListener('resize', this.resizeHandler); if (invoke) { this.resizeHandler(); } - } else if (!props.useResizeHandler && this.resizeHandler) { + } else if (!this.props.useResizeHandler && this.resizeHandler) { window.removeEventListener('resize', this.resizeHandler); this.resizeHandler = null; } @@ -232,13 +204,9 @@ export default function plotComponentFactory(Plotly) { } // Attach and remove event handlers as they're added or removed from props: - syncEventHandlers(propsIn) { - // Allow use of nextProps if passed explicitly: - const props = propsIn || this.props; - - for (let i = 0; i < eventNames.length; i++) { - const eventName = eventNames[i]; - const prop = props['on' + eventName]; + syncEventHandlers() { + eventNames.forEach(eventName => { + const prop = this.props['on' + eventName]; const hasHandler = Boolean(this.handlers[eventName]); if (prop && !hasHandler) { @@ -249,7 +217,7 @@ export default function plotComponentFactory(Plotly) { this.el.removeListener('plotly_' + eventName.toLowerCase(), this.handlers[eventName]); delete this.handlers[eventName]; } - } + }); } render() { @@ -281,9 +249,9 @@ export default function plotComponentFactory(Plotly) { divId: PropTypes.string, }; - for (let i = 0; i < eventNames.length; i++) { - PlotlyComponent.propTypes['on' + eventNames[i]] = PropTypes.func; - } + eventNames.forEach(eventName => { + PlotlyComponent.propTypes['on' + eventName] = PropTypes.func; + }); PlotlyComponent.defaultProps = { debug: false, From 60de2ff5c195b6e810d3d14eae959a2ba556e336 Mon Sep 17 00:00:00 2001 From: dmt0 Date: Mon, 25 Feb 2019 12:54:14 -0500 Subject: [PATCH 2/2] Add test case: data changed, but revision is not --- src/__tests__/react-plotly.test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/__tests__/react-plotly.test.js b/src/__tests__/react-plotly.test.js index 104e6f4..d1b7646 100644 --- a/src/__tests__/react-plotly.test.js +++ b/src/__tests__/react-plotly.test.js @@ -102,6 +102,24 @@ describe('', () => { .catch(err => done.fail(err)); }); + test('updates data when revision is defined but not changed', done => { + createPlot({ + revision: 1, + layout: {width: 123, height: 456}, + onUpdate: once(() => { + expectPlotlyAPICall(Plotly.react, { + data: [{x: [1, 2, 3]}], + layout: {width: 123, height: 456}, + }); + done(); + }), + }) + .then(plot => { + plot.setProps({revision: 1, data: [{x: [1, 2, 3]}]}); + }) + .catch(err => done.fail(err)); + }); + test('sets the title', done => { createPlot({ onUpdate: once(() => {