Repository: vigetlabs/microcosm
Pull request #257 (2017-03-13 19:29:12)

Fix nested snapshots
We create a snapshot of the state for every action so that we call rollback to that state when resolving changes to actions. This commit fixes a bug where actions pushed inside of other actions would cause a rollback to a snapshot that hadn't been saved yet, effectively reverting to initial state.

The original code that introduced the bug:

```javascript
export function spamCheck () {
  return function (action, repo) {
    repo.push(focusBlock, null)

    // Operates on action, pushing more actions
    Api().spamCheck(action, repo)
  }
}
```


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f2a479bd..a75fccb7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog
 
+## 12.1.3
+
+- Fix issue where pushing actions inside of other actions would cause
+  a rollback to initial state.
+
 ## 12.1.2
 
 - Fixed issue where parent repos would send out changes before
diff --git a/package.json b/package.json
index 1e42d738..c85315cb 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "microcosm",
-  "version": "12.1.2",
+  "version": "12.1.3",
   "private": true,
   "description": "Flux with actions at center stage. Write optimistic updates, cancel requests, and track changes with ease.",
   "main": "microcosm.js",
diff --git a/src/history.js b/src/history.js
index ee9bee4a..d0678a88 100644
--- a/src/history.js
+++ b/src/history.js
@@ -84,6 +84,8 @@ History.prototype = {
     this.head = action
     this.size += 1
 
+    this.invoke('createInitialSnapshot', action)
+
     return this.head
   },
 
@@ -105,7 +107,7 @@ History.prototype = {
 
     this.archive()
 
-    this.invoke('update')
+    this.invoke('prepareRelease')
 
     this.invoke('release', action)
   },
@@ -116,7 +118,7 @@ History.prototype = {
 
     while (size > this.limit && root.disposable) {
       size -= 1
-      this.invoke('clean', root.parent)
+      this.invoke('removeSnapshot', root.parent)
       root = root.next
     }
 
diff --git a/src/lifecycle.js b/src/lifecycle.js
index c96ba385..f3f427ae 100644
--- a/src/lifecycle.js
+++ b/src/lifecycle.js
@@ -16,14 +16,18 @@ function sandbox (data, deserialize) {
   }
 }
 
-export const RESET = tag(function reset (data, deserialize) {
+export const RESET = tag(function $reset (data, deserialize) {
   return sandbox(data, deserialize)
 }, '$reset')
 
-export const PATCH = tag(function patch (data, deserialize) {
+export const PATCH = tag(function $patch (data, deserialize) {
   return sandbox(data, deserialize)
 }, '$patch')
 
-export const BIRTH = '$birth'
+export const BIRTH = function $birth () {
+  console.assert(false, 'Birth lifecycle method should never be invoked directly.')
+}
 
-export const START = '$start'
+export const START = function $start () {
+  console.assert(false, 'Start lifecycle method should never be invoked directly.')
+}
diff --git a/src/microcosm.js b/src/microcosm.js
index b43fae9c..e8a4ac68 100644
--- a/src/microcosm.js
+++ b/src/microcosm.js
@@ -78,19 +78,36 @@ inherit(Microcosm, Emitter, {
 
   recall (action) {
     console.assert(action, 'Unable to get ' + typeof action + ' action')
+
     return this.archive.get(action.id)
   },
 
-  save (action, state) {
-    console.assert(action, 'Unable to set ' + typeof action + ' action.')
-    console.assert(action.command !== BIRTH, 'Birth action should never be set.')
+  /**
+   * Create the initial state snapshot for an action. This is
+   * important so that, when rolling back to this action, it always
+   * has a state value.
+   * @param {Action} action - The action to generate a snapshot for
+   */
+  createInitialSnapshot (action) {
+    let state = this.recall(action.parent)
 
-    return this.archive.set(action.id, state)
+    this.updateSnapshot(action, state)
   },
 
-  clean (action) {
+  /**
+   * Update the state snapshot for a given action
+   * @param {Action} action - The action to update the snapshot for
+   */
+  updateSnapshot (action, state) {
+    this.archive.set(action.id, state)
+  },
+
+  /**
+   * Remove the snapshot for a given action
+   * @param {Action} action - The action to remove the snapshot for
+   */
+  removeSnapshot (action) {
     console.assert(action, 'Unable to remove ' + typeof action + ' action.')
-    console.assert(action.command !== BIRTH, 'Birth action should never be removed.')
 
     this.archive.remove(action.id)
   },
@@ -126,12 +143,12 @@ inherit(Microcosm, Emitter, {
       next = this.dispatch(next, action)
     }
 
-    this.save(action, next)
+    this.updateSnapshot(action, next)
 
     return this
   },
 
-  update () {
+  prepareRelease () {
     let next = this.follower ? this.parent.state : this.recall(this.history.head)
 
     this.dirty = next !== this.state


Test patch:
diff --git a/test/integration/nested-actions.test.js b/test/integration/nested-actions.test.js
new file mode 100644
index 00000000..72db768c
--- /dev/null
+++ b/test/integration/nested-actions.test.js
@@ -0,0 +1,63 @@
+/**
+ * This test covers a bug where the state gets out of sync when an
+ * action is pushed within another action. We first encountered this
+ * on a project where we wanted to remove a "focused" state when saving
+ * data using another action.
+ */
+
+import Microcosm, { update } from '../../src/microcosm'
+
+describe('When pushing actions inside of another action', function () {
+
+  it('does not return to an old state when pushing the second action', function () {
+    expect.assertions(8)
+
+    let repo = new Microcosm()
+
+    function focuser (focus) {
+      return focus
+    }
+
+    function stepper () {
+      return function (action, repo) {
+        expect(repo).toHaveState('data.count', 2)
+        expect(repo).toHaveState('data.focus', true)
+
+        repo.push(focuser, false)
+
+        expect(repo).toHaveState('data.count', 2)
+        expect(repo).toHaveState('data.focus', false)
+
+        action.open()
+        expect(repo).toHaveState('data.count', 2)
+        expect(repo).toHaveState('data.focus', false)
+
+        action.resolve(1)
+        expect(repo).toHaveState('data.count', 3)
+        expect(repo).toHaveState('data.focus', false)
+      }
+    }
+
+    repo.addDomain('data', {
+      getInitialState () {
+        return { count: 0, focus: false }
+      },
+      increase (state, amount) {
+        return update(state, 'count', n => n + amount)
+      },
+      setFocus (state, yes) {
+        return update(state, 'focus', yes)
+      },
+      register () {
+        return {
+          [stepper]: this.increase,
+          [focuser]: this.setFocus
+        }
+      }
+    })
+
+    repo.patch({ data: { count: 2, focus: true } })
+    repo.push(stepper)
+  })
+
+})
diff --git a/test/unit/microcosm/lifecycle.test.js b/test/unit/microcosm/lifecycle.test.js
new file mode 100644
index 00000000..a45acaec
--- /dev/null
+++ b/test/unit/microcosm/lifecycle.test.js
@@ -0,0 +1,24 @@
+import Microcosm from '../../../src/microcosm'
+
+import {
+  BIRTH,
+  START
+} from '../../../src/lifecycle'
+
+describe('Lifecycle', function () {
+
+  it('$birth should never be invoked directly', function () {
+    let repo = new Microcosm()
+    let test = repo.prepare(BIRTH)
+
+    expect(test).toThrow()
+  })
+
+  it('$start should never be invoked directly', function () {
+    let repo = new Microcosm()
+    let test = repo.prepare(START)
+
+    expect(test).toThrow()
+  })
+
+})