Skip to content
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

1193 multipage validation #1201

Merged
merged 6 commits into from
Apr 22, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@
All notable changes to `dash` will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).

## [UNRELEASED]
### Added
- [#1201](https://github.com/plotly/dash/pull/1201) New attribute `app.validation_layout` allows you to create a multi-page app without `suppress_callback_exceptions=True` or layout function tricks. Set this to a component layout containing the superset of all IDs on all pages in your app.

### Fixed
- [#1201](https://github.com/plotly/dash/pull/1201) Fixes [#1193](https://github.com/plotly/dash/issues/1193) - prior to Dash 1.11, you could use `flask.has_request_context() == False` inside an `app.layout` function to provide a special layout containing all IDs for validation purposes in a multi-page app. Dash 1.11 broke this when we moved most of this validation into the renderer. This change makes it work again.

## [1.11.0] - 2020-04-10
### Added
- [#1103](https://github.com/plotly/dash/pull/1103) Pattern-matching IDs and callbacks. Component IDs can be dictionaries, and callbacks can reference patterns of components, using three different wildcards: `ALL`, `MATCH`, and `ALLSMALLER`, available from `dash.dependencies`. This lets you create components on demand, and have callbacks respond to any and all of them. To help with this, `dash.callback_context` gets three new entries: `outputs_list`, `inputs_list`, and `states_list`, which contain all the ids, properties, and except for the outputs, the property values from all matched components.
Expand Down
14 changes: 11 additions & 3 deletions dash-renderer/src/actions/dependencies.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {

const mergeMax = mergeWith(Math.max);

import {getPath} from './paths';
import {computePaths, getPath} from './paths';

import {crawlLayout} from './utils';

Expand Down Expand Up @@ -464,9 +464,17 @@ function wildcardOverlap({id, property}, objs) {
}

export function validateCallbacksToLayout(state_, dispatchError) {
const {config, graphs, layout, paths} = state_;
const {outputMap, inputMap, outputPatterns, inputPatterns} = graphs;
const {config, graphs, layout: layout_, paths: paths_} = state_;
const validateIds = !config.suppress_callback_exceptions;
let layout, paths;
if (validateIds && config.validation_layout) {
layout = config.validation_layout;
Copy link
Member

Choose a reason for hiding this comment

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

Is this because validation_layout is pruned down, so faster to crawl?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is just saying if we have a separate validation layout, that's the one we need to check the callbacks against. The fact that it's pruned should make computePaths (next line) faster, but the main reason for pruning was to reduce the size of _dash-config on the index page - I was imagining it could become enormous, if you have multiple tables and graphs on different pages all concatenated together.

paths = computePaths(layout, [], null, paths_.events);
} else {
layout = layout_;
paths = paths_;
}
const {outputMap, inputMap, outputPatterns, inputPatterns} = graphs;

function tail(callbacks) {
return (
Expand Down
45 changes: 37 additions & 8 deletions dash/dash.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ def __init__(
self.routes = []

self._layout = None
self._cached_layout = None
self._layout_is_function = False
self.validation_layout = None

self._setup_dev_tools()
self._hot_reload = AttributeDict(
Expand Down Expand Up @@ -421,18 +422,43 @@ def layout(self):
return self._layout

def _layout_value(self):
if isinstance(self._layout, patch_collections_abc("Callable")):
self._cached_layout = self._layout()
else:
self._cached_layout = self._layout
return self._cached_layout
return self._layout() if self._layout_is_function else self._layout

@layout.setter
def layout(self, value):
_validate.validate_layout_type(value)
self._cached_layout = None
self._layout_is_function = isinstance(value, patch_collections_abc("Callable"))
self._layout = value

# for using flask.has_request_context() to deliver a full layout for
# validation inside a layout function - track if a user might be doing this.
if self._layout_is_function and not self.validation_layout:
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this is something we should turn off if debug=False too, or introduce a new dev_tools_validate_layout function. Just to reduce the performance hit if there are many small components (e.g. a dash_html_components.Table)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. Those mostly wouldn't have IDs, so wouldn't be sent to the front end anyway, but with debug off we don't need it at all. In fact I guess we should be able to short-circuit all the front-end validation in that case too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added clauses to skip this when suppress_callback_validation is used, but didn't remove it entirely when debug=False. There's more to think about here - on the one hand, without this validation the app behavior can change, and on the other there's a lot more validation we could be skipping if that's what we want to do. I've created #1206 to follow up.


def simple_clone(c, children=None):
cls = type(c)
# in Py3 we can use the __init__ signature to reduce to just
# required args and id; in Py2 this doesn't work so we just
# empty out children.
sig = getattr(cls.__init__, "__signature__", None)
props = {
p: getattr(c, p)
for p in c._prop_names # pylint: disable=protected-access
if hasattr(c, p)
and (
p == "id" or not sig or sig.parameters[p].default == c.REQUIRED
)
}
if props.get("children", children):
props["children"] = children or []
return cls(**props)

layout_value = self._layout_value()
_validate.validate_layout(value, layout_value)
self.validation_layout = simple_clone(
# pylint: disable=protected-access
layout_value, [simple_clone(c) for c in layout_value._traverse_ids()]
Copy link
Collaborator Author

@alexcjohnson alexcjohnson Apr 17, 2020

Choose a reason for hiding this comment

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

Make this validation layout as simple as possible, to minimize its overhead on the page:

  • strip out components with no ID
  • strip out most prop values, at least in Py3 where we can fully inspect the component constructor signature so we know what props are required.

This step is only run once when you set the layout, and even then only when you set it to a function AND you haven't already set app.validation_layout

Copy link
Member

Choose a reason for hiding this comment

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

If we're just using this to validate IDs w.r.t. callbacks, why do we need the required values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We also validate props against the component type, so we can't just create dummy components, they have to be the same type as the real components. And unless I were to recreate the JSON serialization for components, the easiest way to do that is to actually instantiate the same component class. But doing so requires that we include required props, because the generated components enforce the required props during construction.

Copy link
Member

Choose a reason for hiding this comment

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

Ohhhhhh by "props against the component type" do you mean during callback validation with messages like e.g. "You registered a callback to update a Div's childrenz property. However, Div does not have a "children" property."?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

e.g. "You registered a callback to update a Div's childrenz property. However, Div does not have a "childrenz" property."?

exactly.

)

@property
def index_string(self):
return self._index_string
Expand Down Expand Up @@ -468,6 +494,9 @@ def _config(self):
"interval": int(self._dev_tools.hot_reload_interval * 1000),
"max_retry": self._dev_tools.hot_reload_max_retry,
}
if self.validation_layout:
config["validation_layout"] = self.validation_layout

return config

def serve_reload_hash(self):
Expand Down Expand Up @@ -602,7 +631,7 @@ def _generate_scripts_html(self):

def _generate_config_html(self):
return '<script id="_dash-config" type="application/json">{}</script>'.format(
json.dumps(self._config())
json.dumps(self._config(), cls=plotly.utils.PlotlyJSONEncoder)
)

def _generate_renderer(self):
Expand Down
11 changes: 8 additions & 3 deletions dash/development/base_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,11 +293,16 @@ def _traverse_with_paths(self):
for p, t in i._traverse_with_paths():
yield "\n".join([list_path, p]), t

def __iter__(self):
"""Yield IDs in the tree of children."""
def _traverse_ids(self):
"""Yield components with IDs in the tree of children."""
for t in self._traverse():
if isinstance(t, Component) and getattr(t, "id", None) is not None:
yield t.id
yield t

def __iter__(self):
"""Yield IDs in the tree of children."""
for t in self._traverse_ids():
yield t.id

def __len__(self):
"""Return the number of items in the tree."""
Expand Down
124 changes: 124 additions & 0 deletions tests/integration/devtools/test_callback_validation.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import flask
import pytest

import dash_core_components as dcc
import dash_html_components as html
from dash import Dash
Expand Down Expand Up @@ -695,3 +698,124 @@ def c3(children):
]
]
check_errors(dash_duo, specs)


def multipage_app(validation=False):
app = Dash(__name__, suppress_callback_exceptions=(validation == "suppress"))

skeleton = html.Div(
[dcc.Location(id="url", refresh=False), html.Div(id="page-content")]
)

layout_index = html.Div(
[
dcc.Link('Navigate to "/page-1"', id="index_p1", href="/page-1"),
dcc.Link('Navigate to "/page-2"', id="index_p2", href="/page-2"),
]
)

layout_page_1 = html.Div(
[
html.H2("Page 1"),
dcc.Input(id="input-1-state", type="text", value="Montreal"),
dcc.Input(id="input-2-state", type="text", value="Canada"),
html.Button(id="submit-button", n_clicks=0, children="Submit"),
html.Div(id="output-state"),
html.Br(),
dcc.Link('Navigate to "/"', id="p1_index", href="/"),
dcc.Link('Navigate to "/page-2"', id="p1_p2", href="/page-2"),
]
)

layout_page_2 = html.Div(
[
html.H2("Page 2"),
dcc.Input(id="page-2-input", value="LA"),
html.Div(id="page-2-display-value"),
html.Br(),
dcc.Link('Navigate to "/"', id="p2_index", href="/"),
dcc.Link('Navigate to "/page-1"', id="p2_p1", href="/page-1"),
]
)

validation_layout = html.Div([skeleton, layout_index, layout_page_1, layout_page_2])

def validation_function():
return skeleton if flask.has_request_context() else validation_layout

app.layout = validation_function if validation == "function" else skeleton
if validation == "attribute":
app.validation_layout = validation_layout

# Index callbacks
@app.callback(Output("page-content", "children"), [Input("url", "pathname")])
def display_page(pathname):
if pathname == "/page-1":
return layout_page_1
elif pathname == "/page-2":
return layout_page_2
else:
return layout_index

# Page 1 callbacks
@app.callback(
Output("output-state", "children"),
[Input("submit-button", "n_clicks")],
[State("input-1-state", "value"), State("input-2-state", "value")],
)
def update_output(n_clicks, input1, input2):
return (
"The Button has been pressed {} times,"
'Input 1 is "{}",'
'and Input 2 is "{}"'
).format(n_clicks, input1, input2)

# Page 2 callbacks
@app.callback(
Output("page-2-display-value", "children"), [Input("page-2-input", "value")]
)
def display_value(value):
print("display_value")
return 'You have selected "{}"'.format(value)

return app


def test_dvcv014_multipage_errors(dash_duo):
app = multipage_app()
dash_duo.start_server(app, **debugging)

specs = [
[
"ID not found in layout",
['"page-2-input"', "page-2-display-value.children"],
],
["ID not found in layout", ['"submit-button"', "output-state.children"]],
[
"ID not found in layout",
['"page-2-display-value"', "page-2-display-value.children"],
],
["ID not found in layout", ['"output-state"', "output-state.children"]],
]
check_errors(dash_duo, specs)


@pytest.mark.parametrize("validation", ("function", "attribute", "suppress"))
def test_dvcv015_multipage_validation_layout(validation, dash_duo):
app = multipage_app(validation)
dash_duo.start_server(app, **debugging)

dash_duo.wait_for_text_to_equal("#index_p1", 'Navigate to "/page-1"')
dash_duo.find_element("#index_p1").click()

dash_duo.find_element("#submit-button").click()
dash_duo.wait_for_text_to_equal(
"#output-state",
"The Button has been pressed 1 times,"
'Input 1 is "Montreal",and Input 2 is "Canada"',
)

dash_duo.find_element("#p1_p2").click()
dash_duo.wait_for_text_to_equal("#page-2-display-value", 'You have selected "LA"')

assert not dash_duo.get_logs()