From 028bc0c4be9309e31f3c77ef2e4244ba17352d0c Mon Sep 17 00:00:00 2001
From: Scott Mikula <mikula@gmail.com>
Date: Mon, 11 Mar 2019 16:14:03 -0700
Subject: [PATCH] Wrap dispatch in a transaction (#106)

For various historical reasons dispatch hasn't been wrapped in `transaction` since an early version of Satchel.  This means that every subscriber to an action executes in its own transaction, possibly causing multiple unnecessary renders.  Now that we've updated to MobX v4, it's possible to use `transaction` again.

The change to actual source code is trivial; the rest of the change is test code to validate that we're handling the transaction correctly.
---
 src/dispatcher.ts     |  6 ++++-
 test/endToEndTests.ts | 63 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/src/dispatcher.ts b/src/dispatcher.ts
index 6fb1ad2..6937e72 100644
--- a/src/dispatcher.ts
+++ b/src/dispatcher.ts
@@ -1,3 +1,4 @@
+import { transaction } from 'mobx';
 import ActionMessage from './interfaces/ActionMessage';
 import Subscriber from './interfaces/Subscriber';
 import { getPrivateActionId } from './actionCreator';
@@ -18,7 +19,10 @@ export function dispatch(actionMessage: ActionMessage) {
     }
 
     let dispatchWithMiddleware = getGlobalContext().dispatchWithMiddleware || finalDispatch;
-    dispatchWithMiddleware(actionMessage);
+
+    transaction(() => {
+        dispatchWithMiddleware(actionMessage);
+    });
 }
 
 export function finalDispatch(actionMessage: ActionMessage): void | Promise<void> {
diff --git a/test/endToEndTests.ts b/test/endToEndTests.ts
index b169fc5..17dd719 100644
--- a/test/endToEndTests.ts
+++ b/test/endToEndTests.ts
@@ -1,11 +1,14 @@
 import 'jasmine';
-import { action } from '../src/actionCreator';
-import applyMiddleware from '../src/applyMiddleware';
-import { dispatch } from '../src/dispatcher';
-import mutator from '../src/mutator';
-import orchestrator from '../src/orchestrator';
-import { mutatorAction } from '../src/simpleSubscribers';
-import createStore from '../src/createStore';
+import { autorun } from 'mobx';
+import {
+    action,
+    applyMiddleware,
+    createStore,
+    dispatch,
+    mutator,
+    mutatorAction,
+    orchestrator,
+} from '../src/index';
 
 describe('satcheljs', () => {
     it('mutators subscribe to actions', () => {
@@ -19,7 +22,7 @@ describe('satcheljs', () => {
         });
 
         // Create a mutator that subscribes to it
-        let onTestAction = mutator(testAction, function(actionMessage) {
+        mutator(testAction, function(actionMessage) {
             actualValue = actionMessage.value;
         });
 
@@ -54,9 +57,10 @@ describe('satcheljs', () => {
     it('mutators can modify the store', () => {
         // Arrange
         let store = createStore('testStore', { testProperty: 'testValue' })();
+        autorun(() => store.testProperty); // strict mode only applies if store is observed
         let modifyStore = action('modifyStore');
 
-        let onModifyStore = mutator(modifyStore, actionMessage => {
+        mutator(modifyStore, () => {
             store.testProperty = 'newValue';
         });
 
@@ -67,6 +71,47 @@ describe('satcheljs', () => {
         expect(store.testProperty).toBe('newValue');
     });
 
+    it('orchestrators cannot modify the store', () => {
+        // Arrange
+        let store = createStore('testStore', { testProperty: 'testValue' })();
+        autorun(() => store.testProperty); // strict mode only applies if store is observed
+        let modifyStore = action('modifyStore');
+
+        orchestrator(modifyStore, () => {
+            store.testProperty = 'newValue';
+        });
+
+        // Act / Assert
+        expect(() => {
+            modifyStore();
+        }).toThrow();
+    });
+
+    it('all subscribers are handled in one transaction', () => {
+        // Arrange
+        let store = createStore('testStore', { testProperty: 0 })();
+        let modifyStore = action('modifyStore');
+
+        mutator(modifyStore, () => {
+            store.testProperty++;
+        });
+
+        mutator(modifyStore, () => {
+            store.testProperty++;
+        });
+
+        let values: number[] = [];
+        autorun(() => {
+            values.push(store.testProperty);
+        });
+
+        // Act
+        modifyStore();
+
+        // Assert
+        expect(values).toEqual([0, 2]);
+    });
+
     it('middleware gets called during dispatch', () => {
         // Arrange
         let actualValue;