fix tree traversal with branched lineages

CHANGELOG.md

@@ -2,6 +2,15 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## [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
+
+### Added
+- 🐌 Experimental behaviour for a customizable "loading state". When a callback is in motion, Dash now appends a `<div class="_dash-loading-callback"/>` to the DOM.
+Users can style this element using custom CSS to display loading screen overlays.
+This feature is in alpha, we may remove it at any time.
+
 ## [0.8.0] - 2017-09-07
 ### Added
 - 🔧 Added support for the `requests_pathname_prefix` config parameter introduced in `dash==0.18.0`

dash_renderer/version.py

@@ -1 +1 @@
-__version__ = '0.8.0'
+__version__ = '0.9.0'

package.json

@@ -1,6 +1,6 @@
 {
   "name": "dash-renderer",
-  "version": "0.8.0",
+  "version": "0.9.0",
   "description": "render dash components in react",
   "main": "src/index.js",
   "scripts": {

src/AppContainer.react.js

@@ -3,6 +3,7 @@ import React from 'react';
 import Authentication from './Authentication.react';
 import APIController from './APIController.react';
 import DocumentTitle from './components/core/DocumentTitle.react';
+import Loading from './components/core/Loading.react';
 import Toolbar from './components/core/Toolbar.react';
 
 function UnconnectedAppContainer() {
@@ -12,6 +13,7 @@ function UnconnectedAppContainer() {
                 <Toolbar/>
                 <APIController/>
                 <DocumentTitle/>
+                <Loading/>
             </div>
         </Authentication>
     );

src/actions/index.js

@@ -136,6 +136,7 @@ export function notifyObservers(payload) {
         const {EventGraph, InputGraph} = graphs;
 
         /*
+
          * Figure out all of the output id's that depend on this
          * event or input.
          * This includes id's that are direct children as well as
@@ -176,10 +177,13 @@ export function notifyObservers(payload) {
         const queuedObservers = [];
         outputObservers.forEach(function filterObservers(outputIdAndProp) {
             const outputComponentId = outputIdAndProp.split('.')[0];
+
             /*
-             * before we make the POST, check that none of its input
-             * dependencies are already in the queue.
-             * if they are in the queue, then don't update.
+             * before we make the POST to update the output, check
+             * that the output doesn't depend on any other inputs that
+             * that depend on the same controller.
+             * if the output has another input with a shared controller,
+             * then don't update this output yet.
              * when each dependency updates, it'll dispatch its own
              * `notifyObservers` action which will allow this
              * component to update.
@@ -193,15 +197,34 @@ export function notifyObservers(payload) {
              * this loop hits C because of the overallOrder sorting logic
              */
 
-            const controllersInQueue = intersection(
+
+             /*
+              * if the output just listens to events, then it won't be in
+              * the InputGraph
+              */
+            const controllers = (InputGraph.hasNode(outputIdAndProp) ?
+                InputGraph.dependantsOf(outputIdAndProp) : []);
+
+            const controllersInFutureQueue = intersection(
                 queuedObservers,
+                controllers
+            );
 
-                /*
-                 * if the output just listens to events, then it won't be in
-                 * the InputGraph
-                 */
-                InputGraph.hasNode(outputIdAndProp) ?
-                InputGraph.dependantsOf(outputIdAndProp) : []
+            /*
+             * check that the output hasn't been triggered to update already
+             * by a different input.
+             *
+             * for example:
+             * Grandparent -> [Parent A, Parent B] -> Child
+             *
+             * when Grandparent changes, it will trigger Parent A and Parent B
+             * to each update Child.
+             * one of the components (Parent A or Parent B) will queue up
+             * 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
             );
 
             /*
@@ -212,18 +235,21 @@ export function notifyObservers(payload) {
              * for example, perhaps the user has hidden one of the observers
              */
              if (
-                 (controllersInQueue.length === 0) &&
-                 (has(outputComponentId, getState().paths))
+                 (controllersInFutureQueue.length === 0) &&
+                 (has(outputComponentId, getState().paths)) &&
+                 (controllersInExistingQueue.length === 0)
              ) {
                  queuedObservers.push(outputIdAndProp)
              }
         });
+
         /*
          * 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
          * action
          */
         dispatch(setRequestQueue(union(queuedObservers, requestQueue)));
+
         const promises = [];
         for (let i = 0; i < queuedObservers.length; i++) {
             const outputIdAndProp = queuedObservers[i];
@@ -300,15 +326,15 @@ export function notifyObservers(payload) {
                     payload: {status: res.status}
                 });
 
-                return res.json().then(function handleJson(data) {
+                // clear this item from the request queue
+                dispatch(setRequestQueue(
+                    reject(
+                        id => id === outputIdAndProp,
+                        getState().requestQueue
+                    )
+                ));
 
-                    // clear this item from the request queue
-                    dispatch(setRequestQueue(
-                        reject(
-                            id => id === outputIdAndProp,
-                            getState().requestQueue
-                        )
-                    ));
+                return res.json().then(function handleJson(data) {
 
                     /*
                      * it's possible that this output item is no longer visible.

src/components/core/DocumentTitle.react.js

@@ -1,7 +1,7 @@
 /* global document:true */
 
-import { connect } from 'react-redux'
-import { isEmpty } from 'ramda'
+import {connect} from 'react-redux'
+import {isEmpty} from 'ramda'
 import {Component, PropTypes} from 'react'
 
 class DocumentTitle extends Component {

src/components/core/Loading.react.js

@@ -0,0 +1,23 @@
+import {connect} from 'react-redux'
+import {isEmpty} from 'ramda'
+import React, {PropTypes} from 'react'
+
+function Loading(props) {
+    if (!isEmpty(props.requestQueue)) {
+        return (
+            <div className="_dash-loading-callback"/>
+        )
+    } else {
+        return null;
+    }
+}
+
+Loading.propTypes = {
+    requestQueue: PropTypes.array.required
+}
+
+export default connect(
+    state => ({
+        requestQueue: state.requestQueue
+    })
+)(Loading);

src/components/core/NotifyObservers.react.js

@@ -100,6 +100,7 @@ function NotifyObserversComponent ({
     if (thisComponentTriggersEvents && paths[id]) {
         extraProps.fireEvent = fireEvent;
     }
+
     if (!isEmpty(extraProps)) {
         return React.cloneElement(children, extraProps);
     } else {

tests/test_render.py

@@ -391,8 +391,7 @@ def test_initial_state(self):
             []
         )
 
-        # Take a screenshot with percy
-        # self.percy_runner.snapshot(name='dcc')
+        self.percy_runner.snapshot(name='layout')
 
         assert_clean_console(self)
 
@@ -424,6 +423,7 @@ def update_output(value):
 
         output1 = self.wait_for_element_by_id('output-1')
         wait_for(lambda: output1.text == 'initial value')
+        self.percy_runner.snapshot(name='simple-callback-1')
 
         input1 = self.wait_for_element_by_id('input')
         input1.clear()
@@ -432,6 +432,7 @@ def update_output(value):
 
         output1 = lambda: self.wait_for_element_by_id('output-1')
         wait_for(lambda: output1().text == 'hello world')
+        self.percy_runner.snapshot(name='simple-callback-2')
 
         self.assertEqual(
             call_count.value,
@@ -509,6 +510,7 @@ def update_input(value):
                 </div>'''.replace('\n', '').replace('  ', '')
             )
         )
+        self.percy_runner.snapshot(name='callback-generating-function-1')
 
         # the paths should include these new output IDs
         self.assertEqual(
@@ -548,6 +550,7 @@ def update_input(value):
             ),
             []
         )
+        self.percy_runner.snapshot(name='callback-generating-function-2')
 
         assert_clean_console(self)
 
@@ -763,6 +766,7 @@ def chapter1_assertions():
             generic_chapter_assertions('chapter1')
 
         chapter1_assertions()
+        self.percy_runner.snapshot(name='chapter-1')
 
         # switch chapters
         (self.driver.find_elements_by_css_selector(
@@ -771,6 +775,7 @@ def chapter1_assertions():
 
         # sleep just to make sure that no calls happen after our check
         time.sleep(2)
+        self.percy_runner.snapshot(name='chapter-2')
         wait_for(lambda: call_counts['body'].value == 2)
         wait_for(lambda: call_counts['chapter2-graph'].value == 1)
         wait_for(lambda: call_counts['chapter2-label'].value == 1)
@@ -815,6 +820,7 @@ def chapter2_assertions():
         )[2]).click()
         # sleep just to make sure that no calls happen after our check
         time.sleep(2)
+        self.percy_runner.snapshot(name='chapter-3')
         wait_for(lambda: call_counts['body'].value == 3)
         wait_for(lambda: call_counts['chapter3-graph'].value == 1)
         wait_for(lambda: call_counts['chapter3-label'].value == 1)
@@ -867,6 +873,8 @@ def chapter3_assertions():
             self.driver.find_element_by_id('body').text ==
             'Just a string'
         ))
+        self.percy_runner.snapshot(name='chapter-4')
+
         # each element should exist in the dom
         paths = self.driver.execute_script(
             'return window.store.getState().paths'
@@ -884,6 +892,7 @@ def chapter3_assertions():
         )[0]).click()
         time.sleep(0.5)
         chapter1_assertions()
+        self.percy_runner.snapshot(name='chapter-1-again')
 
         # switch to 5
         (self.driver.find_elements_by_css_selector(
@@ -901,6 +910,7 @@ def chapter3_assertions():
         chapter5_button().click()
         wait_for(lambda: chapter5_div().text == chapter5_output_children)
         time.sleep(0.5)
+        self.percy_runner.snapshot(name='chapter-5')
         self.assertEqual(call_counts['chapter5-output'].value, 1)
 
     def test_dependencies_on_components_that_dont_exist(self):
@@ -935,6 +945,7 @@ def update_output_2(value):
 
         el = self.wait_for_element_by_id('output-1')
         wait_for(lambda: el.text == 'initial value')
+        self.percy_runner.snapshot(name='dependencies')
         time.sleep(1.0)
         self.assertEqual(output_1_call_count.value, 1)
         self.assertEqual(output_2_call_count.value, 0)
@@ -1178,8 +1189,7 @@ def update_input(value):
         self.assertEqual(call_counts[ids['button-output']].value, 1)
         self.assertEqual(output().text, 'Input is equal to "initial state"')
 
-
-    def test_chained_dependencies(self):
+    def test_chained_dependencies_direct_lineage(self):
         app = Dash(__name__)
         app.layout = html.Div([
             dcc.Input(id='input-1', value='input 1'),
@@ -1230,6 +1240,74 @@ def update_output(input1, input2):
         self.assertEqual(input2().get_attribute('value'), '<<input 1x>>y')
         self.assertEqual(output().text, 'input 1x + <<input 1x>>y')
 
+
+    def test_chained_dependencies_branched_lineage(self):
+        app = Dash(__name__)
+        app.layout = html.Div([
+            dcc.Input(id='grandparent', value='input 1'),
+            dcc.Input(id='parent-a'),
+            dcc.Input(id='parent-b'),
+            html.Div(id='child-a'),
+            html.Div(id='child-b')
+        ])
+        grandparent = lambda: self.driver.find_element_by_id('grandparent')
+        parenta = lambda: self.driver.find_element_by_id('parent-a')
+        parentb = lambda: self.driver.find_element_by_id('parent-b')
+        childa = lambda: self.driver.find_element_by_id('child-a')
+        childb = lambda: self.driver.find_element_by_id('child-b')
+
+        call_counts = {
+            'parent-a': Value('i', 0),
+            'parent-b': Value('i', 0),
+            'child-a': Value('i', 0),
+            'child-b': Value('i', 0)
+        }
+
+        @app.callback(Output('parent-a', 'value'),
+                      [Input('grandparent', 'value')])
+        def update_parenta(value):
+            call_counts['parent-a'].value += 1
+            return 'a: {}'.format(value)
+
+        @app.callback(Output('parent-b', 'value'),
+                      [Input('grandparent', 'value')])
+        def update_parentb(value):
+            time.sleep(0.5)
+            call_counts['parent-b'].value += 1
+            return 'b: {}'.format(value)
+
+        @app.callback(Output('child-a', 'children'),
+                      [Input('parent-a', 'value'),
+                       Input('parent-b', 'value')])
+        def update_childa(parenta_value, parentb_value):
+            time.sleep(1)
+            call_counts['child-a'].value += 1
+            return '{} + {}'.format(parenta_value, parentb_value)
+
+        @app.callback(Output('child-b', 'children'),
+                      [Input('parent-a', 'value'),
+                       Input('parent-b', 'value'),
+                       Input('grandparent', 'value')])
+        def update_childb(parenta_value, parentb_value, grandparent_value):
+            call_counts['child-b'].value += 1
+            return '{} + {} + {}'.format(
+                parenta_value,
+                parentb_value,
+                grandparent_value
+            )
+
+        self.startServer(app)
+
+        wait_for(lambda: childa().text == 'a: input 1 + b: input 1')
+        wait_for(lambda: childb().text == 'a: input 1 + b: input 1 + input 1')
+        time.sleep(1)  # wait for potential requests of app to settle down
+        self.assertEqual(parenta().get_attribute('value'), 'a: input 1')
+        self.assertEqual(parentb().get_attribute('value'), 'b: input 1')
+        self.assertEqual(call_counts['parent-a'].value, 1)
+        self.assertEqual(call_counts['parent-b'].value, 1)
+        self.assertEqual(call_counts['child-a'].value, 1)
+        self.assertEqual(call_counts['child-b'].value, 1)
+
     def test_removing_component_while_its_getting_updated(self):
         app = Dash(__name__)
         app.layout = html.Div([