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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions dash-renderer/src/APIController.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
computePaths,
hydrateInitialOutputs,
setLayout,
setAppIsReady,
} from './actions/index';
import {applyPersistence} from './persistence';
import apiThunk from './actions/api';
Expand Down Expand Up @@ -55,7 +54,6 @@ class UnconnectedContainer extends Component {
dispatch
);
dispatch(setLayout(finalLayout));
dispatch(setAppIsReady());
} else if (isNil(paths)) {
dispatch(computePaths({subTree: layout, startingPath: []}));
}
Expand Down
1 change: 0 additions & 1 deletion dash-renderer/src/actions/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
44 changes: 34 additions & 10 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 All @@ -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'));
Expand All @@ -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);
Expand Down Expand Up @@ -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;
/*
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(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
Expand Down Expand Up @@ -950,8 +976,6 @@ function updateOutput(
);
});
}

dispatch(setAppIsReady());
}
};
if (multi) {
Expand Down
28 changes: 28 additions & 0 deletions dash-renderer/src/actions/isAppReady.js
Original file line number Diff line number Diff line change
@@ -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;
};
77 changes: 0 additions & 77 deletions dash-renderer/src/actions/setAppReadyState.js

This file was deleted.

8 changes: 0 additions & 8 deletions dash-renderer/src/reducers/isAppReady.js

This file was deleted.

2 changes: 0 additions & 2 deletions dash-renderer/src/reducers/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -32,7 +31,6 @@ export const apiRequests = [
function mainReducer() {
const parts = {
appLifecycle,
isAppReady,
layout,
graphs,
paths,
Expand Down
50 changes: 50 additions & 0 deletions dash-renderer/tests/isAppReady.test.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
66 changes: 0 additions & 66 deletions dash-renderer/tests/notifyObservers.test.js

This file was deleted.

Loading