Skip to content

Commit

Permalink
add allowStateChangesInsideComputed to allow changes inside a compute…
Browse files Browse the repository at this point in the history
…d value (#1706)

* make allowStateChanges allow changes inside computed value

in order to fix MST mobxjs/mobx-state-tree#967

* added allowStateChangeInsideComputed

* added unit test

* small unit test error

* safer implementation

* Alternative solution

* Revert "Alternative solution"

This reverts commit e15bc78.

* improve unit test

* Changed to more predictable restoration
  • Loading branch information
xaviergonz authored and mweststrate committed Sep 25, 2018
1 parent c86ba97 commit fa09fa6
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 0 deletions.
12 changes: 12 additions & 0 deletions src/core/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,15 @@ export function allowStateChangesStart(allowStateChanges: boolean) {
export function allowStateChangesEnd(prev: boolean) {
globalState.allowStateChanges = prev
}

export function allowStateChangesInsideComputed<T>(func: () => T): T {
const prev = globalState.computationDepth
globalState.computationDepth = 0
let res: T
try {
res = func()
} finally {
globalState.computationDepth = prev
}
return res
}
1 change: 1 addition & 0 deletions src/core/globalstate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export class MobXGlobals {
* To ensure that those functions stay pure.
*/
allowStateChanges = true

/**
* If strict mode is enabled, state changes are by default not allowed
*/
Expand Down
1 change: 1 addition & 0 deletions src/mobx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export {
getAtom,
getAdministration as _getAdministration,
allowStateChanges as _allowStateChanges,
allowStateChangesInsideComputed as _allowStateChangesInsideComputed,
Lambda,
isArrayLike,
$mobx,
Expand Down
38 changes: 38 additions & 0 deletions test/base/action.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,44 @@ test("should be possible to change unobserved state in an action called from com
mobx._resetGlobalState()
})

test("should be possible to change observed state in an action called from computed if run inside _allowStateChangesInsideComputed", () => {
const a = mobx.observable.box(2)
const d = mobx.autorun(() => {
a.get()
})

const testAction = mobx.action(() => {
mobx._allowStateChangesInsideComputed(() => {
a.set(3)
// a second level computed should throw
expect(() => c2.get()).toThrowError(
/Computed values are not allowed to cause side effects by changing observables that are already being observed/
)
})
expect(a.get()).toBe(3)
expect(() => {
a.set(4)
}).toThrowError(
/Computed values are not allowed to cause side effects by changing observables that are already being observed/
)
})

const c = mobx.computed(() => {
testAction()
return a.get()
})

const c2 = mobx.computed(() => {
a.set(6)
return a.get()
})

c.get()

mobx._resetGlobalState()
d()
})

test("should not be possible to change observed state in an action called from computed", () => {
var a = mobx.observable.box(2)
var d = mobx.autorun(() => {
Expand Down
1 change: 1 addition & 0 deletions test/base/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ test("correct api should be exposed", function() {
"$mobx", // adminstration symbol
"action",
"_allowStateChanges",
"_allowStateChangesInsideComputed",
"autorun",
"comparer",
"computed",
Expand Down

0 comments on commit fa09fa6

Please sign in to comment.