From ad81239ba4aa364a86f455d284b0ba2c22693024 Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@gmail.com>
Date: Tue, 19 Sep 2017 17:31:45 +0200
Subject: [PATCH] Fixed #940: maps not being protected by strict mode

---
 CHANGELOG.md               |  5 +++++
 src/api/transaction.ts     | 16 +++++++---------
 src/types/observablemap.ts | 12 ++++++------
 test/map.js                | 16 ++++++++++++++++
 4 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5d18275fb..d9f8b1658 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+# Next
+
+* Fix bug allowing maps to be modified outside actions when using strict mode, fixes [#940](https://github.com/mobxjs/mobx/issues/940)
+* Fixed [#1139](https://github.com/mobxjs/mobx/issues/1139) properly
+
 # 3.3.0
 
 * Undeprecated `transaction`, see [#1139](https://github.com/mobxjs/mobx/issues/1139)
diff --git a/src/api/transaction.ts b/src/api/transaction.ts
index 19c6a6f55..a7f9ec10b 100644
--- a/src/api/transaction.ts
+++ b/src/api/transaction.ts
@@ -1,20 +1,18 @@
-import { deprecated } from "../utils/utils"
+import { startBatch, endBatch } from '../core/observable';
 import { executeAction } from "../core/action"
 
 /**
- * @deprecated
  * During a transaction no views are updated until the end of the transaction.
  * The transaction will be run synchronously nonetheless.
  *
- * Deprecated to simplify api; transactions offer no real benefit above actions.
- *
  * @param action a function that updates some reactive state
  * @returns any value that was returned by the 'action' parameter.
  */
 export function transaction<T>(action: () => T, thisArg = undefined): T {
-    return runInTransaction.apply(undefined, arguments)
-}
-
-export function runInTransaction<T>(action: () => T, thisArg = undefined): T {
-    return executeAction("", action)
+	startBatch()
+	try {
+		return action.apply(thisArg)
+	} finally {
+		endBatch();
+	}
 }
diff --git a/src/types/observablemap.ts b/src/types/observablemap.ts
index 8e41db58d..675dbfc97 100644
--- a/src/types/observablemap.ts
+++ b/src/types/observablemap.ts
@@ -23,7 +23,7 @@ import { IListenable, registerListener, hasListeners, notifyListeners } from "./
 import { isSpyEnabled, spyReportStart, spyReportEnd } from "../core/spy"
 import { arrayAsIterator, declareIterator, Iterator } from "../utils/iterable"
 import { observable } from "../api/observable"
-import { runInTransaction } from "../api/transaction"
+import { transaction } from "../api/transaction"
 import { referenceEnhancer } from "./modifiers"
 import { getMessage } from "../utils/messages"
 
@@ -166,7 +166,7 @@ export class ObservableMap<V>
                     : null
 
             if (notifySpy) spyReportStart(change)
-            runInTransaction(() => {
+            transaction(() => {
                 this._keys.remove(key)
                 this._updateHasMapEntry(key, false)
                 const observable = this._data[key]!
@@ -221,7 +221,7 @@ export class ObservableMap<V>
     }
 
     private _addValue(name: string, newValue: V | undefined) {
-        runInTransaction(() => {
+        transaction(() => {
             const observable = (this._data[name] = new ObservableValue(
                 newValue,
                 this.enhancer,
@@ -284,7 +284,7 @@ export class ObservableMap<V>
         if (isObservableMap(other)) {
             other = other.toJS()
         }
-        runInTransaction(() => {
+        transaction(() => {
             if (isPlainObject(other)) Object.keys(other).forEach(key => this.set(key, other[key]))
             else if (Array.isArray(other)) other.forEach(([key, value]) => this.set(key, value))
             else if (isES6Map(other)) other.forEach((value, key) => this.set(key, value))
@@ -295,7 +295,7 @@ export class ObservableMap<V>
     }
 
     clear() {
-        runInTransaction(() => {
+        transaction(() => {
             untracked(() => {
                 this.keys().forEach(this.delete, this)
             })
@@ -303,7 +303,7 @@ export class ObservableMap<V>
     }
 
     replace(values: ObservableMap<V> | IKeyValueMap<V> | any): ObservableMap<V> {
-        runInTransaction(() => {
+        transaction(() => {
             this.clear()
             this.merge(values)
         })
diff --git a/test/map.js b/test/map.js
index cc6fd08b1..0f8c302fc 100644
--- a/test/map.js
+++ b/test/map.js
@@ -616,3 +616,19 @@ test("work with 'toString' key", t => {
     t.equal(m.get("toString"), "test")
     t.end()
 })
+
+test("issue 940, should not be possible to change maps outside strict mode", t => {
+	mobx.useStrict(true)
+
+	const m = mobx.observable.map()
+	const d = mobx.autorun(() => m.values())
+
+	t.throws(() => {
+		m.set("x", 1)
+	}, /Since strict-mode is enabled/)
+
+	d();
+
+	mobx.useStrict(false)
+	t.end()
+})