Repository: vigetlabs/microcosm
Pull request #342 (2017-06-05 12:14:28)

Move more query/node concerns to their associated modules
Microcosm has an internal tree for comparing data. This is so that you can do things like `repo.on('change:planets', (planets) => {})`. This commit reduces some responsbilities inside of the comparison tree itself so that individual nodes have greater encapsulation.

I've also added an efficiency for calculating keypaths that avoids re-parsing array-form keypaths. Paths that are already in the correct form are not additionally parsed. 

For example:

```javascript
let path = [['foo', 'bar']]
let parsed = castPath(path)

assert.equal(path, parsed)
```

codecov-io: # [Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/342?src=pr&el=h1) Report
> Merging [#342](https://codecov.io/gh/vigetlabs/microcosm/pull/342?src=pr&el=desc) into [master](https://codecov.io/gh/vigetlabs/microcosm/commit/58c7ba367f076c682a2e937cc9d0424c22d19f93?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/vigetlabs/microcosm/pull/342/graphs/tree.svg?token=AaE0FLz9sU&src=pr&width=650&height=150)](https://codecov.io/gh/vigetlabs/microcosm/pull/342?src=pr&el=tree)

```diff
@@          Coverage Diff          @@
##           master   #342   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          24     24           
  Lines         858    861    +3     
=====================================
+ Hits          858    861    +3
```


| [Impacted Files](https://codecov.io/gh/vigetlabs/microcosm/pull/342?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/key-path.js](https://codecov.io/gh/vigetlabs/microcosm/pull/342?src=pr&el=tree#diff-c3JjL2tleS1wYXRoLmpz) | `100% <100%> (ø)` | :arrow_up: |
| [src/compare-tree/index.js](https://codecov.io/gh/vigetlabs/microcosm/pull/342?src=pr&el=tree#diff-c3JjL2NvbXBhcmUtdHJlZS9pbmRleC5qcw==) | `100% <100%> (ø)` | :arrow_up: |
| [src/compare-tree/query.js](https://codecov.io/gh/vigetlabs/microcosm/pull/342?src=pr&el=tree#diff-c3JjL2NvbXBhcmUtdHJlZS9xdWVyeS5qcw==) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/342?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/342?src=pr&el=footer). Last update [58c7ba3...4375be1](https://codecov.io/gh/vigetlabs/microcosm/pull/342?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/compare-tree/index.js b/src/compare-tree/index.js
index bea42f27..6bd6756f 100644
--- a/src/compare-tree/index.js
+++ b/src/compare-tree/index.js
@@ -1,6 +1,6 @@
 import Node from './node'
 import Query from './query'
-import { getKeyPaths, getKeyString } from '../key-path'
+import { getKeyString } from '../key-path'
 
 // The root key is an empty string. This can be a little
 // counter-intuitive, so we keep track of them as a named constant.
@@ -23,17 +23,12 @@ class CompareTree {
    * @param {*} [scope] Optional scope to invoke the function with
    */
   on(keyPaths, callback, scope) {
-    let dependencies = getKeyPaths(keyPaths)
-    let id = Query.getId(keyPaths)
-
-    let query = this.addQuery(id, dependencies)
-
-    for (var i = 0; i < dependencies.length; i++) {
-      this.addBranch(dependencies[i], query)
-    }
+    let query = this.addQuery(keyPaths)
 
     query.on('change', callback, scope)
 
+    query.forEachPath(this.addBranch, this)
+
     return query
   }
 
@@ -104,9 +99,11 @@ class CompareTree {
    * @param {String} id Identifier for the node.
    * @param {String} keyPaths Each query tracks a list of key paths
    */
-  addQuery(id, keyPaths) {
+  addQuery(dependencies) {
+    let id = Query.getId(dependencies)
+
     if (!this.nodes[id]) {
-      this.nodes[id] = new Query(id, keyPaths)
+      this.nodes[id] = new Query(id, dependencies)
     }
 
     return this.nodes[id]
diff --git a/src/compare-tree/query.js b/src/compare-tree/query.js
index 94da58c0..50f6776b 100644
--- a/src/compare-tree/query.js
+++ b/src/compare-tree/query.js
@@ -18,6 +18,14 @@ class Query extends Emitter {
     this.keyPaths = getKeyPaths(keys)
   }
 
+  forEachPath(callback, scope) {
+    let paths = this.keyPaths
+
+    for (var i = 0, len = paths.length; i < len; i++) {
+      callback.call(scope, paths[i], this)
+    }
+  }
+
   extract(state) {
     let length = this.keyPaths.length
     let values = Array(length)
diff --git a/src/key-path.js b/src/key-path.js
index 10530d2e..6e8f58a9 100644
--- a/src/key-path.js
+++ b/src/key-path.js
@@ -42,6 +42,8 @@ export function getKeyPaths(value) {
 
   if (Array.isArray(value) === false) {
     paths = `${paths}`.split(PATH_DELIMETER)
+  } else if (value.every(Array.isArray)) {
+    return value
   }
 
   return paths.map(castPath)


Test patch:
diff --git a/test/unit/compare-tree/query.test.js b/test/unit/compare-tree/query.test.js
new file mode 100644
index 00000000..c9d53d4f
--- /dev/null
+++ b/test/unit/compare-tree/query.test.js
@@ -0,0 +1,16 @@
+import Query from '../../../src/compare-tree/query'
+
+describe('CompareTree > Query', function() {
+  describe('#forEachPath', function() {
+    it('enumerates through the given dependencies', function() {
+      let query = new Query('1', 'foo.bar,bip.baz')
+      let answer = {}
+
+      query.forEachPath(path => {
+        answer[path.join('.')] = true
+      })
+
+      expect(answer).toEqual({ 'foo.bar': true, 'bip.baz': true })
+    })
+  })
+})
diff --git a/test/unit/key-path/get-key-paths.test.js b/test/unit/key-path/get-key-paths.test.js
index b97536d8..5475a518 100644
--- a/test/unit/key-path/get-key-paths.test.js
+++ b/test/unit/key-path/get-key-paths.test.js
@@ -45,4 +45,11 @@ describe('getKeyPaths', function() {
     expect(path[0]).toEqual(['foo'])
     expect(path[1]).toEqual(['b ar'])
   })
+
+  it('returns the same array if every value is an array', function() {
+    let path = [['foo', 'bar'], ['bip', 'baz']]
+    let answer = getKeyPaths(path)
+
+    expect(answer).toBe(path)
+  })
 })