fix #1105 - isAppReady with an unrendered but in-layout async component

Author: alexcjohnson

Diff for: dash-renderer/src/APIController.react.js

@@ -8,6 +8,7 @@ import {hydrateInitialOutputs, setGraphs, setPaths, setLayout} from './actions';
 import {computePaths} from './actions/paths';
 import {computeGraphs} from './actions/dependencies';
 import apiThunk from './actions/api';
+import {EventEmitter} from './actions/utils';
 import {applyPersistence} from './persistence';
 import {getAppState} from './reducers/constants';
 import {STATUS} from './constants/constants';
@@ -19,18 +20,29 @@ class UnconnectedContainer extends Component {
     constructor(props) {
         super(props);
         this.initialization = this.initialization.bind(this);
+        this.emitReady = this.emitReady.bind(this);
         this.state = {
             errorLoading: false,
         };
+
+        // Event emitter to communicate when the DOM is ready
+        this.events = new EventEmitter();
+        // Flag to determine if we've really updated the dash components
+        this.renderedTree = false;
     }
     componentDidMount() {
         this.initialization(this.props);
+        this.emitReady();
     }
 
     componentWillReceiveProps(props) {
         this.initialization(props);
     }
 
+    componentDidUpdate() {
+        this.emitReady();
+    }
+
     initialization(props) {
         const {
             appLifecycle,
@@ -49,7 +61,9 @@ class UnconnectedContainer extends Component {
                     layoutRequest.content,
                     dispatch
                 );
-                dispatch(setPaths(computePaths(finalLayout, [])));
+                dispatch(
+                    setPaths(computePaths(finalLayout, [], null, this.events))
+                );
                 dispatch(setLayout(finalLayout));
             }
         }
@@ -88,6 +102,13 @@ class UnconnectedContainer extends Component {
         }
     }
 
