Skip to content

Issue 1010 - Non-rendered async components using asyncDecorator lock renderer callbacks #1027

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

Merged
merged 14 commits into from
Nov 26, 2019
36 changes: 31 additions & 5 deletions dash-renderer/src/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
slice,
sort,
type,
uniq,
view,
} from 'ramda';
import {createAction} from 'redux-actions';
Expand Down Expand Up @@ -221,11 +222,12 @@ export function notifyObservers(payload) {
return async function(dispatch, getState) {
const {id, props, excludedOutputs} = payload;

const {graphs, isAppReady, requestQueue} = getState();

if (isAppReady !== true) {
await isAppReady;
}
const {
dependenciesRequest,
graphs,
isAppReady,
requestQueue,
} = getState();

const {InputGraph} = graphs;
/*
Expand Down Expand Up @@ -365,6 +367,30 @@ export function notifyObservers(payload) {
}
});

/**
* Determine the id of all components used as input or state in the callbacks
* triggered by the props change.
*
* Wait for all components associated to these ids to be ready before initiating
* the callbacks.
*/
const deps = queuedObservers.map(output =>
dependenciesRequest.content.find(
dependency => dependency.output === output
)
);

const ids = uniq(
flatten(
deps.map(dep => [
dep.inputs.map(input => input.id),
dep.state.map(state => state.id),
])
)
);

await isAppReady(ids);

/*
* record the set of output IDs that will eventually need to be
* updated in a queue. not all of these requests will be fired in this
Expand Down
72 changes: 41 additions & 31 deletions dash-renderer/src/actions/setAppReadyState.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {isReady} from '@plotly/dash-component-plugins';
const isAppReady = layout => {
const queue = [layout];

const res = {};
const components = {};
const ids = {};

/* Would be much simpler if the Registry was aware of what it contained... */
while (queue.length) {
Expand All @@ -18,12 +19,17 @@ const isAppReady = layout => {
continue;
}

const id = elementLayout.props && elementLayout.props.id;
const children = elementLayout.props && elementLayout.props.children;
const namespace = elementLayout.namespace;
const type = elementLayout.type;

res[namespace] = res[namespace] || {};
res[namespace][type] = type;
components[namespace] = components[namespace] || {};
components[namespace][type] = type;

if (id) {
ids[id] = {namespace, type};
}

if (children) {
const filteredChildren = filter(
Expand All @@ -35,43 +41,47 @@ const isAppReady = layout => {
}
}

const promises = [];
Object.entries(res).forEach(([namespace, item]) => {
Object.entries(item).forEach(([type]) => {
const component = Registry.resolve({
namespace,
type,
return targets => {
const promises = [];

if (Array.isArray(targets)) {
targets.forEach(id => {
const target = ids[id];
if (target) {
const component = Registry.resolve(target);

const ready = isReady(component);

if (ready && typeof ready.then === 'function') {
promises.push(ready);
}
}
});
} else {
Object.entries(components).forEach(([namespace, item]) => {
Object.entries(item).forEach(([type]) => {
const component = Registry.resolve({
namespace,
type,
});

const ready = isReady(component);
const ready = isReady(component);

if (ready && typeof ready.then === 'function') {
promises.push(ready);
}
});
});
if (ready && typeof ready.then === 'function') {
promises.push(ready);
}
});
});
}

return promises.length ? Promise.all(promises) : true;
return promises.length ? Promise.all(promises) : true;
};
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change isAppReady to return either the global state of the app or the state for the target ids given.


const setAction = createAction(getAction('SET_APP_READY'));

export default () => async (dispatch, getState) => {
const ready = isAppReady(getState().layout);

if (ready === true) {
/* All async is ready */
dispatch(setAction(true));
} else {
/* Waiting on async */
dispatch(setAction(ready));
await ready;
/**
* All known async is ready.
*
* Callbacks were blocked while waiting, we can safely
* assume that no update to layout happened to invalidate.
*/
dispatch(setAction(true));
}
dispatch(setAction(ready));
};
23 changes: 15 additions & 8 deletions dash-renderer/tests/notifyObservers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ const WAIT = 1000;
describe('notifyObservers', () => {
const thunk = notifyObservers({
id: 'id',
props: {},
props: {
'prop1': true
},
undefined
});

Expand All @@ -16,13 +18,16 @@ describe('notifyObservers', () => {
() => ({
graphs: {
InputGraph: {
hasNode: () => false,
dependenciesOf: () => [],
hasNode: () => true,
dependenciesOf: () => [
'id.prop2'
],
dependantsOf: () => [],
overallOrder: () => 0
}
},
isAppReady: true,
isAppReady: () => Promise.resolve(true),
paths: {},
requestQueue: []
})
).then(() => { done = true; });
Expand All @@ -33,23 +38,25 @@ describe('notifyObservers', () => {

it('waits on app to be ready', async () => {
let resolve;
const isAppReady = new Promise(r => {
const isAppReady = () => new Promise(r => {
resolve = r;
});

let done = false;
thunk(
() => { },
() => ({
graphs: {
InputGraph: {
hasNode: () => false,
dependenciesOf: () => [],
hasNode: () => true,
dependenciesOf: () => [
'id.prop2'
],
dependantsOf: () => [],
overallOrder: () => 0
}
},
isAppReady,
paths: {},
requestQueue: []
})
).then(() => { done = true; });
Expand Down
36 changes: 35 additions & 1 deletion tests/integration/callbacks/test_basic_callback.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

import dash_core_components as dcc
import dash_html_components as html
import dash_table
import dash
from dash.dependencies import Input, Output
from dash.dependencies import Input, Output, State
from dash.exceptions import PreventUpdate


def test_cbsc001_simple_callback(dash_duo):
Expand Down Expand Up @@ -140,3 +142,35 @@ def update_input(value):

dash_duo.percy_snapshot(name="callback-generating-function-2")
assert dash_duo.get_logs() == [], "console is clean"


def test_cbsc003_callback_with_unloaded_async_component(dash_duo):
app = dash.Dash()
app.layout = html.Div(
children=[
dcc.Tabs(
children=[
dcc.Tab(
children=[
html.Button(id="btn", children="Update Input"),
html.Div(id="output", children=["Hello"]),
]
),
dcc.Tab(children=dash_table.DataTable(id="other-table")),
]
)
]
)

@app.callback(Output("output", "children"), [Input("btn", "n_clicks")])
def update_graph(n_clicks):
if n_clicks is None:
raise PreventUpdate

return "Bye"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before the changes in this PR, this callback is never triggered because of the blocking dash-table (uses the asyncDecorator) in the other tab. Now, since the callback only involves btn and output, the table is non-blocking.


dash_duo.start_server(app)

dash_duo.find_element('#btn').click()
assert dash_duo.find_element('#output').text == "Bye"
assert dash_duo.get_logs() == []