Skip to content

Commit

Permalink
Fixed #940: maps not being protected by strict mode
Browse files Browse the repository at this point in the history
  • Loading branch information
mweststrate committed Sep 19, 2017
1 parent 48aa197 commit ad81239
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 15 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
16 changes: 7 additions & 9 deletions src/api/transaction.ts
Original file line number Diff line number Diff line change
@@ -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();
}
}
12 changes: 6 additions & 6 deletions src/types/observablemap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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]!
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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))
Expand All @@ -295,15 +295,15 @@ export class ObservableMap<V>
}

clear() {
runInTransaction(() => {
transaction(() => {
untracked(() => {
this.keys().forEach(this.delete, this)
})
})
}

replace(values: ObservableMap<V> | IKeyValueMap<V> | any): ObservableMap<V> {
runInTransaction(() => {
transaction(() => {
this.clear()
this.merge(values)
})
Expand Down
16 changes: 16 additions & 0 deletions test/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})

0 comments on commit ad81239

Please sign in to comment.