fix: Fix bug resulting from single queue in Dispatch (Famous#365)

alexanderGugel · Jul 7, 2015 · 2a8341e · 2a8341e
1 parent f2e7318
commit 2a8341e
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 43 deletions.
diff --git a/core/Dispatch.js b/core/Dispatch.js
@@ -38,7 +38,7 @@ var PathUtils = require('./Path');
 function Dispatch () {
     this._nodes = {}; // a container for constant time lookup of nodes
 
-    this._queue = []; // The queue is used for two purposes
+                      // The queue is used for two purposes
                       // 1. It is used to list indicies in the
                       //    Nodes path which are then used to lookup
                       //    a node in the scene graph.
@@ -73,24 +73,14 @@ Dispatch.prototype._setUpdater = function _setUpdater (updater) {
  *
  * @param {Node} node from which to add children to the queue
  */
-Dispatch.prototype.addChildrenToQueue = function addChildrenToQueue (node) {
+function addChildrenToQueue (node, queue) {
     var children = node.getChildren();
     var child;
     for (var i = 0, len = children.length ; i < len ; i++) {
         child = children[i];
-        if (child) this._queue.push(child);
+        if (child) queue.push(child);
     }
-};
-
-/**
- * Returns the next item in the Dispatch's queue.
- *
- * @method next
- * @return {Node} next node in the queue
- */
-Dispatch.prototype.next = function next () {
-    return this._queue.shift();
-};
+}
 
 /**
  * Returns the next node in the queue, but also adds its children to
@@ -100,12 +90,12 @@ Dispatch.prototype.next = function next () {
  * @method breadthFirstNext
  * @return {Node | undefined} the next node in the traversal if one exists
  */
-Dispatch.prototype.breadthFirstNext = function breadthFirstNext () {
-    var child = this._queue.shift();
-    if (!child) return void 0; 
-    this.addChildrenToQueue(child);
+function breadthFirstNext (queue) {
+    var child = queue.shift();
+    if (!child) return void 0;
+    addChildrenToQueue(child, queue);
     return child;
-};
+}
 
 /**
  * Calls the onMount method for the node at a given path and
@@ -249,12 +239,13 @@ Dispatch.prototype.show = function show (path) {
 
     if (node.onShow) node.onShow();
 
-    this.addChildrenToQueue(node);
+    var queue = [];
+
+    addChildrenToQueue(node, queue);
     var child;
 
-    while ((child = this.breadthFirstNext()))
+    while ((child = breadthFirstNext(queue)))
         this.show(child.getLocation());
-
 };
 
 /**
@@ -277,10 +268,12 @@ Dispatch.prototype.hide = function hide (path) {
 
     if (node.onHide) node.onHide();
 
-    this.addChildrenToQueue(node);
+    var queue = [];
+
+    addChildrenToQueue(node, queue);
     var child;
 
-    while ((child = this.breadthFirstNext()))
+    while ((child = breadthFirstNext(queue)))
         this.hide(child.getLocation());
 
 };
@@ -296,8 +289,7 @@ Dispatch.prototype.hide = function hide (path) {
 Dispatch.prototype.lookupNode = function lookupNode (location) {
     if (!location) throw new Error('lookupNode must be called with a path');
 
-    this._queue.length = 0;
-    var path = this._queue;
+    var path = [];
 
     _splitTo(location, path);
 
@@ -327,10 +319,12 @@ Dispatch.prototype.dispatch = function dispatch (path, event, payload) {
     if (!node)
         throw new Error('No node registered at path: ' + path);
 
-    this.addChildrenToQueue(node);
+    var queue = [];
+    addChildrenToQueue(node, queue);
+
     var child;
 
-    while ((child = this.breadthFirstNext()))
+    while ((child = breadthFirstNext(queue)))
         if (child.onReceive)
             child.onReceive(event, payload);
 

diff --git a/core/test/dispatch/Dispatch.spec.js b/core/test/dispatch/Dispatch.spec.js
@@ -39,7 +39,7 @@ test('Dispatch singleton', function (t) {
         var testUpdater = 'a';
 
         t.doesNotThrow(
-            Dispatch._setUpdater.bind(Dispatch, testUpdater), 
+            Dispatch._setUpdater.bind(Dispatch, testUpdater),
             '._setUpdater should be callable'
         );
 
@@ -60,10 +60,10 @@ test('Dispatch singleton', function (t) {
         Dispatch.mount('body/0', stub2);
 
         t.notOk(stub2._setUpdater.getCall(0).calledWith(testUpdater), 'Nodes mounted with the Dispatch ' +
-                                                                      'should have their updaters set to ' + 
+                                                                      'should have their updaters set to ' +
                                                                       'the dispatch\'s current updater and not a previous one');
 
-        t.ok(stub2._setUpdater.getCall(0).calledWith(testUpdater2), 'Nodes mounted with the Dispatch ' + 
+        t.ok(stub2._setUpdater.getCall(0).calledWith(testUpdater2), 'Nodes mounted with the Dispatch ' +
                                                                     'should have their updaters set to ' +
                                                                     'the dispatch\'s current updater');
 
@@ -74,18 +74,6 @@ test('Dispatch singleton', function (t) {
         t.end();
     });
 
-    t.test('.addChildrenToQueue method', function (t) {
-        t.end();
-    });
-
-    t.test('.next method', function (t) {
-        t.end();
-    });
-
-    t.test('.breadthFirstNext method', function (t) {
-        t.end();
-    });
-
     t.test('.mount method', function (t) {
         t.end();
     });