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

CHANGELOG.md

@@ -10,6 +10,7 @@ clientside JavaScript callbacks via inline strings.
 
 ### Fixed
 - [#1018](https://github.com/plotly/dash/pull/1006) Fix the `dash.testing` **stop** API with process application runner in Python2. Use `kill()` instead of `communicate()` to avoid hanging.
+- [#1027](https://github.com/plotly/dash/pull/1027) Fix bug with renderer callback lock never resolving with non-rendered async component using the asyncDecorator
 
 ## [1.6.1] - 2019-11-14
 ### Fixed

dash-renderer/src/APIController.react.js

@@ -9,7 +9,6 @@ import {
     computePaths,
     hydrateInitialOutputs,
     setLayout,
-    setAppIsReady,
 } from './actions/index';
 import {applyPersistence} from './persistence';
 import apiThunk from './actions/api';
@@ -55,7 +54,6 @@ class UnconnectedContainer extends Component {
                     dispatch
                 );
                 dispatch(setLayout(finalLayout));
-                dispatch(setAppIsReady());
             } else if (isNil(paths)) {
                 dispatch(computePaths({subTree: layout, startingPath: []}));
             }

dash-renderer/src/actions/constants.js

@@ -8,7 +8,6 @@ const actionList = {
     SET_CONFIG: 'SET_CONFIG',
     ON_ERROR: 'ON_ERROR',
     SET_HOOKS: 'SET_HOOKS',
-    SET_APP_READY: 'SET_APP_READY',
 };
 
 export const getAction = action => {

dash-renderer/src/actions/index.js

@@ -23,6 +23,7 @@ import {
     slice,
     sort,
     type,
+    uniq,
     view,
 } from 'ramda';
 import {createAction} from 'redux-actions';
@@ -33,7 +34,8 @@ import cookie from 'cookie';
 import {uid, urlBase, isMultiOutputProp, parseMultipleOutputs} from '../utils';
 import {STATUS} from '../constants/constants';
 import {applyPersistence, prunePersistence} from '../persistence';
-import setAppIsReady from './setAppReadyState';
+
+import isAppReady from './isAppReady';
 
 export const updateProps = createAction(getAction('ON_PROP_CHANGE'));
 export const setRequestQueue = createAction(getAction('SET_REQUEST_QUEUE'));
@@ -45,8 +47,6 @@ export const setHooks = createAction(getAction('SET_HOOKS'));
 export const setLayout = createAction(getAction('SET_LAYOUT'));
 export const onError = createAction(getAction('ON_ERROR'));
 
-export {setAppIsReady};
-
 export function hydrateInitialOutputs() {
     return function(dispatch, getState) {
         triggerDefaultState(dispatch, getState);
@@ -221,11 +221,13 @@ 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,
+            layout,
+            paths,
+            requestQueue,
+        } = getState();
 
         const {InputGraph} = graphs;
         /*
@@ -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(layout, paths, 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
@@ -950,8 +976,6 @@ function updateOutput(
                                 );
                             });
                         }
-
-                        dispatch(setAppIsReady());
                     }
                 };
                 if (multi) {

dash-renderer/src/actions/isAppReady.js

@@ -0,0 +1,28 @@
+import {path} from 'ramda';
+import {isReady} from '@plotly/dash-component-plugins';
+
+import Registry from '../registry';
+
+export default (layout, paths, targets) => {
+    const promises = [];
+    targets.forEach(id => {
+        const pathOfId = paths[id];
+        if (!pathOfId) {
+            return;
+        }
+
+        const target = path(pathOfId, layout);
+        if (!target) {
+            return;
+        }
+
+        const component = Registry.resolve(target);
+        const ready = isReady(component);
+
+        if (ready && typeof ready.then === 'function') {
+            promises.push(ready);
+        }
+    });
+
+    return promises.length ? Promise.all(promises) : true;
+};

dash-renderer/src/reducers/reducer.js

@@ -10,7 +10,6 @@ import {
     view,
 } from 'ramda';
 import {combineReducers} from 'redux';
-import isAppReady from './isAppReady';
 import layout from './layout';
 import graphs from './dependencyGraph';
 import paths from './paths';
@@ -32,7 +31,6 @@ export const apiRequests = [
 function mainReducer() {
     const parts = {
         appLifecycle,
-        isAppReady,
         layout,
         graphs,
         paths,

dash-renderer/tests/notifyObservers.test.js

@@ -1,58 +1,43 @@
-import { notifyObservers } from "../src/actions";
+import isAppReady from "../src/actions/isAppReady";
 
 const WAIT = 1000;
 
 describe('notifyObservers', () => {
-    const thunk = notifyObservers({
-        id: 'id',
-        props: {},
-        undefined
+    let resolve;
+    beforeEach(() => {
+        const promise = new Promise(r => {
+            resolve = r;
+        });
+
+        window.__components = {
+            a: { _dashprivate_isLazyComponentReady: promise },
+            b: {}
+        };
     });
 
     it('executes if app is ready', async () => {
         let done = false;
-        thunk(
-            () => { },
-            () => ({
-                graphs: {
-                    InputGraph: {
-                        hasNode: () => false,
-                        dependenciesOf: () => [],
-                        dependantsOf: () => [],
-                        overallOrder: () => 0
-                    }
-                },
-                isAppReady: true,
-                requestQueue: []
-            })
-        ).then(() => { done = true; });
+        Promise.resolve(isAppReady(
+            [{ namespace: '__components', type: 'b', props: { id: 'comp1' } }],
+            { comp1: [0] },
+            ['comp1']
+        )).then(() => {
+            done = true
+        });
 
-        await new Promise(r => setTimeout(r, 0));
+        await new Promise(r => setTimeout(r, WAIT));
         expect(done).toEqual(true);
     });
 
     it('waits on app to be ready', async () => {
-        let resolve;
-        const isAppReady = new Promise(r => {
-            resolve = r;
-        });
-
         let done = false;
-        thunk(
-            () => { },
-            () => ({
-                graphs: {
-                    InputGraph: {
-                        hasNode: () => false,
-                        dependenciesOf: () => [],
-                        dependantsOf: () => [],
-                        overallOrder: () => 0
-                    }
-                },
-                isAppReady,
-                requestQueue: []
-            })
-        ).then(() => { done = true; });
+        Promise.resolve(isAppReady(
+            [{ namespace: '__components', type: 'a', props: { id: 'comp1' } }],
+            { comp1: [0] },
+            ['comp1']
+        )).then(() => {
+            done = true
+        });
 
         await new Promise(r => setTimeout(r, WAIT));
         expect(done).toEqual(false);
@@ -62,5 +47,4 @@ describe('notifyObservers', () => {
         await new Promise(r => setTimeout(r, WAIT));
         expect(done).toEqual(true);
     });
-
 });

tests/integration/callbacks/test_basic_callback.py

@@ -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):
@@ -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"
+
+    dash_duo.start_server(app)
+
+    dash_duo.find_element('#btn').click()
+    assert dash_duo.find_element('#output').text == "Bye"
+    assert dash_duo.get_logs() == []