plotly/dash-renderer

Provide functioning setProps if the component doesn't have an ID

chriddyp opened this issue · 1 comments

(From plotly/dash-core-components#532)

In dash-renderer, we now provide setProps for all components, regardless of whether they are connected to the backend.

However, the setProps that we provide needs an ID in order for it to update the component:

getSetProps() {
return newProps => {
const {
_dashprivate_dependencies,
_dashprivate_dispatch,
_dashprivate_paths
} = this.props;
const id = this.getLayoutProps().id;
// Identify the modified props that are required for callbacks
const watchedKeys = filter(key =>
_dashprivate_dependencies &&
_dashprivate_dependencies.find(dependency =>
dependency.inputs.find(input => input.id === id && input.property === key) ||
dependency.state.find(state => state.id === id && state.property === key)
)
)(keysIn(newProps));
// Always update this component's props
_dashprivate_dispatch(updateProps({
props: newProps,
id: id,
itempath: _dashprivate_paths[id]
}));
// Only dispatch changes to Dash if a watched prop changed
if (watchedKeys.length) {
_dashprivate_dispatch(notifyObservers({
id: id,
props: pick(watchedKeys)(newProps)
}));
}
};
}

We should provide a function setProps no matter what, even if the component doesn't have an ID. Otherwise, component authors will have to write logic like this: https://github.com/plotly/dash-core-components/pull/532/files#diff-2ee8e0d2935bbb7cda96c81810cf07ee

The main complexity is the path store: we cache the path of components with an ID and have nice redux actions for updating the layout based off that path.
There might be two options here:

  1. Keep track of the component path as we recurse through TreeContainer and use that path in our dynamic setProps construction.
  2. Or, for components with no ID, auto-generated one based on the path. It’d effectively be for every component in the tree as we don’t know in advance if a component uses setProps or not. so there might be a memory impact for apps with tons of components. but it’d only be a fraction of the layout store (since we don’t need to save the other props). We already have logic flows for mutating the layout store (and updating paths) when components’ children get updated, so we could potentially mutate the layout store in those flows and inject a private ID prop. There’d be a little mismatch from what the backend provides and what is in the layout store, but might be do able.

A nice side effect of having paths for all components (not just those with IDs) would be to display the path of ID-less components. Here's where we display the ID if it's available.

if (props.id) {
errorMessage += ` with ID "${props.id}"`;
}
errorMessage += ` is required but it was not provided.`;

Fixed by #166