Renderer regression #140
Renderer regression #140
Conversation
| @@ -39,7 +39,7 @@ jobs: | |||
| name: Install dependencies (dash) | |||
| command: | | |||
| git clone git@github.com:plotly/dash.git | |||
| git clone git@github.com:plotly/dash-core-components.git | |||
| git clone -b renderer-regression git@github.com:plotly/dash-core-components.git | |||
There was a problem hiding this comment.
To be removed once in master
src/TreeContainer.js
Outdated
| this.getNestedIds() : | ||
| (layout && layout.props.id ? | ||
| [layout.props.id] : | ||
| []); |
There was a problem hiding this comment.
If component has deepstate flag, get all nested ids, otherwise use own id
src/TreeContainer.js
Outdated
| @@ -107,6 +109,43 @@ class TreeContainer extends Component { | |||
| }; | |||
| } | |||
|
|
|||
| getNestedIds() { | |||
| const { | |||
There was a problem hiding this comment.
Traverse all children for those with an id. Prune other components with deepstate flag set to true
src/TreeContainer.js
Outdated
|
|
||
| queue.push(...filteredChildren); | ||
| } | ||
| } |
There was a problem hiding this comment.
Loop in-place on children and append the newly found children until all elements have been tested
src/TreeContainer.js
Outdated
| const { _dashprivate_layout, _dashprivate_requestQueue } = nextProps; | ||
|
|
||
| return _dashprivate_layout !== this.props._dashprivate_layout || | ||
| (this.isDeepStateElement() && _dashprivate_requestQueue !== this.props._dashprivate_requestQueue); |
There was a problem hiding this comment.
non-deep components only update on layout changes,
deep components update on layout changes and request queue changes
| @@ -243,11 +168,78 @@ function mergeProps(stateProps, dispatchProps, ownProps) { | |||
| _dashprivate_dependencies: stateProps.dependencies, | |||
| _dashprivate_dispatch: dispatchProps.dispatch, | |||
| _dashprivate_layout: ownProps._dashprivate_layout, | |||
| _dashprivate_loadingState: getLoadingState(ownProps._dashprivate_layout, stateProps.requestQueue), | |||
There was a problem hiding this comment.
Calculating the loadingState upfront instead of in the render function -- this allows to block/allow re-renderer based on loading state in shouldComponentUpdate
src/TreeContainer.js
Outdated
|
|
||
| if (children) { | ||
| const filteredChildren = filter( | ||
| child => !isSimpleComponent(child) && !Registry.resolve(child.type, child.namespace)._dashprivate_deepstate, |
Just bad naming on my part, I think the idea evolved as I was trying out things and didn't revisit / improve the name afterwards. Renaming to |
To be reviewed alongside plotly/dash-core-components#496.
Related CI runs for all dash-core projects running against the
regressionbranches of dcc and renderer:https://circleci.com/gh/plotly/dash/3447
https://circleci.com/gh/plotly/dash/3446
https://circleci.com/gh/plotly/dash/3435
https://circleci.com/gh/plotly/dash-core-components/3963
https://circleci.com/gh/plotly/dash-core-components/3962
https://circleci.com/gh/plotly/dash-core-components/3961
https://circleci.com/gh/plotly/dash-html-components/889
https://circleci.com/gh/plotly/dash-html-components/888
https://circleci.com/gh/plotly/dash-html-components/887
https://circleci.com/gh/plotly/dash-table/6283
https://circleci.com/gh/plotly/dash-table/6282
https://circleci.com/gh/plotly/dash-table/6281
https://circleci.com/gh/plotly/dash-table/6280
https://circleci.com/gh/plotly/dash-table/6279
https://circleci.com/gh/plotly/dash-table/6278
This fixes the regression in Loading Component caused by the renderer optimization. The LC was using the nested children tree structure to determine if it was loading or not. Since the tree containers do not contain the child anymore but instead generate them at render, the LC is only capable of reacting to direct children having loading_state=true.
To resolve the loading state of the LC, two information are now required (1) the layout -- accessible through the children and (2) the request queue. A naive implementation adding the request queue to all tree containers and re-rendering on both request queue and layout changes triggers an excessive amount of re-renders, losing a significant portion of the performance gains made through the previous rework.
To remedy this, the renderer now checks for a flag defined on the component
_dashprivate_deepstatethat tells it to determine the state of the component by checking the state of all of its children, pruning when reaching another component with _dashprivate_deepstate set to true. This allows the renderer to determine the loading_state of all components without having them figure it out for themselves -- if the component flag was made "official" this would allow for a community custom loading component to be made. There is no performance impact to this solution.