From 17e4886a0f4c2c9590ae71325d3ecf0afa91e590 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Wed, 14 Aug 2019 07:50:05 -0400 Subject: [PATCH 1/4] fix graph event removal on unmount --- src/components/Graph.react.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/Graph.react.js b/src/components/Graph.react.js index b4bce1e4c..e1e4d20b8 100644 --- a/src/components/Graph.react.js +++ b/src/components/Graph.react.js @@ -212,8 +212,9 @@ class PlotlyGraph extends Component { } componentWillUnmount() { - if (this.eventEmitter) { - this.eventEmitter.removeAllListeners(); + const gd = document.getElementById(this.props.id); + if (gd && gd.removeAllListeners) { + gd.removeAllListeners(); } window.removeEventListener('resize', this.graphResize); } From a92ac3544ca64a8032450948eaf5a6bb902fc7a1 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Wed, 14 Aug 2019 11:02:06 -0400 Subject: [PATCH 2/4] stop making random graph ID, and purge correctly on unmount --- src/components/Graph.react.js | 74 ++++++++++----------- tests/integration/graph/test_graph_purge.py | 53 +++++++++++++++ 2 files changed, 89 insertions(+), 38 deletions(-) create mode 100644 tests/integration/graph/test_graph_purge.py diff --git a/src/components/Graph.react.js b/src/components/Graph.react.js index e1e4d20b8..d14a14868 100644 --- a/src/components/Graph.react.js +++ b/src/components/Graph.react.js @@ -64,67 +64,63 @@ const filterEventData = (gd, eventData, event) => { return filteredEventData; }; -function generateId() { - const charAmount = 36; - const length = 7; - return ( - 'graph-' + - Math.random() - .toString(charAmount) - .substring(2, length) - ); -} - /** * Graph can be used to render any plotly.js-powered data visualization. * * You can define callbacks based on user interaction with Graphs such as * hovering, clicking or selecting */ -const GraphWithDefaults = props => { - const id = props.id ? props.id : generateId(); - return ; -}; - class PlotlyGraph extends Component { constructor(props) { super(props); + this.gd = React.createRef(); this.bindEvents = this.bindEvents.bind(this); this._hasPlotted = false; + this._prevGd = null; this.graphResize = this.graphResize.bind(this); } plot(props) { - const {figure, id, animate, animation_options, config} = props; - const gd = document.getElementById(id); + const {figure, animate, animation_options, config} = props; + const gd = this.gd.current; if ( animate && this._hasPlotted && figure.data.length === gd.data.length ) { - return Plotly.animate(id, figure, animation_options); + return Plotly.animate(gd, figure, animation_options); } - return Plotly.react(id, { + return Plotly.react(gd, { data: figure.data, layout: clone(figure.layout), frames: figure.frames, config: config, }).then(() => { + const gd = this.gd.current; + + // double-check gd hasn't been unmounted + if (!gd) { + return; + } + + // in case we've made a new DOM element, transfer events + if(this._hasPlotted && gd !== this._prevGd) { + this._prevGd.removeAllListeners(); + this._hasPlotted = false; + } + if (!this._hasPlotted) { - // double-check gd hasn't been unmounted - const gd = document.getElementById(id); - if (gd) { - this.bindEvents(); - Plotly.Plots.resize(gd); - this._hasPlotted = true; - } + this.bindEvents(); + Plotly.Plots.resize(gd); + this._hasPlotted = true; + this._prevGd = gd; } }); } extend(props) { - const {id, extendData} = props; + const {extendData} = props; let updateData, traceIndices, maxPoints; if (Array.isArray(extendData) && typeof extendData[0] === 'object') { [updateData, traceIndices, maxPoints] = extendData; @@ -143,20 +139,21 @@ class PlotlyGraph extends Component { traceIndices = generateIndices(updateData); } - return Plotly.extendTraces(id, updateData, traceIndices, maxPoints); + const gd = this.gd.current; + return Plotly.extendTraces(gd, updateData, traceIndices, maxPoints); } graphResize() { - const graphDiv = document.getElementById(this.props.id); - if (graphDiv) { - Plotly.Plots.resize(graphDiv); + const gd = this.gd.current; + if (gd) { + Plotly.Plots.resize(gd); } } bindEvents() { - const {id, setProps, clear_on_unhover} = this.props; + const {setProps, clear_on_unhover} = this.props; - const gd = document.getElementById(id); + const gd = this.gd.current; gd.on('plotly_click', eventData => { const clickData = filterEventData(gd, eventData, 'click'); @@ -212,9 +209,10 @@ class PlotlyGraph extends Component { } componentWillUnmount() { - const gd = document.getElementById(this.props.id); + const gd = this.gd.current; if (gd && gd.removeAllListeners) { gd.removeAllListeners(); + Plotly.purge(gd); } window.removeEventListener('resize', this.graphResize); } @@ -263,6 +261,7 @@ class PlotlyGraph extends Component {
Date: Wed, 14 Aug 2019 11:10:11 -0400 Subject: [PATCH 3/4] more complete purge on new DOM element creation --- src/components/Graph.react.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/Graph.react.js b/src/components/Graph.react.js index d14a14868..4d467dd3a 100644 --- a/src/components/Graph.react.js +++ b/src/components/Graph.react.js @@ -106,7 +106,10 @@ class PlotlyGraph extends Component { // in case we've made a new DOM element, transfer events if(this._hasPlotted && gd !== this._prevGd) { - this._prevGd.removeAllListeners(); + if(this._prevGd && this._prevGd.removeAllListeners) { + this._prevGd.removeAllListeners(); + Plotly.purge(this._prevGd); + } this._hasPlotted = false; } From a788b4e5012e1758b6081912ebf421d4ff0c9587 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Wed, 14 Aug 2019 11:14:04 -0400 Subject: [PATCH 4/4] changelog for Graph cleanup #604 --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bcc8bb945..0b78cef33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## UNRELEASED +### Fixed +- Fixed problems with `Graph` components leaking events and being recreated multiple times if declared with no ID [#604](https://github.com/plotly/dash-core-components/pull/604) + ## [1.1.1] - 2019-08-06 - Upgraded plotly.js to 1.49.1 [#595](https://github.com/plotly/dash-core-components/issues/595) - Patch release [1.49.1](https://github.com/plotly/plotly.js/releases/tag/v1.49.1)