From a6dc976d5512c30b9425d5bf7b1583c25aa80c2b Mon Sep 17 00:00:00 2001 From: byron Date: Thu, 22 Aug 2019 20:44:52 -0400 Subject: [PATCH 01/10] fix infinite loop --- src/components/Graph.react.js | 43 +++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/components/Graph.react.js b/src/components/Graph.react.js index 4d467dd3a..20b20bc8f 100644 --- a/src/components/Graph.react.js +++ b/src/components/Graph.react.js @@ -1,6 +1,6 @@ import React, {Component} from 'react'; import PropTypes from 'prop-types'; -import {contains, filter, clone, has, isNil, type, omit} from 'ramda'; +import {contains, filter, clone, has, isNil, type, omit, equals} from 'ramda'; /* global Plotly:true */ const filterEventData = (gd, eventData, event) => { @@ -105,8 +105,8 @@ class PlotlyGraph extends Component { } // in case we've made a new DOM element, transfer events - if(this._hasPlotted && gd !== this._prevGd) { - if(this._prevGd && this._prevGd.removeAllListeners) { + if (this._hasPlotted && gd !== this._prevGd) { + if (this._prevGd && this._prevGd.removeAllListeners) { this._prevGd.removeAllListeners(); Plotly.purge(this._prevGd); } @@ -154,7 +154,14 @@ class PlotlyGraph extends Component { } bindEvents() { - const {setProps, clear_on_unhover} = this.props; + const { + setProps, + clear_on_unhover, + relayoutData, + restyleData, + hoverData, + selectedData, + } = this.props; const gd = this.gd.current; @@ -172,30 +179,30 @@ class PlotlyGraph extends Component { setProps({clickAnnotationData}); }); gd.on('plotly_hover', eventData => { - const hoverData = filterEventData(gd, eventData, 'hover'); - if (!isNil(hoverData)) { - setProps({hoverData}); + const hover = filterEventData(gd, eventData, 'hover'); + if (!isNil(hover) && !equals(hover, hoverData)) { + setProps({hoverData: hover}); } }); gd.on('plotly_selected', eventData => { - const selectedData = filterEventData(gd, eventData, 'selected'); - if (!isNil(selectedData)) { - setProps({selectedData}); + const selected = filterEventData(gd, eventData, 'selected'); + if (!isNil(selected) && !equals(selected, selectedData)) { + setProps({selectedData: selected}); } }); gd.on('plotly_deselect', () => { setProps({selectedData: null}); }); gd.on('plotly_relayout', eventData => { - const relayoutData = filterEventData(gd, eventData, 'relayout'); - if (!isNil(relayoutData)) { - setProps({relayoutData}); + const r = filterEventData(gd, eventData, 'relayout'); + if (!isNil(r) && !equals(r, relayoutData)) { + setProps({relayoutData: r}); } }); gd.on('plotly_restyle', eventData => { - const restyleData = filterEventData(gd, eventData, 'restyle'); - if (!isNil(restyleData)) { - setProps({restyleData}); + const restyle = filterEventData(gd, eventData, 'restyle'); + if (!isNil(restyle) && !equals(restyle, restyleData)) { + setProps({restyleData: restyle}); } }); gd.on('plotly_unhover', () => { @@ -237,9 +244,7 @@ class PlotlyGraph extends Component { return; } - const figureChanged = this.props.figure !== nextProps.figure; - - if (figureChanged) { + if (!equals(this.props.figure, nextProps.figure)) { this.plot(nextProps); } From e811094efc11dfbdc3e6e7c7ae3e09f34b8b8e6a Mon Sep 17 00:00:00 2001 From: byron Date: Mon, 26 Aug 2019 13:44:07 -0400 Subject: [PATCH 02/10] tmp save --- digest.json | 7 +++++++ src/components/Graph.react.js | 22 ++++++++++++++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 digest.json diff --git a/digest.json b/digest.json new file mode 100644 index 000000000..38bc80408 --- /dev/null +++ b/digest.json @@ -0,0 +1,7 @@ +{ + "MD5 (dash_core_components.min.js)":"f7dcf7f8f0f3594c7223b428e19ce4bc", + "MD5 (dash_core_components.min.js.map)":"5d2e1b1b01e1cb0812568176a34f76c5", + "MD5 (highlight.pack.js)":"417f2e7cd423e20f392ac0b0c823878b", + "MD5 (plotly-1.49.3.min.js)":"b40953b4a122ab7f35263469fe78875d", + "dash-core-components":"1.1.1" +} \ No newline at end of file diff --git a/src/components/Graph.react.js b/src/components/Graph.react.js index ca7bc1ac8..13b0a3fe3 100644 --- a/src/components/Graph.react.js +++ b/src/components/Graph.react.js @@ -83,7 +83,7 @@ class PlotlyGraph extends Component { plot(props) { const {figure, animate, animation_options, config} = props; const gd = this.gd.current; - + console.log('plot func called', props); if ( animate && this._hasPlotted && @@ -123,6 +123,7 @@ class PlotlyGraph extends Component { } extend(props) { + console.log('extend called', props); const {extendData} = props; let updateData, traceIndices, maxPoints; if (Array.isArray(extendData) && typeof extendData[0] === 'object') { @@ -147,6 +148,7 @@ class PlotlyGraph extends Component { } graphResize() { + console.log('graph resize', gd); const gd = this.gd.current; if (gd) { Plotly.Plots.resize(gd); @@ -213,6 +215,7 @@ class PlotlyGraph extends Component { } componentDidMount() { + console.log('did mount'); this.plot(this.props).then(() => { window.addEventListener('resize', this.graphResize); }); @@ -241,15 +244,24 @@ class PlotlyGraph extends Component { * then the dom needs to get re-rendered with a new ID. * the graph will get updated in componentDidUpdate */ + console.log('will receive props => id changed'); return; } - + console.log('will receive props => id '); if (!equals(this.props.figure, nextProps.figure)) { + console.log( + 'plot as figure diff', + this.props.figure, + nextProps.figure + ); this.plot(nextProps); } - const extendDataChanged = - this.props.extendData !== nextProps.extendData; + const extendDataChanged = !equals( + this.props.extendData, + nextProps.extendData + ); + // this.props.extendData !== nextProps.extendData; if (extendDataChanged) { this.extend(nextProps); @@ -257,7 +269,9 @@ class PlotlyGraph extends Component { } componentDidUpdate(prevProps) { + console.log('did update', prevProps, this.props); if (prevProps.id !== this.props.id) { + console.log('id diff plot'); this.plot(this.props); } } From a952eb973d23be8e88febc6a0d5946a54f3ee4f5 Mon Sep 17 00:00:00 2001 From: byron Date: Tue, 27 Aug 2019 17:26:00 -0400 Subject: [PATCH 03/10] :bug: fix infinite loop with relayout event --- src/components/Graph.react.js | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/components/Graph.react.js b/src/components/Graph.react.js index 13b0a3fe3..f504da2da 100644 --- a/src/components/Graph.react.js +++ b/src/components/Graph.react.js @@ -83,7 +83,6 @@ class PlotlyGraph extends Component { plot(props) { const {figure, animate, animation_options, config} = props; const gd = this.gd.current; - console.log('plot func called', props); if ( animate && this._hasPlotted && @@ -98,14 +97,13 @@ class PlotlyGraph extends Component { 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) { + if (this._hasPlotted && !equals(gd, this._prevGd)) { if (this._prevGd && this._prevGd.removeAllListeners) { this._prevGd.removeAllListeners(); Plotly.purge(this._prevGd); @@ -123,7 +121,6 @@ class PlotlyGraph extends Component { } extend(props) { - console.log('extend called', props); const {extendData} = props; let updateData, traceIndices, maxPoints; if (Array.isArray(extendData) && typeof extendData[0] === 'object') { @@ -148,7 +145,6 @@ class PlotlyGraph extends Component { } graphResize() { - console.log('graph resize', gd); const gd = this.gd.current; if (gd) { Plotly.Plots.resize(gd); @@ -196,9 +192,13 @@ class PlotlyGraph extends Component { setProps({selectedData: null}); }); gd.on('plotly_relayout', eventData => { - const r = filterEventData(gd, eventData, 'relayout'); - if (!isNil(r) && !equals(r, relayoutData)) { - setProps({relayoutData: r}); + const relayout = filterEventData(gd, eventData, 'relayout'); + if ( + !isNil(relayout) && + !equals(relayout, relayoutData) && + !equals(relayout, {autosize: true}) + ) { + setProps({relayoutData: relayout}); } }); gd.on('plotly_restyle', eventData => { @@ -215,7 +215,6 @@ class PlotlyGraph extends Component { } componentDidMount() { - console.log('did mount'); this.plot(this.props).then(() => { window.addEventListener('resize', this.graphResize); }); @@ -244,16 +243,9 @@ class PlotlyGraph extends Component { * then the dom needs to get re-rendered with a new ID. * the graph will get updated in componentDidUpdate */ - console.log('will receive props => id changed'); return; } - console.log('will receive props => id '); if (!equals(this.props.figure, nextProps.figure)) { - console.log( - 'plot as figure diff', - this.props.figure, - nextProps.figure - ); this.plot(nextProps); } @@ -269,9 +261,7 @@ class PlotlyGraph extends Component { } componentDidUpdate(prevProps) { - console.log('did update', prevProps, this.props); if (prevProps.id !== this.props.id) { - console.log('id diff plot'); this.plot(this.props); } } From 522586d1120273c0760bdb130ec0bf28f0357395 Mon Sep 17 00:00:00 2001 From: byron Date: Tue, 27 Aug 2019 17:27:49 -0400 Subject: [PATCH 04/10] :hocho: not in dev --- digest.json | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 digest.json diff --git a/digest.json b/digest.json deleted file mode 100644 index 38bc80408..000000000 --- a/digest.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "MD5 (dash_core_components.min.js)":"f7dcf7f8f0f3594c7223b428e19ce4bc", - "MD5 (dash_core_components.min.js.map)":"5d2e1b1b01e1cb0812568176a34f76c5", - "MD5 (highlight.pack.js)":"417f2e7cd423e20f392ac0b0c823878b", - "MD5 (plotly-1.49.3.min.js)":"b40953b4a122ab7f35263469fe78875d", - "dash-core-components":"1.1.1" -} \ No newline at end of file From 06eca2b63fc09a72707d5ca0ba4e292a4d3a3ebf Mon Sep 17 00:00:00 2001 From: byron Date: Tue, 27 Aug 2019 17:32:19 -0400 Subject: [PATCH 05/10] remove one comment --- src/components/Graph.react.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/Graph.react.js b/src/components/Graph.react.js index f504da2da..fc86654ac 100644 --- a/src/components/Graph.react.js +++ b/src/components/Graph.react.js @@ -253,7 +253,6 @@ class PlotlyGraph extends Component { this.props.extendData, nextProps.extendData ); - // this.props.extendData !== nextProps.extendData; if (extendDataChanged) { this.extend(nextProps); From b02076d6f002400bd9f4d64a40c3a91b28f9493f Mon Sep 17 00:00:00 2001 From: byron Date: Tue, 27 Aug 2019 23:15:47 -0400 Subject: [PATCH 06/10] :ok_hand: fix the first two dangerous equals --- src/components/Graph.react.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Graph.react.js b/src/components/Graph.react.js index fc86654ac..cc5bfa384 100644 --- a/src/components/Graph.react.js +++ b/src/components/Graph.react.js @@ -103,7 +103,7 @@ class PlotlyGraph extends Component { } // in case we've made a new DOM element, transfer events - if (this._hasPlotted && !equals(gd, this._prevGd)) { + if (this._hasPlotted && gd !== this._prevGd) { if (this._prevGd && this._prevGd.removeAllListeners) { this._prevGd.removeAllListeners(); Plotly.purge(this._prevGd); @@ -245,7 +245,7 @@ class PlotlyGraph extends Component { */ return; } - if (!equals(this.props.figure, nextProps.figure)) { + if (this.props.figure !== nextProps.figure) { this.plot(nextProps); } From 224b32c0eb46ec2cf6a53b1f5e178945ae7caa89 Mon Sep 17 00:00:00 2001 From: byron Date: Wed, 28 Aug 2019 13:28:36 -0400 Subject: [PATCH 07/10] :ok_hand: --- src/components/Graph.react.js | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/components/Graph.react.js b/src/components/Graph.react.js index cc5bfa384..c97cb8267 100644 --- a/src/components/Graph.react.js +++ b/src/components/Graph.react.js @@ -193,11 +193,7 @@ class PlotlyGraph extends Component { }); gd.on('plotly_relayout', eventData => { const relayout = filterEventData(gd, eventData, 'relayout'); - if ( - !isNil(relayout) && - !equals(relayout, relayoutData) && - !equals(relayout, {autosize: true}) - ) { + if (!isNil(relayout) && !equals(relayout, relayoutData)) { setProps({relayoutData: relayout}); } }); @@ -249,12 +245,7 @@ class PlotlyGraph extends Component { this.plot(nextProps); } - const extendDataChanged = !equals( - this.props.extendData, - nextProps.extendData - ); - - if (extendDataChanged) { + if (this.props.extendData !== nextProps.extendData) { this.extend(nextProps); } } From 7699ea2e3edba31b3838a97f5db2ce63ae475154 Mon Sep 17 00:00:00 2001 From: byron Date: Wed, 28 Aug 2019 14:31:19 -0400 Subject: [PATCH 08/10] add test case to check the infinite loop --- tests/integration/graph/test_graph_basics.py | 60 ++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/integration/graph/test_graph_basics.py b/tests/integration/graph/test_graph_basics.py index 960e22ee3..59c395957 100644 --- a/tests/integration/graph/test_graph_basics.py +++ b/tests/integration/graph/test_graph_basics.py @@ -1,6 +1,10 @@ +import pandas as pd +import numpy as np import dash import dash_html_components as html import dash_core_components as dcc +from dash.dependencies import Input, Output +import dash.testing.wait as wait def test_grbs001_graph_without_ids(dash_duo): @@ -20,3 +24,59 @@ def test_grbs001_graph_without_ids(dash_duo): assert not dash_duo.wait_for_element(".graph-no-id-2").get_attribute( "id" ), "the graph should contain no more auto-generated id" + + +@pytest.mark.DCC608 +def test_grbs002_wrapped_graph_has_no_infinite_loop(dash_duo): + + df = pd.DataFrame(np.random.randn(50, 50)) + figure = { + "data": [ + {"x": df.columns, "y": df.index, "z": df.values, "type": "heatmap"} + ], + "layout": {"xaxis": {"scaleanchor": "y"}}, + } + + app = dash.Dash(__name__) + app.layout = html.Div( + style={ + "backgroundColor": "red", + "height": "100vmin", + "width": "100vmin", + "overflow": "hidden", + "position": "relative", + }, + children=[ + dcc.Loading( + children=[ + dcc.Graph( + id="graph", + figure=figure, + style={ + "position": "absolute", + "top": 0, + "left": 0, + "backgroundColor": "blue", + "width": "100%", + "height": "100%", + "overflow": "hidden", + }, + ) + ] + ) + ], + ) + + @app.callback(Output("graph", "figure"), [Input("graph", "relayoutData")]) + def selected_df_figure(selection): + figure["data"][0]["x"] = df.columns + figure["data"][0]["y"] = df.index + figure["data"][0]["z"] = df.values + return figure + + dash_duo.start_server(app) + + wait.until(lambda: dash_duo.driver.title == "Dash", timeout=2) + assert ( + len({dash_duo.driver.title for _ in range(20)}) == 1 + ), "after the first update, there should contain no extra Updating..." From e769071a6aaf89b6adecf1ca1659019bf817dd63 Mon Sep 17 00:00:00 2001 From: byron Date: Wed, 28 Aug 2019 14:35:10 -0400 Subject: [PATCH 09/10] :lipstick: add import --- tests/integration/graph/test_graph_basics.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/graph/test_graph_basics.py b/tests/integration/graph/test_graph_basics.py index 59c395957..304d6ef99 100644 --- a/tests/integration/graph/test_graph_basics.py +++ b/tests/integration/graph/test_graph_basics.py @@ -1,3 +1,4 @@ +import pytest import pandas as pd import numpy as np import dash From 0a1a39e870f1c140a386166b45c91aec2595fba0 Mon Sep 17 00:00:00 2001 From: byron Date: Wed, 28 Aug 2019 14:39:43 -0400 Subject: [PATCH 10/10] :pencil2: changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fd2628a3..3b3cae4b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased +### Fixed + +- Fixed an infinite loop problem when `Graph` is wrapped by `Loading` component [#608](https://github.com/plotly/dash-core-components/issues/608) + ## [1.1.2] - 2019-08-27 ### 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)