Repository: vigetlabs/microcosm
Pull request #371 (2017-07-21 12:22:47)

History is an iterator
This commit implements the iterator interface for history. This is important for interoperability with other libraries: specifically Observables.

```
// Enumerate over history
for (var action of repo.history) {
  console.log(action)
}

// Or quickly build an array from history
let activeBranch = Array.from(repo.history)

// Or wait for all history to finish
Observable.of(repo.history).subscribe({ next, error, complete })
```

codecov-io: # [Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/371?src=pr&el=h1) Report
> Merging [#371](https://codecov.io/gh/vigetlabs/microcosm/pull/371?src=pr&el=desc) into [master](https://codecov.io/gh/vigetlabs/microcosm/commit/f6831227a1340a773d3f140c7190ac76cf5627b5?src=pr&el=desc) will **decrease** coverage by `0.11%`.
> The diff coverage is `94.73%`.

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

```diff
@@            Coverage Diff             @@
##           master     #371      +/-   ##
==========================================
- Coverage     100%   99.88%   -0.12%     
==========================================
  Files          25       26       +1     
  Lines         854      864      +10     
==========================================
+ Hits          854      863       +9     
- Misses          0        1       +1
```


| [Impacted Files](https://codecov.io/gh/vigetlabs/microcosm/pull/371?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/symbols.js](https://codecov.io/gh/vigetlabs/microcosm/pull/371?src=pr&el=tree#diff-c3JjL3N5bWJvbHMuanM=) | `100% <100%> (ø)` | |
| [src/history.js](https://codecov.io/gh/vigetlabs/microcosm/pull/371?src=pr&el=tree#diff-c3JjL2hpc3RvcnkuanM=) | `100% <100%> (ø)` | :arrow_up: |
| [src/utils.js](https://codecov.io/gh/vigetlabs/microcosm/pull/371?src=pr&el=tree#diff-c3JjL3V0aWxzLmpz) | `98.57% <66.66%> (-1.43%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/371?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/371?src=pr&el=footer). Last update [f683122...40ac747](https://codecov.io/gh/vigetlabs/microcosm/pull/371?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/action.js b/src/action.js
index 32f946bb..1dee76fe 100644
--- a/src/action.js
+++ b/src/action.js
@@ -127,10 +127,10 @@ class Action extends Emitter {
     let outstanding = actions.length
 
     const onResolve = () => {
-      outstanding -= 1
-
-      if (outstanding <= 0) {
+      if (outstanding <= 1) {
         this.resolve()
+      } else {
+        outstanding -= 1
       }
     }
 
@@ -140,6 +140,8 @@ class Action extends Emitter {
       action.onError(this.reject)
     })
 
+    onResolve()
+
     return this
   }
 
@@ -270,6 +272,10 @@ class Action extends Emitter {
     return this
   }
 
+  toString() {
+    return this.command.toString()
+  }
+
   toJSON() {
     return {
       id: this.id,
diff --git a/src/archive.js b/src/archive.js
index 35ff9032..8af5199b 100644
--- a/src/archive.js
+++ b/src/archive.js
@@ -25,7 +25,7 @@ class Archive {
    * Access a prior snapshot for a given action
    */
   get(action: ?Action, fallback: ?Object): Object {
-    console.assert(action, 'Unable to get ' + typeof action + ' action')
+    console.assert(action, 'Unable to get %s action', action)
 
     if (action && this.has(action)) {
       return this.pool[action.id]
@@ -52,7 +52,7 @@ class Archive {
    * Remove a snapshot for an action.
    */
   remove(action: Action) {
-    console.assert(action, 'Unable to remove ' + typeof action + ' action.')
+    console.assert(action, 'Unable to remove %s action.', action)
 
     delete this.pool[action.id]
   }
diff --git a/src/history.js b/src/history.js
index 1c31a823..e92f9674 100644
--- a/src/history.js
+++ b/src/history.js
@@ -10,8 +10,9 @@ import Action from './action'
 import Emitter from './emitter'
 import defaultUpdateStrategy from './default-update-strategy'
 import { merge } from './utils'
-import { BIRTH, START } from './lifecycle'
+import { BIRTH, START, ADD_DOMAIN } from './lifecycle'
 import { type Updater } from './default-update-strategy'
+import { iteratorTag } from './symbols'
 
 type HistoryOptions = {
   maxHistory?: number,
@@ -115,16 +116,44 @@ class History extends Emitter {
   /**
    * Map over the active branch.
    */
-  map(fn: (action: Action) => *, scope?: Object) {
-    let items = []
+  // $FlowFixMe
+  [iteratorTag](): Iterator<Action, void> {
     let cursor = this.root
 
-    while (cursor) {
-      items.push(fn.call(scope, cursor))
+    let iterator = {
+      next: () => {
+        var next = cursor
 
-      if (cursor == this.head) break
-      cursor = cursor.next
+        if (next == null) {
+          return { done: true }
+        }
+
+        cursor = next == this.head ? null : cursor.next
+
+        // Ignore certain lifecycle actions that are only for
+        // internal purposes
+        if (next && (next.command === BIRTH || next.command === START || next.command === ADD_DOMAIN)) {
+          return iterator.next()
+        }
+
+        return { value: next, done: false }
+      }
+    }
+
+    return iterator
+  }
+
+  map(fn: (action: Action, index: number) => *, scope?: Object) {
+    // $FlowFixMe
+    let iterator = this[iteratorTag]()
+    let next = iterator.next()
+    let items = []
+
+    while (!next.done) {
+      items.push(fn.call(scope, next.value, items.length))
+      next = iterator.next()
     }
+
     return items
   }
 
@@ -287,6 +316,7 @@ class History extends Emitter {
    * archival process at the end of every reconciliation. If the
    * active branch of history is greater than the `limit` property,
    * signal that the action should be removed.
+   * @private
    */
   archive() {
     let size = this.size
@@ -365,6 +395,10 @@ class History extends Emitter {
     return this.head
   }
 
+  toString() {
+    return this.toArray().join(', ')
+  }
+
   /**
    * Serialize history into JSON data
    */
diff --git a/src/symbols.js b/src/symbols.js
new file mode 100644
index 00000000..42380fcd
--- /dev/null
+++ b/src/symbols.js
@@ -0,0 +1,10 @@
+/**
+ * @flow
+ */
+import { get, isFunction } from './utils'
+
+const $Symbol = isFunction(Symbol) ? Symbol : {}
+
+export const toStringTag: * = get($Symbol, 'toStringTag', '@@toStringTag')
+
+export const iteratorTag: * = get($Symbol, 'iterator', '@@iterator')
diff --git a/src/utils.js b/src/utils.js
index 04013e70..922b7dd6 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -4,6 +4,7 @@
 
 import type Microcosm from './microcosm'
 import { castPath, type KeyPath } from './key-path'
+import { toStringTag } from './symbols'
 
 type MixedObject = { [key: string]: mixed }
 
@@ -173,14 +174,12 @@ export function isBlank(value: any): boolean {
  * some legacy support.
  */
 /* istanbul ignore next */
-const $Symbol = typeof Symbol === 'function' ? Symbol : {}
-const toStringTagSymbol = get($Symbol, 'toStringTag', '@@toStringTag')
-export function toStringTag(value: any): string {
+export function getStringTag(value: any): string {
   if (!value) {
     return ''
   }
 
-  return value[toStringTagSymbol] || ''
+  return value[toStringTag] || ''
 }
 
 /**
@@ -188,7 +187,7 @@ export function toStringTag(value: any): string {
  * informed by the regenerator runtime.
  */
 export function isGeneratorFn(value: any): boolean {
-  return toStringTag(value) === 'GeneratorFunction'
+  return getStringTag(value) === 'GeneratorFunction'
 }
 
 export function createOrClone(target: any, options: ?Object, repo: Microcosm) {
@@ -219,3 +218,10 @@ export function update(
 
   return set(state, path, next)
 }
+
+/**
+ * A couple of methods use identity functions. This avoids duplication.
+ */
+export function identity(n: *) {
+  return n
+}


Test patch:
diff --git a/test/unit/history/archive.test.js b/test/unit/history/archive.test.js
index 43f6f33a..62e29e8e 100644
--- a/test/unit/history/archive.test.js
+++ b/test/unit/history/archive.test.js
@@ -68,9 +68,9 @@ describe('History::archive', function() {
   it('archived nodes have no relations', function() {
     const history = new History()
 
-    history.append(action).resolve()
+    history.append('one').resolve()
 
-    history.append(action)
+    history.append('two')
 
     history.archive()
 
diff --git a/test/unit/history/iterator.test.js b/test/unit/history/iterator.test.js
new file mode 100644
index 00000000..3e884171
--- /dev/null
+++ b/test/unit/history/iterator.test.js
@@ -0,0 +1,64 @@
+import History from '../../../src/history'
+import Observable from 'zen-observable'
+
+describe('History @@iterator', function() {
+  it('works with Array.from', function() {
+    let history = new History({ maxHistory: Infinity })
+
+    history.append('one', 'resolve')
+    history.append('two', 'resolve')
+
+    let list = Array.from(history)
+
+    expect(list.map(i => i.type)).toEqual(['one', 'two'])
+  })
+
+  it('works with for...of', function() {
+    let history = new History({ maxHistory: Infinity })
+
+    history.append('one', 'resolve')
+    history.append('two', 'resolve')
+
+    let list = []
+
+    for (var action of history) {
+      list.push(action.type)
+    }
+
+    expect(list).toEqual(['one', 'two'])
+  })
+
+  it('works with Promise.all', async function() {
+    let history = new History({ maxHistory: Infinity })
+
+    let one = history.append('one', 'resolve')
+    let two = history.append('two', 'resolve')
+
+    let promise = Promise.all(history)
+
+    one.resolve()
+    two.resolve()
+
+    // Basically, this should complete
+    await promise.then(() => {
+      for (var action of history) {
+        expect(action).toHaveStatus('resolve')
+      }
+    })
+  })
+
+  it('works with Observable.of', function() {
+    let history = new History({ maxHistory: Infinity })
+
+    let one = history.append('one', 'resolve')
+    let two = history.append('two', 'resolve')
+    let complete = jest.fn()
+
+    Observable.of(history).subscribe({ complete })
+
+    one.resolve()
+    two.resolve()
+
+    expect(complete).toHaveBeenCalled()
+  })
+})
diff --git a/test/unit/history/map.test.js b/test/unit/history/map.test.js
new file mode 100644
index 00000000..5d1ec7c1
--- /dev/null
+++ b/test/unit/history/map.test.js
@@ -0,0 +1,17 @@
+import History from '../../../src/history'
+
+describe('History::map', function() {
+  const action = n => n
+
+  it('provides the index', function() {
+    const history = new History({ maxHistory: Infinity })
+
+    history.append('one', 'resolve')
+    history.append('two', 'resolve')
+    history.append('three', 'resolve')
+
+    let result = history.map((action, i) => i)
+
+    expect(result).toEqual([0, 1, 2])
+  })
+})
diff --git a/test/unit/history/remove.test.js b/test/unit/history/remove.test.js
index 365756a4..2da5f6e7 100644
--- a/test/unit/history/remove.test.js
+++ b/test/unit/history/remove.test.js
@@ -49,9 +49,9 @@ describe('History::remove', function() {
     it('adjusts the head to the prior node', function() {
       let history = new History({ maxHistory: Infinity })
 
-      history.append(function one() {}, 'resolve')
-      history.append(function two() {}, 'resolve')
-      history.append(function three() {}, 'resolve')
+      history.append('one', 'resolve')
+      history.append('two', 'resolve')
+      history.append('three', 'resolve')
 
       let head = history.head
       let prior = head.parent
@@ -64,13 +64,13 @@ describe('History::remove', function() {
     it('removes the node from the active branch', function() {
       let history = new History({ maxHistory: Infinity })
 
-      history.append(function one() {}, 'resolve')
-      history.append(function two() {}, 'resolve')
+      history.append('one', 'resolve')
+      history.append('two', 'resolve')
       let three = history.append(function three() {}, 'resolve')
 
       history.remove(three)
 
-      expect(history.map(a => a.command.name)).toEqual(['$start', 'one', 'two'])
+      expect(history.toString()).toEqual('one, two')
     })
 
     it('removing the head node eliminates the reference to "next"', function() {
@@ -105,13 +105,13 @@ describe('History::remove', function() {
     it('updates the active branch', function() {
       let history = new History({ maxHistory: Infinity })
 
-      history.append(function one() {}, 'resolve')
-      history.append(function two() {}, 'resolve')
-      history.append(function three() {}, 'resolve')
+      history.append('one', 'resolve')
+      history.append('two', 'resolve')
+      history.append('three', 'resolve')
 
       history.remove(history.root)
 
-      expect(history.map(a => a.command.name)).toEqual(['one', 'two', 'three'])
+      expect(history.toString()).toEqual('one, two, three')
     })
   })
 
@@ -119,38 +119,34 @@ describe('History::remove', function() {
     it('joins the parent and child', function() {
       let history = new History({ maxHistory: Infinity })
 
-      history.append(function one() {}, 'resolve')
-      let two = history.append(function two() {}, 'resolve')
-      history.append(function three() {}, 'resolve')
+      history.append('one', 'resolve')
+      let two = history.append('two', 'resolve')
+      history.append('three', 'resolve')
 
       history.remove(two)
 
-      expect(history.map(a => a.command.name)).toEqual([
-        '$start',
-        'one',
-        'three'
-      ])
+      expect(history.toString()).toEqual('one, three')
     })
 
     it('resets the head if removing the head', function() {
       let history = new History({ maxHistory: Infinity })
 
-      let one = history.append(function one() {}, 'resolve')
-      let two = history.append(function two() {}, 'resolve')
-      history.append(function three() {}, 'resolve')
+      let one = history.append('one', 'resolve')
+      let two = history.append('two', 'resolve')
+      history.append('three', 'resolve')
 
       history.checkout(two)
       history.remove(two)
 
-      expect(history.head.id).toBe(one.id)
+      expect(history.toString()).toEqual('one')
     })
 
     it('reconciles at the next action', function() {
       let history = new History({ maxHistory: Infinity })
 
-      history.append(function one() {}, 'resolve')
-      let two = history.append(function two() {}, 'resolve')
-      let three = history.append(function three() {}, 'resolve')
+      history.append('one', 'resolve')
+      let two = history.append('two', 'resolve')
+      let three = history.append('three', 'resolve')
 
       jest.spyOn(history, 'reconcile')
 
@@ -162,9 +158,9 @@ describe('History::remove', function() {
     it('reconciles at the parent if the action is head of an active branch', function() {
       let history = new History({ maxHistory: Infinity })
 
-      let one = history.append(function one() {}, 'resolve')
-      let two = history.append(function two() {}, 'resolve')
-      history.append(function three() {}, 'resolve')
+      let one = history.append('one', 'resolve')
+      let two = history.append('two', 'resolve')
+      history.append('three', 'resolve')
 
       history.checkout(two)
 
@@ -178,9 +174,9 @@ describe('History::remove', function() {
     it('does not reconcile if the action is not in active branch', function() {
       let history = new History({ maxHistory: Infinity })
 
-      history.append(function one() {}, 'resolve')
-      let two = history.append(function two() {}, 'resolve')
-      let three = history.append(function three() {}, 'resolve')
+      history.append('one', 'resolve')
+      let two = history.append('two', 'resolve')
+      let three = history.append('three', 'resolve')
 
       history.checkout(two)
 
@@ -196,11 +192,11 @@ describe('History::remove', function() {
     it('leaves the head reference alone', function() {
       let history = new History({ maxHistory: Infinity })
 
-      let one = history.append(function one() {}, 'resolve')
-      let two = history.append(function two() {}, 'resolve')
+      let one = history.append('one', 'resolve')
+      let two = history.append('two', 'resolve')
 
       history.checkout(one)
-      let three = history.append(function three() {}, 'resolve')
+      let three = history.append('three', 'resolve')
 
       // History tree now looks like this:
       //                |- [two]
@@ -209,7 +205,7 @@ describe('History::remove', function() {
 
       history.remove(two)
 
-      expect(history.head.id).toBe(three.id)
+      expect(history.toString()).toBe('one, three')
     })
   })
 
@@ -217,43 +213,44 @@ describe('History::remove', function() {
     it('eliminates references to removed on the left', function() {
       let history = new History({ maxHistory: Infinity })
 
-      let one = history.append(function one() {}, 'resolve')
-      let two = history.append(function two() {}, 'resolve')
+      let one = history.append('one', 'resolve')
+      let two = history.append('two', 'resolve')
 
       history.checkout(one)
 
-      history.append(function three() {}, 'resolve')
+      let three = history.append('three', 'resolve')
 
       history.remove(two)
 
-      expect(one.children.map(i => i.command.name)).toEqual(['three'])
+      expect(one.children).not.toContain(two)
+      expect(one.children).toContain(three)
     })
 
     it('maintains children on the left when the next action is removed', function() {
       let history = new History({ maxHistory: Infinity })
 
-      let one = history.append(function one() {}, 'resolve')
-      history.append(function two() {}, 'resolve')
+      let one = history.append('one', 'resolve')
+      let two = history.append('two', 'resolve')
 
       history.checkout(one)
 
-      let three = history.append(function three() {}, 'resolve')
+      let three = history.append('three', 'resolve')
 
       history.remove(three)
 
-      expect(one.children.map(i => i.command.name)).toEqual(['two'])
+      expect(one.children).toEqual([two])
     })
 
     it('allows having children, but no next value', function() {
       let history = new History({ maxHistory: Infinity })
 
-      let one = history.append(function one() {}, 'resolve')
+      let one = history.append('one', 'resolve')
 
-      history.append(function two() {}, 'resolve')
+      history.append('two', 'resolve')
 
       history.checkout(one)
 
-      let three = history.append(function three() {}, 'resolve')
+      let three = history.append('three', 'resolve')
 
       history.remove(three)
 
diff --git a/test/unit/history/updater.test.js b/test/unit/history/updater.test.js
index 848c942a..2ede919b 100644
--- a/test/unit/history/updater.test.js
+++ b/test/unit/history/updater.test.js
@@ -59,7 +59,7 @@ describe('History updater', function() {
 
     history.on('release', handler)
 
-    history.append('action').resolve()
+    history.append('one').resolve()
 
     await history.wait()