Skip to content

Commit 13fb387

Browse files
dmt0dmt0
authored
Merge pull request #128 from plotly/remove-componentWillUpdate
Remove deprecated react methods; refactor
2 parents 44b2bd5 + 60de2ff commit 13fb387

File tree

2 files changed

+61
-75
lines changed

2 files changed

+61
-75
lines changed

‎src/__tests__/react-plotly.test.js

+23-5
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ describe('<Plotly/>', () => {
4444
});
4545

4646
describe('initialization', function() {
47-
test('calls Plotly.newPlot on instantiation', done => {
47+
test('calls Plotly.react on instantiation', done => {
4848
createPlot({})
4949
.then(() => {
50-
expect(Plotly.newPlot).toHaveBeenCalled();
50+
expect(Plotly.react).toHaveBeenCalled();
5151
})
5252
.catch(err => {
5353
done.fail(err);
@@ -61,7 +61,7 @@ describe('<Plotly/>', () => {
6161
layout: {title: 'foo'},
6262
})
6363
.then(() => {
64-
expectPlotlyAPICall(Plotly.newPlot, {
64+
expectPlotlyAPICall(Plotly.react, {
6565
data: [{x: [1, 2, 3]}],
6666
layout: {title: 'foo'},
6767
});
@@ -75,7 +75,7 @@ describe('<Plotly/>', () => {
7575
layout: {width: 320, height: 240},
7676
})
7777
.then(() => {
78-
expectPlotlyAPICall(Plotly.newPlot, {
78+
expectPlotlyAPICall(Plotly.react, {
7979
layout: {width: 320, height: 240},
8080
});
8181
})
@@ -102,6 +102,24 @@ describe('<Plotly/>', () => {
102102
.catch(err => done.fail(err));
103103
});
104104

105+
test('updates data when revision is defined but not changed', done => {
106+
createPlot({
107+
revision: 1,
108+
layout: {width: 123, height: 456},
109+
onUpdate: once(() => {
110+
expectPlotlyAPICall(Plotly.react, {
111+
data: [{x: [1, 2, 3]}],
112+
layout: {width: 123, height: 456},
113+
});
114+
done();
115+
}),
116+
})
117+
.then(plot => {
118+
plot.setProps({revision: 1, data: [{x: [1, 2, 3]}]});
119+
})
120+
.catch(err => done.fail(err));
121+
});
122+
105123
test('sets the title', done => {
106124
createPlot({
107125
onUpdate: once(() => {
@@ -129,7 +147,7 @@ describe('<Plotly/>', () => {
129147
// we've checked the third and fourth calls:
130148
if (callCnt === 2) {
131149
setTimeout(() => {
132-
expect(Plotly.newPlot).not.toHaveBeenCalledTimes(2);
150+
expect(Plotly.react).not.toHaveBeenCalledTimes(2);
133151
done();
134152
}, 100);
135153
}

‎src/factory.js

+38-70
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,10 @@ export default function plotComponentFactory(Plotly) {
6161
this.getRef = this.getRef.bind(this);
6262
this.handleUpdate = this.handleUpdate.bind(this);
6363
this.figureCallback = this.figureCallback.bind(this);
64+
this.updatePlotly = this.updatePlotly.bind(this);
6465
}
6566

66-
componentDidMount() {
67-
this.unmounting = false;
68-
67+
updatePlotly(shouldInvokeResizeHandler, figureCallbackFunction, shouldAttachUpdateEvents) {
6968
this.p = this.p
7069
.then(() => {
7170
if (!this.el) {
@@ -78,17 +77,17 @@ export default function plotComponentFactory(Plotly) {
7877
}
7978
throw error;
8079
}
81-
return Plotly.newPlot(this.el, {
80+
return Plotly.react(this.el, {
8281
data: this.props.data,
8382
layout: this.props.layout,
8483
config: this.props.config,
8584
frames: this.props.frames,
8685
});
8786
})
88-
.then(() => this.syncWindowResize(null, true))
87+
.then(() => this.syncWindowResize(shouldInvokeResizeHandler))
8988
.then(this.syncEventHandlers)
90-
.then(this.attachUpdateEvents)
91-
.then(() => this.figureCallback(this.props.onInitialized))
89+
.then(() => this.figureCallback(figureCallbackFunction))
90+
.then(shouldAttachUpdateEvents ? this.attachUpdateEvents : () => {})
9291
.catch(err => {
9392
if (err.reason === 'unmounting') {
9493
return;
@@ -100,59 +99,35 @@ export default function plotComponentFactory(Plotly) {
10099
});
101100
}
102101

103-
UNSAFE_componentWillUpdate(nextProps) {
102+
componentDidMount() {
103+
this.unmounting = false;
104+
105+
this.updatePlotly(true, this.props.onInitialized, true);
106+
}
107+
108+
componentDidUpdate(prevProps) {
104109
this.unmounting = false;
105110

106111
// frames *always* changes identity so fall back to check length only :(
107112
const numPrevFrames =
108-
this.props.frames && this.props.frames.length ? this.props.frames.length : 0;
113+
prevProps.frames && prevProps.frames.length ? prevProps.frames.length : 0;
109114
const numNextFrames =
110-
nextProps.frames && nextProps.frames.length ? nextProps.frames.length : 0;
115+
this.props.frames && this.props.frames.length ? this.props.frames.length : 0;
111116

112117
const figureChanged = !(
113-
nextProps.layout === this.props.layout &&
114-
nextProps.data === this.props.data &&
115-
nextProps.config === this.props.config &&
118+
prevProps.layout === this.props.layout &&
119+
prevProps.data === this.props.data &&
120+
prevProps.config === this.props.config &&
116121
numNextFrames === numPrevFrames
117122
);
118-
const revisionDefined = nextProps.revision !== void 0;
119-
const revisionChanged = nextProps.revision !== this.props.revision;
123+
const revisionDefined = prevProps.revision !== void 0;
124+
const revisionChanged = prevProps.revision !== this.props.revision;
120125

121126
if (!figureChanged && (!revisionDefined || (revisionDefined && !revisionChanged))) {
122127
return;
123128
}
124129

125-
this.p = this.p
126-
.then(() => {
127-
if (!this.el) {
128-
let error;
129-
if (this.unmounting) {
130-
error = new Error('Component is unmounting');
131-
error.reason = 'unmounting';
132-
} else {
133-
error = new Error('Missing element reference');
134-
}
135-
throw error;
136-
}
137-
return Plotly.react(this.el, {
138-
data: nextProps.data,
139-
layout: nextProps.layout,
140-
config: nextProps.config,
141-
frames: nextProps.frames,
142-
});
143-
})
144-
.then(() => this.syncEventHandlers(nextProps))
145-
.then(() => this.syncWindowResize(nextProps))
146-
.then(() => this.figureCallback(nextProps.onUpdate))
147-
.catch(err => {
148-
if (err.reason === 'unmounting') {
149-
return;
150-
}
151-
console.error('Error while plotting:', err); // eslint-disable-line no-console
152-
if (this.props.onError) {
153-
this.props.onError(err);
154-
}
155-
});
130+
this.updatePlotly(false, this.props.onUpdate, false);
156131
}
157132

158133
componentWillUnmount() {
@@ -175,19 +150,19 @@ export default function plotComponentFactory(Plotly) {
175150
return;
176151
}
177152

178-
for (let i = 0; i < updateEvents.length; i++) {
179-
this.el.on(updateEvents[i], this.handleUpdate);
180-
}
153+
updateEvents.forEach(updateEvent => {
154+
this.el.on(updateEvent, this.handleUpdate);
155+
});
181156
}
182157

183158
removeUpdateEvents() {
184159
if (!this.el || !this.el.removeListener) {
185160
return;
186161
}
187162

188-
for (let i = 0; i < updateEvents.length; i++) {
189-
this.el.removeListener(updateEvents[i], this.handleUpdate);
190-
}
163+
updateEvents.forEach(updateEvent => {
164+
this.el.removeListener(updateEvent, this.handleUpdate);
165+
});
191166
}
192167

193168
handleUpdate() {
@@ -203,21 +178,18 @@ export default function plotComponentFactory(Plotly) {
203178
}
204179
}
205180

206-
syncWindowResize(propsIn, invoke) {
207-
const props = propsIn || this.props;
181+
syncWindowResize(invoke) {
208182
if (!isBrowser) {
209183
return;
210184
}
211185

212-
if (props.useResizeHandler && !this.resizeHandler) {
213-
this.resizeHandler = () => {
214-
return Plotly.Plots.resize(this.el);
215-
};
186+
if (this.props.useResizeHandler && !this.resizeHandler) {
187+
this.resizeHandler = () => Plotly.Plots.resize(this.el);
216188
window.addEventListener('resize', this.resizeHandler);
217189
if (invoke) {
218190
this.resizeHandler();
219191
}
220-
} else if (!props.useResizeHandler && this.resizeHandler) {
192+
} else if (!this.props.useResizeHandler && this.resizeHandler) {
221193
window.removeEventListener('resize', this.resizeHandler);
222194
this.resizeHandler = null;
223195
}
@@ -232,13 +204,9 @@ export default function plotComponentFactory(Plotly) {
232204
}
233205

234206
// Attach and remove event handlers as they're added or removed from props:
235-
syncEventHandlers(propsIn) {
236-
// Allow use of nextProps if passed explicitly:
237-
const props = propsIn || this.props;
238-
239-
for (let i = 0; i < eventNames.length; i++) {
240-
const eventName = eventNames[i];
241-
const prop = props['on' + eventName];
207+
syncEventHandlers() {
208+
eventNames.forEach(eventName => {
209+
const prop = this.props['on' + eventName];
242210
const hasHandler = Boolean(this.handlers[eventName]);
243211

244212
if (prop && !hasHandler) {
@@ -249,7 +217,7 @@ export default function plotComponentFactory(Plotly) {
249217
this.el.removeListener('plotly_' + eventName.toLowerCase(), this.handlers[eventName]);
250218
delete this.handlers[eventName];
251219
}
252-
}
220+
});
253221
}
254222

255223
render() {
@@ -281,9 +249,9 @@ export default function plotComponentFactory(Plotly) {
281249
divId: PropTypes.string,
282250
};
283251

284-
for (let i = 0; i < eventNames.length; i++) {
285-
PlotlyComponent.propTypes['on' + eventNames[i]] = PropTypes.func;
286-
}
252+
eventNames.forEach(eventName => {
253+
PlotlyComponent.propTypes['on' + eventName] = PropTypes.func;
254+
});
287255

288256
PlotlyComponent.defaultProps = {
289257
debug: false,

0 commit comments

Comments
 (0)