+    emitReady() {
+        if (this.renderedTree) {
+            this.renderedTree = false;
+            this.events.emit('rendered');
+        }
+    }
+
     render() {
         const {
             appLifecycle,
@@ -113,19 +134,19 @@ class UnconnectedContainer extends Component {
                 <div className="_dash-error">Error loading dependencies</div>
             );
         } else if (appLifecycle === getAppState('HYDRATED')) {
-            return config.ui === true ? (
-                <GlobalErrorContainer>
-                    <TreeContainer
-                        _dashprivate_layout={layout}
-                        _dashprivate_path={[]}
-                    />
-                </GlobalErrorContainer>
-            ) : (
+            this.renderedTree = true;
+
+            const tree = (
                 <TreeContainer
                     _dashprivate_layout={layout}
                     _dashprivate_path={[]}
                 />
             );
+            return config.ui === true ? (
+                <GlobalErrorContainer>{tree}</GlobalErrorContainer>
+            ) : (
+                tree
+            );
         }
 
         return <div className="_dash-loading">Loading...</div>;

Diff for: dash-renderer/src/TreeContainer.js

@@ -80,11 +80,7 @@ function CheckedComponent(p) {
         propTypeErrorHandler(errorMessage, props, type);
     }
 
-    return React.createElement(
-        element,
-        mergeRight(props, extraProps),
-        ...(Array.isArray(children) ? children : [children])
-    );
+    return createElement(element, props, extraProps, children);
 }
 
 CheckedComponent.propTypes = {
@@ -95,6 +91,15 @@ CheckedComponent.propTypes = {
     extraProps: PropTypes.any,
     id: PropTypes.string,
 };
+
+function createElement(element, props, extraProps, children) {
+    const allProps = mergeRight(props, extraProps);
+    if (Array.isArray(children)) {
+        return React.createElement(element, allProps, ...children);
+    }
+    return React.createElement(element, allProps, children);
+}
+
 class TreeContainer extends Component {
     constructor(props) {
         super(props);
@@ -188,6 +193,7 @@ class TreeContainer extends Component {
             // just the id we pass on to the rendered component
             props.id = stringifyId(props.id);
         }
+        const extraProps = {loading_state, setProps};
 
         return (
             <ComponentErrorBoundary
@@ -200,15 +206,11 @@ class TreeContainer extends Component {
                         children={children}
                         element={element}
                         props={props}
-                        extraProps={{loading_state, setProps}}
+                        extraProps={extraProps}
                         type={_dashprivate_layout.type}
                     />
                 ) : (
-                    React.createElement(
-                        element,
-                        mergeRight(props, {loading_state, setProps}),
-                        ...(Array.isArray(children) ? children : [children])
-                    )
+                    createElement(element, props, extraProps, children)
                 )}
             </ComponentErrorBoundary>
         );

Diff for: dash-renderer/src/actions/isAppReady.js

@@ -3,10 +3,19 @@ import {isReady} from '@plotly/dash-component-plugins';
 
 import Registry from '../registry';
 import {getPath} from './paths';
+import {stringifyId} from './dependencies';
 
 export default (layout, paths, targets) => {
+    if (!targets.length) {
+        return true;
+    }
     const promises = [];
 
+    const {events} = paths;
+    const rendered = new Promise(resolveRendered => {
+        events.once('rendered', resolveRendered);
+    });
+
     targets.forEach(id => {
         const pathOfId = getPath(paths, id);
         if (!pathOfId) {
@@ -22,7 +31,14 @@ export default (layout, paths, targets) => {
         const ready = isReady(component);
 
         if (ready && typeof ready.then === 'function') {
-            promises.push(ready);
+            promises.push(
+                Promise.race([
+                    ready,
+                    rendered.then(
+                        () => document.getElementById(stringifyId(id)) && ready
+                    ),
+                ])
+            );
         }
     });
 

Diff for: dash-renderer/src/actions/paths.js

@@ -20,7 +20,7 @@ import {crawlLayout} from './utils';
  * values: array of values in the id, in order of keys
  */
 
-export function computePaths(subTree, startingPath, oldPaths) {
+export function computePaths(subTree, startingPath, oldPaths, events) {
     const {strs: oldStrs, objs: oldObjs} = oldPaths || {strs: {}, objs: {}};
 
     const diffHead = path => startingPath.some((v, i) => path[i] !== v);
@@ -53,7 +53,9 @@ export function computePaths(subTree, startingPath, oldPaths) {
         }
     });
 
-    return {strs, objs};
+    // We include an event emitter here because it will be used along with
+    // paths to determine when the app is ready for callbacks.
+    return {strs, objs, events: events || oldPaths.events};
 }
 
 export function getPath(paths, id) {

Diff for: dash-renderer/src/actions/utils.js

@@ -43,3 +43,37 @@ export const crawlLayout = (object, func, currentPath = []) => {
         }
     }
 };
+
+// There are packages for this but it's simple enough, I just
+// adapted it from https://gist.github.com/mudge/5830382
+export class EventEmitter {
+    constructor() {
+        this._ev = {};
+    }
+    on(event, listener) {
+        const events = (this._ev[event] = this._ev[event] || []);
+        events.push(listener);
+        return () => this.removeListener(event, listener);
+    }
+    removeListener(event, listener) {
+        const events = this._ev[event];
+        if (events) {
+            const idx = events.indexOf(listener);
+            if (idx > -1) {
+                events.splice(idx, 1);
+            }
+        }
+    }
+    emit(event, ...args) {
+        const events = this._ev[event];
+        if (events) {
+            events.forEach(listener => listener.apply(this, args));
+        }
+    }
+    once(event, listener) {
+        const remove = this.on(event, (...args) => {
+            remove();
+            listener.apply(this, args);
+        });
+    }
+}

Diff for: tests/integration/callbacks/test_basic_callback.py

@@ -177,47 +177,63 @@ def update_out(n_clicks):
     assert dash_duo.get_logs() == []
 
 
-@pytest.mark.skip(reason="https://github.com/plotly/dash/issues/1105")
 def test_cbsc004_callback_using_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",
-                            columns=[{"id": "a", "name": "A"}],
-                            data=[{"a": "b"}]
-                        )
-                    ),
-                ]
-            )
-        ]
-    )
+    app.layout = html.Div([
+        dcc.Tabs([
+            dcc.Tab("boo!"),
+            dcc.Tab(
+                dash_table.DataTable(
+                    id="table",
+                    columns=[{"id": "a", "name": "A"}],
+                    data=[{"a": "b"}]
+                )
+            ),
+        ]),
+        html.Button("Update Input", id="btn"),
+        html.Div("Hello", id="output"),
+        html.Div(id="output2")
+    ])
 
     @app.callback(
         Output("output", "children"),
         [Input("btn", "n_clicks")],
-        [State("other-table", "data")]
+        [State("table", "data")]
     )
     def update_out(n_clicks, data):
-        if n_clicks is None:
-            return len(data)
+        return json.dumps(data) + ' - ' + str(n_clicks)
 
+    @app.callback(
+        Output("output2", "children"),
+        [Input("btn", "n_clicks")],
+        [State("table", "derived_viewport_data")]
+    )
+    def update_out2(n_clicks, data):
         return json.dumps(data) + ' - ' + str(n_clicks)
 
     dash_duo.start_server(app)
 
     dash_duo.wait_for_text_to_equal("#output", '[{"a": "b"}] - None')
+    dash_duo.wait_for_text_to_equal("#output2", 'null - None')
+
     dash_duo.find_element("#btn").click()
     dash_duo.wait_for_text_to_equal("#output", '[{"a": "b"}] - 1')
+    dash_duo.wait_for_text_to_equal("#output2", 'null - 1')
+
+    dash_duo.find_element(".tab:not(.tab--selected)").click()
+    dash_duo.wait_for_text_to_equal("#table th", "A")
+    # table props are in state so no change yet
+    dash_duo.wait_for_text_to_equal("#output2", 'null - 1')
+
+    # repeat a few times, since one of the failure modes I saw during dev was
+    # intermittent - but predictably so?
+    for i in range(2, 10):
+        expected = '[{"a": "b"}] - ' + str(i)
+        dash_duo.find_element("#btn").click()
+        dash_duo.wait_for_text_to_equal("#output", expected)
+        # now derived props are available
+        dash_duo.wait_for_text_to_equal("#output2", expected)
+
     assert dash_duo.get_logs() == []