diff --git a/CHANGELOG.md b/CHANGELOG.md index c01162d74e..a815905269 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/dash-renderer/src/APIController.react.js b/dash-renderer/src/APIController.react.js index 1d8e7054ed..ae9f84aa93 100644 --- a/dash-renderer/src/APIController.react.js +++ b/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: []})); } diff --git a/dash-renderer/src/actions/constants.js b/dash-renderer/src/actions/constants.js index 5438c8b4dc..37866de19d 100644 --- a/dash-renderer/src/actions/constants.js +++ b/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 => { diff --git a/dash-renderer/src/actions/index.js b/dash-renderer/src/actions/index.js index 029220af9c..c6c9b6c772 100644 --- a/dash-renderer/src/actions/index.js +++ b/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) { diff --git a/dash-renderer/src/actions/isAppReady.js b/dash-renderer/src/actions/isAppReady.js new file mode 100644 index 0000000000..c83019da54 --- /dev/null +++ b/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; +}; diff --git a/dash-renderer/src/actions/setAppReadyState.js b/dash-renderer/src/actions/setAppReadyState.js deleted file mode 100644 index eeccacd8a9..0000000000 --- a/dash-renderer/src/actions/setAppReadyState.js +++ /dev/null @@ -1,77 +0,0 @@ -import {filter} from 'ramda'; -import {createAction} from 'redux-actions'; - -import isSimpleComponent from '../isSimpleComponent'; -import Registry from './../registry'; -import {getAction} from './constants'; -import {isReady} from '@plotly/dash-component-plugins'; - -const isAppReady = layout => { - const queue = [layout]; - - const res = {}; - - /* Would be much simpler if the Registry was aware of what it contained... */ - while (queue.length) { - const elementLayout = queue.shift(); - if (!elementLayout) { - continue; - } - - const children = elementLayout.props && elementLayout.props.children; - const namespace = elementLayout.namespace; - const type = elementLayout.type; - - res[namespace] = res[namespace] || {}; - res[namespace][type] = type; - - if (children) { - const filteredChildren = filter( - child => !isSimpleComponent(child), - Array.isArray(children) ? children : [children] - ); - - queue.push(...filteredChildren); - } - } - - const promises = []; - Object.entries(res).forEach(([namespace, item]) => { - Object.entries(item).forEach(([type]) => { - const component = Registry.resolve({ - namespace, - type, - }); - - const ready = isReady(component); - - if (ready && typeof ready.then === 'function') { - promises.push(ready); - } - }); - }); - - return promises.length ? Promise.all(promises) : true; -}; - -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)); - } -}; diff --git a/dash-renderer/src/reducers/isAppReady.js b/dash-renderer/src/reducers/isAppReady.js deleted file mode 100644 index 2dd86ece71..0000000000 --- a/dash-renderer/src/reducers/isAppReady.js +++ /dev/null @@ -1,8 +0,0 @@ -import {getAction} from '../actions/constants'; - -export default function config(state = false, action) { - if (action.type === getAction('SET_APP_READY')) { - return action.payload; - } - return state; -} diff --git a/dash-renderer/src/reducers/reducer.js b/dash-renderer/src/reducers/reducer.js index 22af71779b..1123134675 100644 --- a/dash-renderer/src/reducers/reducer.js +++ b/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, diff --git a/dash-renderer/tests/isAppReady.test.js b/dash-renderer/tests/isAppReady.test.js new file mode 100644 index 0000000000..a85ea81240 --- /dev/null +++ b/dash-renderer/tests/isAppReady.test.js @@ -0,0 +1,50 @@ +import isAppReady from "../src/actions/isAppReady"; + +const WAIT = 1000; + +describe('isAppReady', () => { + 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; + Promise.resolve(isAppReady( + [{ namespace: '__components', type: 'b', props: { id: 'comp1' } }], + { comp1: [0] }, + ['comp1'] + )).then(() => { + done = true + }); + + await new Promise(r => setTimeout(r, WAIT)); + expect(done).toEqual(true); + }); + + it('waits on app to be ready', async () => { + let done = false; + 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); + + resolve(); + + await new Promise(r => setTimeout(r, WAIT)); + expect(done).toEqual(true); + }); +}); \ No newline at end of file diff --git a/dash-renderer/tests/notifyObservers.test.js b/dash-renderer/tests/notifyObservers.test.js deleted file mode 100644 index 3c023ab006..0000000000 --- a/dash-renderer/tests/notifyObservers.test.js +++ /dev/null @@ -1,66 +0,0 @@ -import { notifyObservers } from "../src/actions"; - -const WAIT = 1000; - -describe('notifyObservers', () => { - const thunk = notifyObservers({ - id: 'id', - props: {}, - undefined - }); - - 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; }); - - await new Promise(r => setTimeout(r, 0)); - 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; }); - - await new Promise(r => setTimeout(r, WAIT)); - expect(done).toEqual(false); - - resolve(); - - await new Promise(r => setTimeout(r, WAIT)); - expect(done).toEqual(true); - }); - -}); \ No newline at end of file diff --git a/tests/integration/callbacks/test_basic_callback.py b/tests/integration/callbacks/test_basic_callback.py index c94d29dd1c..48fd53a7b6 100644 --- a/tests/integration/callbacks/test_basic_callback.py +++ b/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() == [] diff --git a/tests/integration/test_render.py b/tests/integration/test_render.py index ea1915bc7f..04e4bbc3c2 100644 --- a/tests/integration/test_render.py +++ b/tests/integration/test_render.py @@ -972,11 +972,11 @@ def render_content(tab): )[0].click() graph_1_expected_clickdata = { - "points": [{"curveNumber": 0, "pointNumber": 1, "pointIndex": 1, "x": 2, "y": 10}] + "points": [{"curveNumber": 0, "pointNumber": 1, "pointIndex": 1, "x": 2, "y": 10, "label": 2, "value": 10}] } graph_2_expected_clickdata = { - "points": [{"curveNumber": 0, "pointNumber": 1, "pointIndex": 1, "x": 3, "y": 10}] + "points": [{"curveNumber": 0, "pointNumber": 1, "pointIndex": 1, "x": 3, "y": 10, "label": 3, "value": 10}] } self.wait_for_text_to_equal('#graph1_info', json.dumps(graph_1_expected_clickdata))