Merge pull request #16 from songzhenqi/fix-rollback-from-action-error

fix #15 - rollback by recorder.undo() should not be an UndoState

Author: coolsoftwaretyler

__tests__/undo-manager.test.ts

@@ -154,7 +154,7 @@ test("can time travel with Mutable object", () => {
             setUndoManagerDifferentTree(self)
             return {
                 setProp(k: string, v: any) {
-                    ;(self.mutable as any).set(k, v)
+                    ; (self.mutable as any).set(k, v)
                 }
             }
         })
@@ -568,3 +568,31 @@ describe("includeHooks flag", () => {
         ])
     })
 })
+
+test("#15 - rollback by recorder.undo() should not be an UndoState", () => {
+    let _undoManager: any
+    const Model = types
+        .model('Model', {
+            value: 1,
+        })
+        .actions(self => {
+            _undoManager = UndoManager.create({}, { targetStore: self })
+            function setValue(value: number) {
+                self.value = value;
+                throw 'some error throw by actions'
+            }
+            return {
+                setValue,
+            }
+        })
+
+    const model = Model.create({})
+    try {
+        model.setValue(2)
+    } catch (e) {
+        expect(e).toBe('some error throw by actions')
+    }
+    // rollback successfully and no UndoState
+    expect(model.value).toBe(1)
+    expect(_undoManager.history).toHaveLength(0)
+})

src/undo-manager.ts

@@ -85,10 +85,10 @@ const UndoManager = types
             },
             onFinish(call, error) {
                 const recorder = call.env!.recorder
-                call.env = undefined
                 recorder.stop()
-
                 if (error === undefined) {
+                    // if no errors from the action, we can safely clean up call.env before proceeding further
+                    call.env = undefined 
                     if (groupRecorders.length > 0) {
                         const groupRecorder = groupRecorders[groupRecorders.length - 1]
                         groupRecorder.patches = groupRecorder.patches.concat(recorder.patches)
@@ -99,7 +99,9 @@ const UndoManager = types
                         ;(self as any).addUndoState(recorder)
                     }
                 } else {
+                    // if there was an error, we need call.env to filtered action from `recorder.undo -> applyPatch`, so we clean up call.env afterwards
                     recorder.undo()
+                    call.env = undefined
                 }
             }
         })