Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Renderer regression #140

Merged
merged 10 commits into from
Mar 25, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be removed once in master

git clone git@github.com:plotly/dash-html-components.git
git clone git@github.com:plotly/dash-table.git
. venv/bin/activate
Expand Down
114 changes: 77 additions & 37 deletions src/TreeContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import PropTypes from 'prop-types';
import Registry from './registry';
import {connect} from 'react-redux';
import {
any,
contains,
filter,
forEach,
Expand All @@ -18,7 +19,6 @@ import {
propOr,
type
} from 'ramda';
import {STATUS} from './constants/constants';
import { notifyObservers, updateProps } from './actions';

const SIMPLE_COMPONENT_TYPES = ['String', 'Number', 'Null', 'Boolean'];
Expand Down Expand Up @@ -75,38 +75,6 @@ class TreeContainer extends Component {
);
}

getLoadingState(id, requestQueue) {
// loading prop coming from TreeContainer
let isLoading = false;
let loadingProp;
let loadingComponent;

if (requestQueue && requestQueue.filter) {
forEach(r => {
const controllerId = isNil(r.controllerId) ? '' : r.controllerId;
if (r.status === 'loading' && contains(id, controllerId)) {
isLoading = true;
[loadingComponent, loadingProp] = r.controllerId.split('.');
}
}, requestQueue);

const thisRequest = requestQueue.filter(r => {
const controllerId = isNil(r.controllerId) ? '' : r.controllerId;
return contains(id, controllerId);
});
if (thisRequest.status === STATUS.OK) {
isLoading = false;
}
}

// Set loading state
return {
is_loading: isLoading,
prop_name: loadingProp,
component_name: loadingComponent,
};
}

getSetProps() {
return newProps => {
const {
Expand Down Expand Up @@ -145,7 +113,10 @@ class TreeContainer extends Component {
}

shouldComponentUpdate(nextProps) {
return nextProps._dashprivate_layout !== this.props._dashprivate_layout;
const { _dashprivate_layout, _dashprivate_loadingState } = nextProps;

return _dashprivate_layout !== this.props._dashprivate_layout ||
_dashprivate_loadingState.is_loading !== this.props._dashprivate_loadingState.is_loading;
}

getLayoutProps() {
Expand All @@ -162,17 +133,20 @@ class TreeContainer extends Component {
const layoutProps = this.getLayoutProps();

const children = this.getChildren(layoutProps.children);
const loadingState = this.getLoadingState(layoutProps.id, _dashprivate_requestQueue);
const setProps = this.getSetProps(_dashprivate_dispatch);

return this.getComponent(_dashprivate_layout, children, loadingState, setProps);
const loadingState = getLoadingState(_dashprivate_layout, _dashprivate_requestQueue);

return this.getComponent(
_dashprivate_layout, children, loadingState, setProps);
}
}

TreeContainer.propTypes = {
_dashprivate_dependencies: PropTypes.any,
_dashprivate_dispatch: PropTypes.func,
_dashprivate_layout: PropTypes.object,
_dashprivate_loadingState: PropTypes.object,
_dashprivate_paths: PropTypes.any,
_dashprivate_requestQueue: PropTypes.object,
};
Expand All @@ -194,12 +168,78 @@ function mergeProps(stateProps, dispatchProps, ownProps) {
_dashprivate_dependencies: stateProps.dependencies,
_dashprivate_dispatch: dispatchProps.dispatch,
_dashprivate_layout: ownProps._dashprivate_layout,
_dashprivate_loading: ownProps._dashprivate_loading,
_dashprivate_loadingState: getLoadingState(ownProps._dashprivate_layout, stateProps.requestQueue),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calculating the loadingState upfront instead of in the render function -- this allows to block/allow re-renderer based on loading state in shouldComponentUpdate

_dashprivate_paths: stateProps.paths,
_dashprivate_requestQueue: stateProps.requestQueue,
};
}

function getLoadingState(layout, requestQueue) {
const ids = isDeepStateElement(layout) ?
getNestedIds() :
(layout && layout.props.id ?
[layout.props.id] :
[]);

let isLoading = false;
let loadingProp;
let loadingComponent;

if (requestQueue) {
forEach(r => {
const controllerId = isNil(r.controllerId) ? '' : r.controllerId;
if (r.status === 'loading' && any(id => contains(id, controllerId), ids)) {
isLoading = true;
[loadingComponent, loadingProp] = r.controllerId.split('.');
}
}, requestQueue);
}

// Set loading state
return {
is_loading: isLoading,
prop_name: loadingProp,
component_name: loadingComponent,
};
}

function getNestedIds(layout) {
const ids = [];
const queue = [layout];

while (queue.length) {
const elementLayout = queue.shift();

const props = elementLayout &&
elementLayout.props;

if (!props) {
continue;
}

const { children, id } = props;

if (id) {
ids.push(id);
}

if (children) {
const filteredChildren = filter(
child => !isSimpleComponent(child) && !Registry.resolve(child.type, child.namespace)._dashprivate_deepstate,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isDeepStateElement

Array.isArray(children) ? children : [children]
);

queue.push(...filteredChildren);
}
}

return ids;
}

function isDeepStateElement(layout) {
return Registry.resolve(layout.type, layout.namespace)._dashprivate_deepstate;
}

export const AugmentedTreeContainer = connect(mapStateToProps, mapDispatchToProps, mergeProps)(TreeContainer);

export default AugmentedTreeContainer;