Repository: vigetlabs/microcosm
Pull request #352 (2017-06-23 21:56:17)

Add types for compare tree
This commit adds flow type coverage for CompareTree, Node, and Query

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

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

```diff
@@          Coverage Diff          @@
##           master   #352   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          25     25           
  Lines         866    867    +1     
=====================================
+ Hits          866    867    +1
```


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

------

[Continue to review full report at Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/352?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/352?src=pr&el=footer). Last update [542566a...5868682](https://codecov.io/gh/vigetlabs/microcosm/pull/352?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 6bd6756f..cd700e62 100644
--- a/src/compare-tree/index.js
+++ b/src/compare-tree/index.js
@@ -1,28 +1,31 @@
+/**
+ * @flow
+ * @private
+ */
+
 import Node from './node'
 import Query from './query'
-import { getKeyString } from '../key-path'
+import { getKeyString, type KeyPath } 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.
 const ROOT_PATH = ''
 
 class CompareTree {
-  /**
-   * @param {*} initial Starting state
-   */
-  constructor(initial) {
+  snapshot: Object
+  nodes: { [key: string]: Node }
+  queries: { [key: string]: Query }
+
+  constructor(initial: Object) {
     this.snapshot = initial
     this.nodes = {}
+    this.queries = {}
   }
 
   /**
    * Create a subscription to a particular set of key paths.
-   * @public
-   * @param {String} keyPaths A list of key paths to trigger the callback
-   * @param {Function} callback Function to execute when a key path changes
-   * @param {*} [scope] Optional scope to invoke the function with
    */
-  on(keyPaths, callback, scope) {
+  on(keyPaths: Array<KeyPath>, callback: Function, scope?: Object) {
     let query = this.addQuery(keyPaths)
 
     query.on('change', callback, scope)
@@ -34,15 +37,11 @@ class CompareTree {
 
   /**
    * Remove a subscription created by .on()
-   * @public
-   * @param {String} keyPaths A list of key paths for the subscription
-   * @param {Function} callback Associated callback function
-   * @param {*} [scope] Associated scope
    */
-  off(keyPaths, callback, scope) {
+  off(keyPaths: string | Array<KeyPath>, callback: Function, scope?: Object) {
     let id = Query.getId(keyPaths)
 
-    let query = this.nodes[id]
+    let query: Query = this.queries[id]
 
     if (query) {
       query.off('change', callback, scope)
@@ -56,10 +55,8 @@ class CompareTree {
   /**
    * Compare a new snapshot to the last snapshot, triggering event
    * subscriptions along the way.
-   * @public
-   * @param {*} state - New snapshot of state
    */
-  update(snapshot) {
+  update(snapshot: Object) {
     let last = this.snapshot
     this.snapshot = snapshot
 
@@ -76,12 +73,8 @@ class CompareTree {
 
   /**
    * Add a node to the tree if it has not otherwise been added.
-   * @private
-   * @param {String} id Identifier for the node.
-   * @param {String} key Each node represents a key in a nested object.
-   * @param {Node} parent Parent to connect this node to.
    */
-  addNode(key, parent) {
+  addNode(key: string, parent: ?Node) {
     let id = Node.getId(key, parent)
 
     if (!this.nodes[id]) {
@@ -95,36 +88,22 @@ class CompareTree {
    * Add a query to the tree if it has not otherwise been
    * added. Queries are leaf nodes responsible for managing
    * subscriptions.
-   * @private
-   * @param {String} id Identifier for the node.
-   * @param {String} keyPaths Each query tracks a list of key paths
    */
-  addQuery(dependencies) {
+  addQuery(dependencies: string | Array<KeyPath>): Query {
     let id = Query.getId(dependencies)
 
-    if (!this.nodes[id]) {
-      this.nodes[id] = new Query(id, dependencies)
+    if (!this.queries[id]) {
+      this.queries[id] = new Query(id, dependencies)
     }
 
-    return this.nodes[id]
-  }
-
-  /**
-   * Remove a node from this tree.
-   * @private
-   * @param {Node} node Node to remove
-   */
-  remove(node) {
-    delete this.nodes[node.id]
+    return this.queries[id]
   }
 
   /**
    * Remove a query, then traverse that queries key paths to remove
    * unused parents.
-   * @private
-   * @param {Query} query Query to remove
    */
-  prune(query) {
+  prune(query: Query) {
     let ids = query.keyPaths.map(getKeyString)
 
     for (var i = 0, len = ids.length; i < len; i++) {
@@ -135,7 +114,7 @@ class CompareTree {
       do {
         if (node.isAlone()) {
           node.orphan()
-          this.remove(node)
+          delete this.nodes[node.id]
         } else {
           break
         }
@@ -144,17 +123,14 @@ class CompareTree {
       } while (node)
     }
 
-    this.remove(query)
+    delete this.queries[query.id]
   }
 
   /**
    * Build up a branch of nodes given a path of keys, appending a query
    * to the end.
-   * @private
-   * @param {String} path A list of keys
-   * @param {Query} query Query to append to the end of the branch
    */
-  addBranch(path, query) {
+  addBranch(path: KeyPath, query: Query) {
     let last = this.addNode(ROOT_PATH, null)
 
     for (var i = 0, len = path.length; i < len; i++) {
@@ -166,12 +142,8 @@ class CompareTree {
 
   /**
    * Traverse the tree of subscriptions, triggering queries along the way
-   * @private
-   * @param {Node} root Starting point to scan
-   * @param {*} from Starting snapshot
-   * @param {*} from Next snapshot
    */
-  scan(node, last, next, queries) {
+  scan(node: Node, last: *, next: *, queries: Array<Query>) {
     if (last !== next) {
       var edges = node.edges
 
@@ -180,7 +152,7 @@ class CompareTree {
 
         if (edge instanceof Query && queries.indexOf(edge) < 0) {
           queries.push(edge)
-        } else {
+        } else if (edge instanceof Node) {
           var edgeLast = last == null ? last : last[edge.key]
           var edgeNext = next == null ? next : next[edge.key]
 
diff --git a/src/compare-tree/node.js b/src/compare-tree/node.js
index a38f1ab6..1595704c 100644
--- a/src/compare-tree/node.js
+++ b/src/compare-tree/node.js
@@ -1,17 +1,22 @@
 /**
  * @fileoverview A node in a CompareTree represents a single key with
  * a JavaScript object.
+ * @flow
  */
+
+import type Query from './query'
+
 class Node {
+  id: string
+  key: string
+  parent: ?Node
+  edges: Array<Node | Query>
+
   static getId(key, parent) {
     return parent && parent.id ? parent.id + '.' + key : key
   }
 
-  /**
-   * @param {String} id Identifier for the node.
-   * @param {String} key String name of the key this node represents
-   */
-  constructor(id, key, parent) {
+  constructor(id: string, key: string, parent: ?Node) {
     this.id = id
     this.key = key
     this.edges = []
@@ -25,10 +30,8 @@ class Node {
   /**
    * Connect another node to this instance by adding it to
    * the list of edges.
-   * @public
-   * @param {Node} node
    */
-  connect(node) {
+  connect(node: Node | Query) {
     if (node !== this && this.edges.indexOf(node) < 0) {
       this.edges.push(node)
     }
@@ -36,10 +39,8 @@ class Node {
 
   /**
    * Remove a node this instances list of edges.
-   * @public
-   * @param {Node} node
    */
-  disconnect(node) {
+  disconnect(node: Node | Query) {
     let index = this.edges.indexOf(node)
 
     if (~index) {
@@ -50,7 +51,7 @@ class Node {
   /**
    * Does a node have any edges?
    */
-  isAlone() {
+  isAlone(): boolean {
     return this.edges.length <= 0
   }
 
diff --git a/src/compare-tree/query.js b/src/compare-tree/query.js
index 50f6776b..20645bfc 100644
--- a/src/compare-tree/query.js
+++ b/src/compare-tree/query.js
@@ -1,24 +1,29 @@
-import Emitter from '../emitter'
-import { get } from '../utils'
-import { getKeyPaths, getKeyStrings } from '../key-path'
-
 /**
  * @fileoverview Leaf nodes of the comparison tree are queries. A
  * grouping of data subscriptions.
+ * @flow
  */
+
+import Emitter from '../emitter'
+import { get } from '../utils'
+import { getKeyPaths, getKeyStrings, type KeyPath } from '../key-path'
+
 class Query extends Emitter {
-  static getId(keyPaths) {
+  static getId(keyPaths: string | Array<KeyPath>) {
     return 'query:' + getKeyStrings(getKeyPaths(keyPaths))
   }
 
-  constructor(id, keys) {
+  id: string
+  keyPaths: Array<KeyPath>
+
+  constructor(id: string, keys: string | Array<KeyPath>) {
     super()
 
     this.id = id
     this.keyPaths = getKeyPaths(keys)
   }
 
-  forEachPath(callback, scope) {
+  forEachPath(callback: Function, scope?: *) {
     let paths = this.keyPaths
 
     for (var i = 0, len = paths.length; i < len; i++) {
@@ -26,7 +31,7 @@ class Query extends Emitter {
     }
   }
 
-  extract(state) {
+  extract(state: Object) {
     let length = this.keyPaths.length
     let values = Array(length)
 
@@ -37,7 +42,7 @@ class Query extends Emitter {
     return values
   }
 
-  trigger(state) {
+  trigger(state: Object) {
     let values = this.extract(state)
 
     this._emit('change', ...values)
diff --git a/src/emitter.js b/src/emitter.js
index 330e5259..e305af5b 100644
--- a/src/emitter.js
+++ b/src/emitter.js
@@ -1,21 +1,21 @@
 /**
  * @fileoverview Emitter is an abstract class used by a few other
  * classes to communicate via events
+ * @flow
  */
 
 import { isFunction } from './utils'
 
 class Listener {
-  /**
-   * @param {string} event Type of event
-   * @param {Function} fn Event callback
-   * @param {*} scope Scope of callback
-   * @param {boolean} once Only run the event callback once
-   */
-  constructor(event, fn, scope, once) {
+  event: string
+  fn: Function
+  scope: ?Object
+  once: boolean
+
+  constructor(event: string, fn: Function, scope: ?Object, once: boolean) {
     console.assert(
       isFunction(fn),
-      `Expected ${event} listener to be function, instead got ${fn}`
+      `Expected ${event} listener to be function, instead got ${typeof fn}`
     )
 
     this.event = event
@@ -31,17 +31,16 @@ class Listener {
  * @property {Array.<Listener>} _events A pool of event listeners
  */
 class Emitter {
+  _events: Array<Listener>
+
   constructor() {
     this._events = []
   }
 
   /**
    * Add an event listener.
-   * @param {string} event Type of event
-   * @param {Function} fn Event callback
-   * @param {*} [scope] Optional scope to invoke callback with
    */
-  on(event, fn, scope) {
+  on(event: string, fn: Function, scope?: Object) {
     let listener = new Listener(event, fn, scope, false)
 
     this._events.push(listener)
@@ -52,11 +51,8 @@ class Emitter {
   /**
    * Adds an `event` listener that will be invoked a single time then
    * automatically removed.
-   * @param {string} event Type of event
-   * @param {Function} fn Event callback
-   * @param {*} [scope] Optional scope to invoke callback with
    */
-  once(event, fn, scope) {
+  once(event: string, fn: Function, scope?: Object) {
     let listener = new Listener(event, fn, scope, true)
 
     this._events.push(listener)
@@ -67,11 +63,8 @@ class Emitter {
   /**
    * Unsubscribe a callback. If no event is provided, removes all callbacks. If
    * no callback is provided, removes all callbacks for the given type.
-   * @param {string} event Type of event
-   * @param {Function} fn Event callback
-   * @param {*} [scope] Optional scope to invoke callback with
    */
-  off(event, fn, scope) {
+  off(event: string, fn: Function, scope?: Object) {
     var removeAll = fn == null
 
     let i = 0
@@ -103,7 +96,7 @@ class Emitter {
    * @param {string} event Type of event
    * @param {*} payload Value to send with callback
    */
-  _emit(event, ...args) {
+  _emit(event: string, ...args: Array<*>) {
     let i = 0
     while (i < this._events.length) {
       var cb = this._events[i]
@@ -126,7 +119,7 @@ class Emitter {
   /**
    * Remove all events for a given scope
    */
-  _removeScope(scope) {
+  _removeScope(scope: Object) {
     let i = 0
     while (i < this._events.length) {
       var cb = this._events[i]


Test patch:
diff --git a/test/unit/compare-tree/compare-tree.test.js b/test/unit/compare-tree/compare-tree.test.js
index 56f80233..e58488d4 100644
--- a/test/unit/compare-tree/compare-tree.test.js
+++ b/test/unit/compare-tree/compare-tree.test.js
@@ -133,7 +133,7 @@ describe('CompareTree', function() {
 
       tree.off('meta', handler)
 
-      expect(tree.nodes[query.id]).toBeUndefined()
+      expect(query.id in tree.queries).toBe(false)
     })
 
     it('removes nodes without edges up the chain', function() {
@@ -142,9 +142,9 @@ describe('CompareTree', function() {
 
       tree.off('meta.selected', handler)
 
-      expect(tree.nodes[query.id]).toBeUndefined()
       expect(tree.nodes['meta.selected']).toBeUndefined()
       expect(tree.nodes['meta']).toBeUndefined()
+      expect(tree.queries[query.id]).toBeUndefined()
     })
 
     it('does not remove parents with other compares', function() {
@@ -154,10 +154,10 @@ describe('CompareTree', function() {
       tree.on('meta.focused', handler)
       tree.off('meta.selected', handler)
 
-      expect(tree.nodes[query.id]).toBeUndefined()
       expect(tree.nodes['meta.selected']).toBeUndefined()
       expect(tree.nodes['meta.focused']).toBeDefined()
       expect(tree.nodes['meta']).toBeDefined()
+      expect(tree.queries[query.id]).toBeUndefined()
     })
 
     it('removes the root node when there are no subscriptions', function() {
@@ -165,12 +165,11 @@ describe('CompareTree', function() {
 
       tree.on('meta.selected', handler)
 
-      // 'root', 'meta', 'selected', 'handler'
-      expect(Object.keys(tree.nodes)).toHaveLength(4)
+      expect('' in tree.nodes).toBe(true)
 
       tree.off('meta.selected', handler)
 
-      expect(Object.keys(tree.nodes)).toHaveLength(0)
+      expect('' in tree.nodes).toBe(false)
     })
 
     it('keeps a query if it still has compares left', function() {
@@ -189,7 +188,7 @@ describe('CompareTree', function() {
       expect(one).not.toHaveBeenCalled()
       expect(two).toHaveBeenCalled()
 
-      expect(tree.nodes[query.id]).toBeDefined()
+      expect(query.id in tree.queries).toBe(true)
     })
 
     it('gracefully handles parents missing an edge (which should never happen)', function() {
@@ -205,24 +204,22 @@ describe('CompareTree', function() {
 
       tree.off('a.b.c', one)
 
-      let keys = Object.keys(tree.nodes)
-
-      expect(keys).toContain('a')
-      expect(keys).toContain('a.b')
-      expect(keys).toContain('query:a.b')
-      expect(keys).not.toContain('query:a.b.c')
+      expect('a' in tree.nodes).toBe(true)
+      expect('a.b' in tree.nodes).toBe(true)
+      expect('query:a.b' in tree.queries).toBe(true)
+      expect('query:a.b.c' in tree.queries).not.toBe(true)
     })
 
-    it('gracefully handles compares that do not exist', function() {
-      tree.on('a.b.c', n => n)
-      tree.off('a.b.c', n => n)
+    it('gracefully handles nodes that do not exist', function() {
+      let callback = n => n
 
-      let keys = Object.keys(tree.nodes)
+      tree.on('a.b.c', callback)
+      tree.off('a.b.c', callback)
 
-      expect(keys).toContain('a')
-      expect(keys).toContain('a.b')
-      expect(keys).toContain('a.b.c')
-      expect(keys).toContain('query:a.b.c')
+      expect('a' in tree.nodes).toBe(false)
+      expect('a.b' in tree.nodes).toBe(false)
+      expect('a.b.c' in tree.nodes).toBe(false)
+      expect('query:a.b.c' in tree.queries).toBe(false)
     })
   })
 
@@ -278,7 +275,7 @@ describe('CompareTree', function() {
 
       expect(tree.nodes['meta']).toBeDefined()
       expect(tree.nodes['meta.selected']).toBeDefined()
-      expect(tree.nodes['query:meta.selected']).toBeDefined()
+      expect(tree.queries['query:meta.selected']).toBeDefined()
     })
 
     it('allows a simple, comma separated string', function() {
@@ -287,7 +284,7 @@ describe('CompareTree', function() {
       expect(tree.nodes['meta']).toBeDefined()
       expect(tree.nodes['meta.selected']).toBeDefined()
       expect(tree.nodes['meta.focused']).toBeDefined()
-      expect(tree.nodes['query:meta.selected,meta.focused']).toBeDefined()
+      expect(tree.queries['query:meta.selected,meta.focused']).toBeDefined()
     })
 
     it('allows arrays of paths', function() {
@@ -295,18 +292,16 @@ describe('CompareTree', function() {
 
       expect(tree.nodes['meta']).toBeDefined()
       expect(tree.nodes['meta.selected']).toBeDefined()
-      expect(tree.nodes['query:meta.selected']).toBeDefined()
+      expect(tree.queries['query:meta.selected']).toBeDefined()
     })
 
     it('allows arrays of strings', function() {
       tree.on(['meta.selected', 'planets'], jest.fn())
 
-      let keys = Object.keys(tree.nodes)
-
-      expect(keys).toContain('planets')
-      expect(keys).toContain('meta')
-      expect(keys).toContain('meta.selected')
-      expect(keys).toContain('query:meta.selected,planets')
+      expect(tree.nodes['planets']).toBeDefined()
+      expect(tree.nodes['meta']).toBeDefined()
+      expect(tree.nodes['meta.selected']).toBeDefined()
+      expect(tree.queries['query:meta.selected,planets']).toBeDefined()
     })
   })
 })