diff --git a/@plotly/dash-test-components/src/components/WidthComponent.js b/@plotly/dash-test-components/src/components/WidthComponent.js new file mode 100644 index 0000000000..47f5ed9b84 --- /dev/null +++ b/@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 => ( + {props.width} +); + +WidthComponent.propTypes = { + id: PropTypes.string, + width: PropTypes.number +}; + +WidthComponent.defaultProps = { + width: 0 +}; + +export default WidthComponent; \ No newline at end of file diff --git a/@plotly/dash-test-components/src/index.js b/@plotly/dash-test-components/src/index.js index 249c9d2aec..171c6da17f 100644 --- a/@plotly/dash-test-components/src/index.js +++ b/@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 }; diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fc42ae279..f1f66c5f89 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/dash-renderer/package-lock.json b/dash-renderer/package-lock.json index 553aac570a..1f8246b5a6 100644 --- a/dash-renderer/package-lock.json +++ b/dash-renderer/package-lock.json @@ -6562,6 +6562,14 @@ "integrity": "sha1-tf1UIgqivFq1eqtxQMlAdUUDwac=", "dev": true }, + "cose-base": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/cose-base/-/cose-base-1.0.3.tgz", + "integrity": "sha512-s9whTXInMSgAp/NVXVNuVxVKzGH2qck3aQlVHxDCdAEPgtMKwc4Wq6/QKhgdEdgbLSi9rBTAcPoRa6JpiG4ksg==", + "requires": { + "layout-base": "^1.0.0" + } + }, "cosmiconfig": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/cosmiconfig/-/cosmiconfig-6.0.0.tgz", @@ -6832,6 +6840,14 @@ "dagre": "^0.8.2" } }, + "cytoscape-fcose": { + "version": "1.2.3", + "resolved": "https://registry.npmjs.org/cytoscape-fcose/-/cytoscape-fcose-1.2.3.tgz", + "integrity": "sha512-khXpYM3SST8nZIjG1nItJUxgBE8ke2uKxx+CpDQa+zna9jnmKVdJtECuKRt57sD3JyJVx5wFp+6Fvs+p3supdg==", + "requires": { + "cose-base": "^1.0.0" + } + }, "dagre": { "version": "0.8.5", "resolved": "https://registry.npmjs.org/dagre/-/dagre-0.8.5.tgz", @@ -13002,6 +13018,11 @@ "package-json": "^4.0.0" } }, + "layout-base": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/layout-base/-/layout-base-1.0.2.tgz", + "integrity": "sha512-8h2oVEZNktL4BH2JCOI90iD1yXwL6iNW7KcCKT2QZgQJR2vbqDsldCTPRU9NifTCqHZci57XvQQ15YTu+sTYPg==" + }, "lazy-cache": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/lazy-cache/-/lazy-cache-1.0.4.tgz", diff --git a/dash-renderer/package.json b/dash-renderer/package.json index e61b69fb75..5c60c8d869 100644 --- a/dash-renderer/package.json +++ b/dash-renderer/package.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", diff --git a/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainer.react.js b/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainer.react.js index 02c80f6711..6acb49512c 100644 --- a/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainer.react.js +++ b/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainer.react.js @@ -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) => { diff --git a/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainerStylesheet.js b/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainerStylesheet.js index 60a7305b43..4bdb322dc1 100644 --- a/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainerStylesheet.js +++ b/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainerStylesheet.js @@ -24,6 +24,13 @@ const stylesheet = [ } }, + { + selector: 'edge[type="hidden"]', + style: { + display: 'none' + } + }, + { selector: 'edge[type="output"]', style: { diff --git a/tests/integration/devtools/test_devtools_ui.py b/tests/integration/devtools/test_devtools_ui.py index 78bdfc67b0..bcc7dd18a0 100644 --- a/tests/integration/devtools/test_devtools_ui.py +++ b/tests/integration/devtools/test_devtools_ui.py @@ -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() == []