From 0dca300ffbff3e7bce3676705bd3d3082e741d4b Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 15:55:25 -0400 Subject: [PATCH 01/13] rm debugging statements --- tests/IntegrationTests.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/IntegrationTests.py b/tests/IntegrationTests.py index 2730336..5650ecd 100644 --- a/tests/IntegrationTests.py +++ b/tests/IntegrationTests.py @@ -24,10 +24,6 @@ def percy_snapshot(cls, name=''): @classmethod def setUpClass(cls): - print('PERCY_PARALLEL_NONCE') - print(os.environ['PERCY_PARALLEL_NONCE']) - print('PERCY_PARALLEL_TOTAL') - print(os.environ['PERCY_PARALLEL_TOTAL']) super(IntegrationTests, cls).setUpClass() cls.driver = webdriver.Chrome() From c3b5ecefdde70fdffb89f7dbd576af63baed6124 Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 15:56:10 -0400 Subject: [PATCH 02/13] add failing test this test demonstrates the issue found in https://github.com/plotly/dash/issues/133#issuecomment-332299141 --- tests/test_render.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/test_render.py b/tests/test_render.py index 093313d..8b67c5e 100644 --- a/tests/test_render.py +++ b/tests/test_render.py @@ -1501,3 +1501,35 @@ def dynamic_output(*args): ) self.assertEqual(call_count.value, 1) + + def test_callbacks_called_multiple_times_and_out_of_order(self): + app = Dash(__name__) + app.layout = html.Div([ + html.Button(id='input', n_clicks=0), + html.Div(id='output') + ]) + + call_count = Value('i', 0) + + @app.callback( + Output('output', 'children'), + [Input('input', 'n_clicks')]) + def update_output(n_clicks): + call_count.value = call_count.value + 1 + if n_clicks == 1: + time.sleep(4) + return n_clicks + + self.startServer(app) + button = self.wait_for_element_by_id('input') + button.click() + button.click() + time.sleep(8) + self.percy_snapshot( + name='test_callbacks_called_multiple_times_and_out_of_order' + ) + self.assertEqual(call_count.value, 3) + self.assertEqual( + self.driver.find_element_by_id('output').text, + '2' + ) From 96d5c46091dc4d60e65950b9e6f485c4d605808a Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 18:24:41 -0400 Subject: [PATCH 03/13] reject old responses of the same output component fixes https://github.com/plotly/dash/issues/133 --- src/actions/index.js | 62 +++++++++++++++++----- src/components/core/DocumentTitle.react.js | 4 +- src/components/core/Loading.react.js | 6 +-- src/utils.js | 10 ++++ 4 files changed, 63 insertions(+), 19 deletions(-) diff --git a/src/actions/index.js b/src/actions/index.js index 6da1100..1e34651 100644 --- a/src/actions/index.js +++ b/src/actions/index.js @@ -1,18 +1,23 @@ /* global fetch:true, Promise:true, document:true */ import { + adjust, + any, concat, contains, + findIndex, + findLastIndex, has, intersection, isEmpty, keys, lensPath, + lensProp, pluck, - reject, + propEq, + set, slice, sort, type, - union, view } from 'ramda'; import {createAction} from 'redux-actions'; @@ -20,7 +25,7 @@ import {crawlLayout, hasId} from '../reducers/utils'; import {APP_STATES} from '../reducers/constants'; import {ACTIONS} from './constants'; import cookie from 'cookie'; -import {urlBase} from '../utils'; +import {uid, urlBase} from '../utils'; export const updateProps = createAction(ACTIONS('ON_PROP_CHANGE')); export const setRequestQueue = createAction(ACTIONS('SET_REQUEST_QUEUE')); @@ -253,8 +258,9 @@ export function notifyObservers(payload) { * the change for Child. if this update has already been queued up, * then skip the update for the other component */ - const controllersInExistingQueue = intersection( - requestQueue, controllers + const controllerIsInExistingQueue = any(r => + contains(r.controllerId, controllers) && r.status === 'loading', + requestQueue ); /* @@ -267,7 +273,7 @@ export function notifyObservers(payload) { if ( (controllersInFutureQueue.length === 0) && (has(outputComponentId, getState().paths)) && - (controllersInExistingQueue.length === 0) + !controllerIsInExistingQueue ) { queuedObservers.push(outputIdAndProp) } @@ -278,7 +284,15 @@ export function notifyObservers(payload) { * updated in a queue. not all of these requests will be fired in this * action */ - dispatch(setRequestQueue(union(queuedObservers, requestQueue))); + const newRequestQueue = queuedObservers.map( + i => ({controllerId: i, status: 'loading', uid: uid()}) + ) + dispatch(setRequestQueue( + concat( + requestQueue, + newRequestQueue + ) + )); const promises = []; for (let i = 0; i < queuedObservers.length; i++) { @@ -356,13 +370,33 @@ export function notifyObservers(payload) { payload: {status: res.status} }); - // clear this item from the request queue - dispatch(setRequestQueue( - reject( - id => id === outputIdAndProp, - getState().requestQueue - ) - )); + // update the status of this request + const postRequestQueue = getState().requestQueue; + const requestUid = newRequestQueue[i].uid; + const thisRequestIndex = findIndex( + propEq('uid', requestUid), + postRequestQueue + ); + const updatedQueue = adjust( + set(lensProp('status'), res.status), + thisRequestIndex, + postRequestQueue + ); + dispatch(setRequestQueue(updatedQueue)); + + /* + * Check to see if another request has already come back + * _after_ this one. + * If so, ignore this request. + */ + const latestRequestIndex = findLastIndex( + propEq('controllerId', newRequestQueue[i].controllerId), + updatedQueue + ); + if (latestRequestIndex > thisRequestIndex && + updatedQueue[latestRequestIndex].status === 200) { + return; + } return res.json().then(function handleJson(data) { diff --git a/src/components/core/DocumentTitle.react.js b/src/components/core/DocumentTitle.react.js index 685d388..a96087e 100644 --- a/src/components/core/DocumentTitle.react.js +++ b/src/components/core/DocumentTitle.react.js @@ -1,7 +1,7 @@ /* global document:true */ import {connect} from 'react-redux' -import {isEmpty} from 'ramda' +import {any} from 'ramda' import {Component, PropTypes} from 'react' class DocumentTitle extends Component { @@ -13,7 +13,7 @@ class DocumentTitle extends Component { } componentWillReceiveProps(props) { - if (!isEmpty(props.requestQueue)) { + if (any(r => r.status === 'loading', props.requestQueue)) { document.title = 'Updating...'; } else { document.title = this.state.initialTitle; diff --git a/src/components/core/Loading.react.js b/src/components/core/Loading.react.js index 8a39920..22aaa1b 100644 --- a/src/components/core/Loading.react.js +++ b/src/components/core/Loading.react.js @@ -1,12 +1,12 @@ import {connect} from 'react-redux' -import {isEmpty} from 'ramda' +import {any} from 'ramda' import React, {PropTypes} from 'react' function Loading(props) { - if (!isEmpty(props.requestQueue)) { + if (any(r => r.status === 'loading', props.requestQueue)) { return (
- ) + ); } else { return null; } diff --git a/src/utils.js b/src/utils.js index 57cb455..fe0559f 100644 --- a/src/utils.js +++ b/src/utils.js @@ -25,3 +25,13 @@ export function urlBase(config) { requests_pathname_prefix from config`, config); } } + +export function uid() { + function s4() { + return Math.floor((1 + Math.random()) * 0x10000) + .toString(16) + .substring(1); + } + return s4() + s4() + '-' + s4() + '-' + s4() + '-' + + s4() + '-' + s4() + s4() + s4(); +} From e751cf924a5fe126e69acc5b0efcbd69f2eec19e Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 18:25:15 -0400 Subject: [PATCH 04/13] remove `lastUpdateComponentRequest` it was only used for auth and auth is now a separate package --- src/APIController.react.js | 21 ++------------------- src/actions/index.js | 4 ---- src/reducers/api.js | 3 --- src/reducers/reducer.js | 1 - 4 files changed, 2 insertions(+), 27 deletions(-) diff --git a/src/APIController.react.js b/src/APIController.react.js index ce8ca34..1335d65 100644 --- a/src/APIController.react.js +++ b/src/APIController.react.js @@ -1,5 +1,5 @@ import {connect} from 'react-redux' -import {any, contains, equals, isEmpty, isNil} from 'ramda' +import {contains, isEmpty, isNil} from 'ramda' import React, {Component, PropTypes} from 'react'; import renderTree from './renderTree'; import { @@ -10,7 +10,6 @@ import { } from './actions/index'; import {getDependencies, getLayout} from './actions/api'; import {APP_STATES} from './reducers/constants'; -import AccessDenied from './AccessDenied.react'; /** * Fire off API calls for initialization @@ -75,24 +74,12 @@ class UnconnectedContainer extends Component { render () { const { appLifecycle, - config, dependenciesRequest, - lastUpdateComponentRequest, layoutRequest, layout } = this.props; - // Auth protected routes - if (any(equals(true), - [dependenciesRequest, lastUpdateComponentRequest, - layoutRequest].map( - request => (request.status && request.status === 403)) - )) { - return (); - } - - - else if (layoutRequest.status && + if (layoutRequest.status && !contains(layoutRequest.status, [200, 'loading']) ) { return (
{'Error loading layout'}
); @@ -126,9 +113,7 @@ UnconnectedContainer.propTypes = { APP_STATES('HYDRATED') ]), dispatch: PropTypes.function, - config: PropTypes.object, dependenciesRequest: PropTypes.object, - lastUpdateComponentRequest: PropTypes.objec, layoutRequest: PropTypes.object, layout: PropTypes.object, paths: PropTypes.object, @@ -139,9 +124,7 @@ const Container = connect( // map state to props state => ({ appLifecycle: state.appLifecycle, - config: state.config, dependenciesRequest: state.dependenciesRequest, - lastUpdateComponentRequest: state.lastUpdateComponentRequest, layoutRequest: state.layoutRequest, layout: state.layout, graphs: state.graphs, diff --git a/src/actions/index.js b/src/actions/index.js index 1e34651..5780d40 100644 --- a/src/actions/index.js +++ b/src/actions/index.js @@ -365,10 +365,6 @@ export function notifyObservers(payload) { credentials: 'same-origin', body: JSON.stringify(payload) }).then(function handleResponse(res) { - dispatch({ - type: 'lastUpdateComponentRequest', - payload: {status: res.status} - }); // update the status of this request const postRequestQueue = getState().requestQueue; diff --git a/src/reducers/api.js b/src/reducers/api.js index f929c30..a7e8d95 100644 --- a/src/reducers/api.js +++ b/src/reducers/api.js @@ -27,8 +27,5 @@ function createApiReducer(store) { } export const dependenciesRequest = createApiReducer('dependenciesRequest'); -export const lastUpdateComponentRequest = createApiReducer( - 'lastUpdateComponentRequest' -); export const layoutRequest = createApiReducer('layoutRequest'); export const loginRequest = createApiReducer('loginRequest'); diff --git a/src/reducers/reducer.js b/src/reducers/reducer.js index 1d0d5d2..33ca050 100644 --- a/src/reducers/reducer.js +++ b/src/reducers/reducer.js @@ -19,7 +19,6 @@ const reducer = combineReducers({ config, dependenciesRequest: API.dependenciesRequest, layoutRequest: API.layoutRequest, - lastUpdateComponentRequest: API.lastUpdateComponentRequest, loginRequest: API.loginRequest, history }); From 2d78de54596652d7d6bb3fd24e8b8cb6962a52d6 Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 18:25:28 -0400 Subject: [PATCH 05/13] add comment about throttling --- src/actions/index.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/actions/index.js b/src/actions/index.js index 5780d40..d859494 100644 --- a/src/actions/index.js +++ b/src/actions/index.js @@ -263,6 +263,19 @@ export function notifyObservers(payload) { requestQueue ); + /* + * TODO - Place throttling logic here? + * + * Only process the last two requests for a _single_ output + * at a time. + * + * For example, if A -> B, and A is changed 10 times, then: + * 1 - processing the first two requests + * 2 - if more than 2 requests come in while the first two + * are being processed, then skip updating all of the + * requests except for the last 2 + */ + /* * also check that this observer is actually in the current * component tree. From aa5ae15601c9be90d8e052d05977759fa06ddf11 Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 18:31:14 -0400 Subject: [PATCH 06/13] v0.11.0rc1 --- CHANGELOG.md | 7 +++++++ dash_renderer/__init__.py | 3 ++- dash_renderer/version.py | 2 +- package.json | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bbd836..f801ecc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## [0.11.0rc1] - 2017-09-27 +### Fixed +- 🐞 Previously, old requests could override new requests if their response was longer than the new one. +This caused subtle bugs when apps are deployed on multiple processes or threads with component +callbacks that update at varying rates like urls. Originally reported in github.com/plotly/dash/issues/133 + + ## [0.9.0] - 2017-09-07 ### Fixed - 🐞 Fixed a bug where Dash would fire updates for each parent of a grandchild node that shared the same grandparent. Originally reported in https://community.plot.ly/t/specifying-dependency-tree-traversal/5080/5 diff --git a/dash_renderer/__init__.py b/dash_renderer/__init__.py index d9005d0..d9a753c 100644 --- a/dash_renderer/__init__.py +++ b/dash_renderer/__init__.py @@ -7,7 +7,8 @@ # command in the dash_html_components package which printed out: # `dash_html_components.__init__: module references __file__` # TODO - Understand this better -from .version import __version__ +# from .version import __version__ +__version__ = '0.11.0-rc1' __file__ # Dash renderer's dependencies get loaded in a special order by the server: diff --git a/dash_renderer/version.py b/dash_renderer/version.py index 9d1bb72..6b33f15 100644 --- a/dash_renderer/version.py +++ b/dash_renderer/version.py @@ -1 +1 @@ -__version__ = '0.10.0' +__version__ = '0.11.0rc1' diff --git a/package.json b/package.json index 1dacfd2..a2298ab 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dash-renderer", - "version": "0.10.0", + "version": "0.11.0-rc1", "description": "render dash components in react", "main": "src/index.js", "scripts": { From c271d940763b4ea80ddc4d2a3d46bc169c3682b8 Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 23:01:03 -0400 Subject: [PATCH 07/13] check for out-of-order requests after the async `res.json()` call also add some debugging metadata to the request queue --- src/actions/index.js | 87 +++++++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 29 deletions(-) diff --git a/src/actions/index.js b/src/actions/index.js index d859494..53250fc 100644 --- a/src/actions/index.js +++ b/src/actions/index.js @@ -1,5 +1,6 @@ /* global fetch:true, Promise:true, document:true */ import { + __, adjust, any, concat, @@ -11,10 +12,9 @@ import { isEmpty, keys, lensPath, - lensProp, + merge, pluck, propEq, - set, slice, sort, type, @@ -298,7 +298,12 @@ export function notifyObservers(payload) { * action */ const newRequestQueue = queuedObservers.map( - i => ({controllerId: i, status: 'loading', uid: uid()}) + i => ({ + controllerId: i, + status: 'loading', + uid: uid(), + requestTime: Date.now() + }) ) dispatch(setRequestQueue( concat( @@ -379,35 +384,58 @@ export function notifyObservers(payload) { body: JSON.stringify(payload) }).then(function handleResponse(res) { - // update the status of this request - const postRequestQueue = getState().requestQueue; - const requestUid = newRequestQueue[i].uid; - const thisRequestIndex = findIndex( - propEq('uid', requestUid), - postRequestQueue - ); - const updatedQueue = adjust( - set(lensProp('status'), res.status), - thisRequestIndex, - postRequestQueue - ); - dispatch(setRequestQueue(updatedQueue)); - - /* - * Check to see if another request has already come back - * _after_ this one. - * If so, ignore this request. - */ - const latestRequestIndex = findLastIndex( - propEq('controllerId', newRequestQueue[i].controllerId), - updatedQueue - ); - if (latestRequestIndex > thisRequestIndex && - updatedQueue[latestRequestIndex].status === 200) { + const getThisRequestIndex = () => { + const postRequestQueue = getState().requestQueue; + const requestUid = newRequestQueue[i].uid; + const thisRequestIndex = findIndex( + propEq('uid', requestUid), + postRequestQueue + ); + return thisRequestIndex; + } + + const updateRequestQueue = rejected => { + const postRequestQueue = getState().requestQueue + const thisRequestIndex = getThisRequestIndex(); + const updatedQueue = adjust( + merge(__, { + status: res.status, + responseTime: Date.now(), + rejected + }), + thisRequestIndex, + postRequestQueue + ); + + dispatch(setRequestQueue(updatedQueue)); + } + + if (res.status !== 200) { + // update the status of this request + updateRequestQueue(true); return; } return res.json().then(function handleJson(data) { + /* + * Check to see if another request has already come back + * _after_ this one. + * If so, ignore this request. + */ + const latestRequestIndex = findLastIndex( + propEq('controllerId', newRequestQueue[i].controllerId), + getState().requestQueue + ); + let rejected = latestRequestIndex > getThisRequestIndex(); + + updateRequestQueue(rejected); + + if (rejected) { + /* eslint-disable */ + console.warn('--> update rejected'); + /* eslint-enable */ + return; + } /* * it's possible that this output item is no longer visible. @@ -510,7 +538,8 @@ export function notifyObservers(payload) { } - })})); + }); + })); } From 078dc2ee352223f864f5cb19673e8a963e50a255 Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 23:01:22 -0400 Subject: [PATCH 08/13] simple componentShouldMount in tree --- src/APIController.react.js | 4 ++-- src/{renderTree.js => TreeContainer.js} | 30 ++++++++++++++++++------- 2 files changed, 24 insertions(+), 10 deletions(-) rename src/{renderTree.js => TreeContainer.js} (69%) diff --git a/src/APIController.react.js b/src/APIController.react.js index 1335d65..36a5754 100644 --- a/src/APIController.react.js +++ b/src/APIController.react.js @@ -1,7 +1,7 @@ import {connect} from 'react-redux' import {contains, isEmpty, isNil} from 'ramda' import React, {Component, PropTypes} from 'react'; -import renderTree from './renderTree'; +import TreeContainer from './TreeContainer'; import { computeGraphs, computePaths, @@ -97,7 +97,7 @@ class UnconnectedContainer extends Component { else if (appLifecycle === APP_STATES('HYDRATED')) { return (
- {renderTree(layout, dependenciesRequest.content)} +
); } diff --git a/src/renderTree.js b/src/TreeContainer.js similarity index 69% rename from src/renderTree.js rename to src/TreeContainer.js index e3e18ee..40a7295 100644 --- a/src/renderTree.js +++ b/src/TreeContainer.js @@ -1,11 +1,25 @@ 'use strict' import R from 'ramda'; -import React, {PropTypes} from 'react'; +import React, {Component, PropTypes} from 'react'; import Registry from './registry'; import NotifyObservers from './components/core/NotifyObservers.react'; -export default function render(component) { +export default class TreeContainer extends Component { + shouldComponentUpdate(nextProps) { + return nextProps.layout !== this.props.layout; + } + + render() { + return render(this.props.layout); + } +} + +TreeContainer.propTypes = { + layout: PropTypes.object, +} + +function render(component) { if (R.contains(R.type(component), ['String', 'Number', 'Null'])) { return component; } @@ -13,7 +27,7 @@ export default function render(component) { // Create list of child elements let children; - const props = R.propOr({}, 'props', component); + const componentProps = R.propOr({}, 'props', component); if (!R.has('props', component) || !R.has('children', component.props) || @@ -34,8 +48,9 @@ export default function render(component) { // One or multiple objects // Recursively render the tree // TODO - I think we should pass in `key` here. - children = (Array.isArray(props.children) ? props.children : [props.children]) - .map(render); + children = (Array.isArray(componentProps.children) ? + componentProps.children : [componentProps.children]) + .map(render); } @@ -60,13 +75,12 @@ export default function render(component) { ); return ( - + {parent} ); } render.propTypes = { - children: PropTypes.object, - id: PropTypes.string + children: PropTypes.object } From 1af8230cfc767237ba56d4e2e642a69c64fc4ecd Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 23:02:00 -0400 Subject: [PATCH 09/13] v0.11.0-rc4 --- dash_renderer/__init__.py | 2 +- dash_renderer/version.py | 2 +- package.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dash_renderer/__init__.py b/dash_renderer/__init__.py index d9a753c..58b10ae 100644 --- a/dash_renderer/__init__.py +++ b/dash_renderer/__init__.py @@ -8,7 +8,7 @@ # `dash_html_components.__init__: module references __file__` # TODO - Understand this better # from .version import __version__ -__version__ = '0.11.0-rc1' +__version__ = '0.11.0-rc4' __file__ # Dash renderer's dependencies get loaded in a special order by the server: diff --git a/dash_renderer/version.py b/dash_renderer/version.py index 6b33f15..9fe681e 100644 --- a/dash_renderer/version.py +++ b/dash_renderer/version.py @@ -1 +1 @@ -__version__ = '0.11.0rc1' +__version__ = '0.11.0rc4' diff --git a/package.json b/package.json index a2298ab..c7a64d2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dash-renderer", - "version": "0.11.0-rc1", + "version": "0.11.0-rc4", "description": "render dash components in react", "main": "src/index.js", "scripts": { From 92365a822854b1dca7ef823708d5a8d588822ed7 Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 23:29:26 -0400 Subject: [PATCH 10/13] check for out-of-order responses upon receiving headers *and* upon receiving body (json) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit in other words, in `res.then(…)` *and* in `res.json().then(…)`. fetch exposes 2 async methods! --- src/actions/index.js | 52 +++++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/src/actions/index.js b/src/actions/index.js index 53250fc..e8367a4 100644 --- a/src/actions/index.js +++ b/src/actions/index.js @@ -410,32 +410,50 @@ export function notifyObservers(payload) { dispatch(setRequestQueue(updatedQueue)); } + const isRejected = () => { + const latestRequestIndex = findLastIndex( + propEq('controllerId', newRequestQueue[i].controllerId), + getState().requestQueue + ); + const rejected = latestRequestIndex > getThisRequestIndex(); + return rejected; + } + if (res.status !== 200) { // update the status of this request updateRequestQueue(true); return; } + /* + * Check to see if another request has already come back + * _after_ this one. + * If so, ignore this request. + */ + if (isRejected()) { + /* eslint-disable */ + console.warn('---> rejecting old response (upon receiving headers)'); + /* eslint-enable */ + updateRequestQueue(true); + return; + } + return res.json().then(function handleJson(data) { /* - * Check to see if another request has already come back - * _after_ this one. - * If so, ignore this request. + * Even if the `res` was received in the correct order, + * the remainder of the response (res.json()) could happen + * at different rates causing the parsed responses to + * get out of order */ - const latestRequestIndex = findLastIndex( - propEq('controllerId', newRequestQueue[i].controllerId), - getState().requestQueue - ); - let rejected = latestRequestIndex > getThisRequestIndex(); - - updateRequestQueue(rejected); - - if (rejected) { - /* eslint-disable */ - console.warn('--> update rejected'); - /* eslint-enable */ - return; - } + if (isRejected()) { + /* eslint-disable */ + console.warn('---> rejecting old response (upon receiving json)'); + /* eslint-enable */ + updateRequestQueue(true); + return; + } + + updateRequestQueue(false); /* * it's possible that this output item is no longer visible. From 061ff85032be69f62a3e416ac7b9eca19860d6c7 Mon Sep 17 00:00:00 2001 From: chriddyp Date: Wed, 27 Sep 2017 23:29:40 -0400 Subject: [PATCH 11/13] v0.11.0-rc5 --- dash_renderer/__init__.py | 2 +- dash_renderer/version.py | 2 +- package.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dash_renderer/__init__.py b/dash_renderer/__init__.py index 58b10ae..304c720 100644 --- a/dash_renderer/__init__.py +++ b/dash_renderer/__init__.py @@ -8,7 +8,7 @@ # `dash_html_components.__init__: module references __file__` # TODO - Understand this better # from .version import __version__ -__version__ = '0.11.0-rc4' +__version__ = '0.11.0-rc5' __file__ # Dash renderer's dependencies get loaded in a special order by the server: diff --git a/dash_renderer/version.py b/dash_renderer/version.py index 9fe681e..d45e7c0 100644 --- a/dash_renderer/version.py +++ b/dash_renderer/version.py @@ -1 +1 @@ -__version__ = '0.11.0rc4' +__version__ = '0.11.0rc5' diff --git a/package.json b/package.json index c7a64d2..136e679 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dash-renderer", - "version": "0.11.0-rc4", + "version": "0.11.0-rc5", "description": "render dash components in react", "main": "src/index.js", "scripts": { From ca211610e1bbcc9c6dd6f2294ad20d4bb2db3440 Mon Sep 17 00:00:00 2001 From: chriddyp Date: Thu, 28 Sep 2017 19:26:36 -0400 Subject: [PATCH 12/13] fix tests --- src/actions/index.js | 6 ---- tests/test_render.py | 66 +++++++++++++++++++++++--------------------- 2 files changed, 35 insertions(+), 37 deletions(-) diff --git a/src/actions/index.js b/src/actions/index.js index e8367a4..510124e 100644 --- a/src/actions/index.js +++ b/src/actions/index.js @@ -431,9 +431,6 @@ export function notifyObservers(payload) { * If so, ignore this request. */ if (isRejected()) { - /* eslint-disable */ - console.warn('---> rejecting old response (upon receiving headers)'); - /* eslint-enable */ updateRequestQueue(true); return; } @@ -446,9 +443,6 @@ export function notifyObservers(payload) { * get out of order */ if (isRejected()) { - /* eslint-disable */ - console.warn('---> rejecting old response (upon receiving json)'); - /* eslint-enable */ updateRequestQueue(true); return; } diff --git a/tests/test_render.py b/tests/test_render.py index 8b67c5e..9303c36 100644 --- a/tests/test_render.py +++ b/tests/test_render.py @@ -21,6 +21,29 @@ def wait_for_element_by_id(id): return self.driver.find_element_by_id(id) self.wait_for_element_by_id = wait_for_element_by_id + def request_queue_assertions( + self, check_rejected=True, expected_length=None): + request_queue = self.driver.execute_script( + 'return window.store.getState().requestQueue' + ) + self.assertTrue( + all([ + (r['status'] == 200) + for r in request_queue + ]) + ) + + if check_rejected: + self.assertTrue( + all([ + (r['rejected'] is False) + for r in request_queue + ]) + ) + + if expected_length is not None: + self.assertEqual(len(request_queue), expected_length) + def test_initial_state(self): app = Dash(__name__) app.layout = html.Div([ @@ -385,12 +408,7 @@ def test_initial_state(self): } ) - self.assertEqual( - self.driver.execute_script( - 'return window.store.getState().requestQueue' - ), - [] - ) + self.request_queue_assertions(0) self.percy_snapshot(name='layout') @@ -443,12 +461,8 @@ def update_output(value): len('hello world') ) - self.assertEqual( - self.driver.execute_script( - 'return window.store.getState().requestQueue' - ), - [] - ) + self.request_queue_assertions( + expected_length=call_count.value, check_rejected=False) assert_clean_console(self) @@ -545,12 +559,7 @@ def update_input(value): self.assertEqual(call_count.value, 2) - self.assertEqual( - self.driver.execute_script( - 'return window.store.getState().requestQueue' - ), - [] - ) + self.request_queue_assertions(call_count.value + 1) self.percy_snapshot(name='callback-generating-function-2') assert_clean_console(self) @@ -728,13 +737,7 @@ def generic_chapter_assertions(chapter): ) == value ) ) - - self.assertEqual( - self.driver.execute_script( - 'return window.store.getState().requestQueue' - ), - [] - ) + self.request_queue_assertions() def chapter1_assertions(): paths = self.driver.execute_script( @@ -960,12 +963,7 @@ def update_output_2(value): self.assertEqual(output_1_call_count.value, 2) self.assertEqual(output_2_call_count.value, 0) - self.assertEqual( - self.driver.execute_script( - 'return window.store.getState().requestQueue' - ), - [] - ) + self.request_queue_assertions(2) assert_clean_console(self) @@ -1533,3 +1531,9 @@ def update_output(n_clicks): self.driver.find_element_by_id('output').text, '2' ) + request_queue = self.driver.execute_script( + 'return window.store.getState().requestQueue' + ) + self.assertFalse(request_queue[0]['rejected']) + self.assertTrue(request_queue[1]['rejected']) + self.assertFalse(request_queue[2]['rejected']) From dadbcbfbb421d2e35a2ebbddea2f4ce278f29585 Mon Sep 17 00:00:00 2001 From: chriddyp Date: Thu, 28 Sep 2017 19:28:56 -0400 Subject: [PATCH 13/13] v0.11.0 --- CHANGELOG.md | 4 ++-- dash_renderer/__init__.py | 3 +-- dash_renderer/version.py | 2 +- package.json | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f801ecc..fe2eea5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,11 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). -## [0.11.0rc1] - 2017-09-27 +## [0.11.0rc1] - 2017-09-28 ### Fixed - 🐞 Previously, old requests could override new requests if their response was longer than the new one. This caused subtle bugs when apps are deployed on multiple processes or threads with component -callbacks that update at varying rates like urls. Originally reported in github.com/plotly/dash/issues/133 +callbacks that update at varying rates like urls. Originally reported in github.com/plotly/dash/issues/133. This fix should also improve performance when many updates happen at once as outdated requests will get dropped instead of updating the UI. ## [0.9.0] - 2017-09-07 diff --git a/dash_renderer/__init__.py b/dash_renderer/__init__.py index 304c720..d9005d0 100644 --- a/dash_renderer/__init__.py +++ b/dash_renderer/__init__.py @@ -7,8 +7,7 @@ # command in the dash_html_components package which printed out: # `dash_html_components.__init__: module references __file__` # TODO - Understand this better -# from .version import __version__ -__version__ = '0.11.0-rc5' +from .version import __version__ __file__ # Dash renderer's dependencies get loaded in a special order by the server: diff --git a/dash_renderer/version.py b/dash_renderer/version.py index d45e7c0..f323a57 100644 --- a/dash_renderer/version.py +++ b/dash_renderer/version.py @@ -1 +1 @@ -__version__ = '0.11.0rc5' +__version__ = '0.11.0' diff --git a/package.json b/package.json index 136e679..437f816 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dash-renderer", - "version": "0.11.0-rc5", + "version": "0.11.0", "description": "render dash components in react", "main": "src/index.js", "scripts": {