-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Callback graph robustness #1416
Conversation
import PropTypes from 'prop-types'; | ||
import React, { Fragment } from 'react'; | ||
|
||
const WidthComponent = props => (<Fragment> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Callback graph fails against components with width
prop.. simple component with width
dash_duo.find_element(".dash-debug-menu__button--callbacks").click() | ||
sleep(3) # wait for callback graph to draw | ||
|
||
assert dash_duo.get_logs() == [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some test with a callback involving a width
prop. Fails against dash-renderer==1.8.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexcjohnson If you're 💃 with the test, I'm 💃 with the code change 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks! 💃
The callback graph barfs on some apps, for two reasons:
width
the Dagre layout throws an error the way we constructed our graph elements. I just mangled the props in the graph element IDs to fix this.ranker
from the defaultnetwork-simplex
totight-tree
appears to be more robust and gives generally comparable results.In addition, I switched the force layout to
fcose
, which is more powerful than the built-incose
, I gave it stronger coupling between props in a single component to group them closer together, and I added a modeforce-loose
that skips that stronger coupling in case that works better in some circumstances.I didn't add any tests of the new behavior, as I couldn't find a simple and public app that has problems previously.