Merge pull request #1416 from plotly/callback-graph-robustness

Callback graph robustness

Author: alexcjohnson

@plotly/dash-test-components/src/components/WidthComponent.js

@@ -0,0 +1,17 @@
+import PropTypes from 'prop-types';
+import React, { Fragment } from 'react';
+
+const WidthComponent = props => (<Fragment>
+    {props.width}
+</Fragment>);
+
+WidthComponent.propTypes = {
+    id: PropTypes.string,
+    width: PropTypes.number
+};
+
+WidthComponent.defaultProps = {
+    width: 0
+};
+
+export default WidthComponent;

@plotly/dash-test-components/src/index.js

@@ -2,10 +2,10 @@ import AsyncComponent from './components/AsyncComponent';
 import CollapseComponent from './components/CollapseComponent';
 import DelayedEventComponent from './components/DelayedEventComponent';
 import FragmentComponent from './components/FragmentComponent';
-import StyledComponent from './components/StyledComponent';
 import MyPersistedComponent from './components/MyPersistedComponent';
 import MyPersistedComponentNested from './components/MyPersistedComponentNested';
-
+import StyledComponent from './components/StyledComponent';
+import WidthComponent from './components/WidthComponent';
 
 export {
     AsyncComponent,
@@ -14,5 +14,6 @@ export {
     FragmentComponent,
     MyPersistedComponent,
     MyPersistedComponentNested,
-    StyledComponent
+    StyledComponent,
+    WidthComponent
 };

CHANGELOG.md

@@ -5,6 +5,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 ## Unreleased
 ### Fixed
 - [#1415](https://github.com/plotly/dash/pull/1415) Fix a regression with some layouts callbacks involving dcc.Tabs, not yet loaded dash_table.DataTable and dcc.Graph to not be called
+- [#1416](https://github.com/plotly/dash/pull/1416) Make callback graph more robust for complex apps and some specific props (`width` in particular) that previously caused errors.
 
 ## [1.16.1] - 2020-09-16
 ### Changed

dash-renderer/package-lock.json

@@ -29,6 +29,7 @@
     "cookie": "^0.4.0",
     "cytoscape": "^3.14.1",
     "cytoscape-dagre": "^2.2.2",
+    "cytoscape-fcose": "^1.2.3",
     "dependency-graph": "^0.9.0",
     "fast-isnumeric": "^1.1.3",
     "prop-types": "15.7.2",

dash-renderer/package.json

@@ -3,9 +3,10 @@ import PropTypes from 'prop-types';
 import {connect, useSelector} from 'react-redux';
 import Cytoscape from 'cytoscape';
 import CytoscapeComponent from 'react-cytoscapejs';
-import Dagre from 'cytoscape-dagre';
+import dagre from 'cytoscape-dagre';
+import fcose from 'cytoscape-fcose';
 import JSONTree from 'react-json-tree';
-import {keys, mergeRight, omit, path} from 'ramda';
+import {keys, mergeRight, omit, path, values} from 'ramda';
 
 import {getPath} from '../../../actions/paths';
 import {stringifyId} from '../../../actions/dependencies';
@@ -19,46 +20,51 @@ import {
     updateCallback
 } from './CallbackGraphEffects';
 
-Cytoscape.use(Dagre);
+Cytoscape.use(dagre);
+Cytoscape.use(fcose);
 
 /*
  * Generates all the elements (nodes, edeges) for the dependency graph.
  */
-function generateElements(graphs, profile) {
+function generateElements(graphs, profile, extraLinks) {
     const consumed = [];
     const elements = [];
+    const structure = {};
 
     function recordNode(id, property) {
         const idStr = stringifyId(id);
         const idType = typeof id === 'object' ? 'wildcard' : 'component';
 
-        const parent = idStr;
-        const child = `${idStr}.${property}`;
+        // dagre layout has problems with eg `width` property - so prepend an X
+        const parentId = idStr;
+        const childId = `${parentId}.X${property}`;
 
-        if (!consumed.includes(parent)) {
-            consumed.push(parent);
+        if (!consumed.includes(parentId)) {
+            consumed.push(parentId);
             elements.push({
                 data: {
-                    id: idStr,
+                    id: parentId,
                     label: idStr,
                     type: idType
                 }
             });
+            structure[parentId] = [];
         }
 
-        if (!consumed.includes(child)) {
-            consumed.push(child);
+        if (!consumed.includes(childId)) {
+            consumed.push(childId);
             elements.push({
                 data: {
-                    id: child,
+                    id: childId,
                     label: property,
-                    parent: parent,
+                    parent: parentId,
                     type: 'property'
                 }
             });
+            structure[parentId].push(childId);
         }
 
-        return child;
+        return childId;
     }
 
     function recordEdge(source, target, type) {
@@ -91,21 +97,34 @@ function generateElements(graphs, profile) {
         });
 
         callback.outputs.map(({id, property}) => {
-            const node = recordNode(id, property);
-            recordEdge(cb, node, 'output');
+            const nodeId = recordNode(id, property);
+            recordEdge(cb, nodeId, 'output');
         });
 
         callback.inputs.map(({id, property}) => {
-            const node = recordNode(id, property);
-            recordEdge(node, cb, 'input');
+            const nodeId = recordNode(id, property);
+            recordEdge(nodeId, cb, 'input');
         });
 
         callback.state.map(({id, property}) => {
-            const node = recordNode(id, property);
-            recordEdge(node, cb, 'state');
+            const nodeId = recordNode(id, property);
+            recordEdge(nodeId, cb, 'state');
         });
     });
 
+    // pull together props in the same component
+    if (extraLinks) {
+        values(structure).forEach(childIds => {
+            childIds.forEach(childFrom => {
+                childIds.forEach(childTo => {
+                    if (childFrom !== childTo) {
+                        recordEdge(childFrom, childTo, 'hidden');
+                    }
+                });
+            });
+        });
+    }
+
     return elements;
 }
 
@@ -141,24 +160,19 @@ function flattenInputs(inArray, final) {
 // len('__dash_callback__.')
 const cbPrefixLen = 18;
 
+const dagreLayout = {
+    name: 'dagre',
+    padding: 10,
+    ranker: 'tight-tree'
+};
+
+const forceLayout = {name: 'fcose', padding: 10, animate: false};
+
 const layouts = {
-    'top-down': {
-        name: 'dagre',
-        padding: 10,
-        spacingFactor: 0.8
-    },
-    'left-right': {
-        name: 'dagre',
-        padding: 10,
-        nodeSep: 0,
-        rankSep: 80,
-        rankDir: 'LR'
-    },
-    force: {
-        name: 'cose',
-        padding: 10,
-        animate: false
-    }
+    'top-down': {...dagreLayout, spacingFactor: 0.8},
+    'left-right': {...dagreLayout, nodeSep: 0, rankSep: 80, rankDir: 'LR'},
+    force: forceLayout,
+    'force-loose': forceLayout
 };
 
 function CallbackGraph() {
@@ -180,7 +194,10 @@ function CallbackGraph() {
     const [layoutType, setLayoutType] = useState(chosenType || 'top-down');
 
     // Generate and memoize the elements.
-    const elements = useMemo(() => generateElements(graphs, profile), [graphs]);
+    const elements = useMemo(
+        () => generateElements(graphs, profile, layoutType === 'force'),
+        [graphs, layoutType]
+    );
 
     // Custom hook to make sure cytoscape is loaded.
     const useCytoscapeEffect = (effect, condition) => {

dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainer.react.js

@@ -24,6 +24,13 @@ const stylesheet = [
         }
     },
 
+    {
+        selector: 'edge[type="hidden"]',
+        style: {
+            display: 'none'
+        }
+    },
+
     {
         selector: 'edge[type="output"]',
         style: {

dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainerStylesheet.js

@@ -3,8 +3,10 @@
 import dash_core_components as dcc
 import dash_html_components as html
 import dash
+from dash.dependencies import Input, Output
 import dash.testing.wait as wait
 
+from dash_test_components import WidthComponent
 from ...assets.todo_app import todo_app
 
 
@@ -102,7 +104,7 @@ def test_dvui003_callback_graph(dash_duo):
 
     pos = dash_duo.driver.execute_script(
         """
-        const pos = store.getState().profile.graphLayout.positions['new-item.value'];
+        const pos = store.getState().profile.graphLayout.positions['new-item.Xvalue'];
         pos.y -= 100;
         return pos.y;
         """
@@ -119,7 +121,36 @@ def test_dvui003_callback_graph(dash_duo):
     # the manually moved node is still in its new position
     assert pos == dash_duo.driver.execute_script(
         """
-        const pos = store.getState().profile.graphLayout.positions['new-item.value'];
+        const pos = store.getState().profile.graphLayout.positions['new-item.Xvalue'];
         return pos.y;
         """
     )
+
+
+def test_dvui004_width_props(dash_duo):
+    app = dash.Dash(__name__)
+
+    app.layout = html.Div(
+        [html.Button(["Click me!"], id="btn"), WidthComponent(id="width")]
+    )
+
+    @app.callback(Output("width", "width"), Input("btn", "n_clicks"))
+    def get_width(n_clicks):
+        n_clicks = n_clicks if n_clicks is not None else 0
+
+        return (n_clicks + 1) * 10
+
+    dash_duo.start_server(
+        app,
+        debug=True,
+        use_reloader=False,
+        use_debugger=True,
+        dev_tools_hot_reload=False,
+    )
+
+    dash_duo.find_element(".dash-debug-menu").click()
+    sleep(1)  # wait for debug menu opening animation
+    dash_duo.find_element(".dash-debug-menu__button--callbacks").click()
+    sleep(3)  # wait for callback graph to draw
+
+    assert dash_duo.get_logs() == []