diff --git a/packages/mobx-react/__tests__/observer.test.tsx b/packages/mobx-react/__tests__/observer.test.tsx index bb633fe79..98fcede5d 100644 --- a/packages/mobx-react/__tests__/observer.test.tsx +++ b/packages/mobx-react/__tests__/observer.test.tsx @@ -1113,7 +1113,7 @@ test(`Component react's to observable changes in componenDidMount #3691`, () => expect(container).toHaveTextContent("1") unmount() }) -// TODO + test(`Observable changes in componenWillUnmount don't cause any warnings or errors`, () => { const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {}) const consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}) diff --git a/packages/mobx/__tests__/v4/base/extras.js b/packages/mobx/__tests__/v4/base/extras.js index 8fd4927b9..420161fa7 100644 --- a/packages/mobx/__tests__/v4/base/extras.js +++ b/packages/mobx/__tests__/v4/base/extras.js @@ -104,9 +104,9 @@ test("spy 1", function () { lines.push(line) }) - a.set(4) + m._allowStateChanges(true, () => a.set(4)) stop() - a.set(5) + m._allowStateChanges(true, () => a.set(5)) expect(stripTrackerOutput(lines)).toMatchSnapshot() }) @@ -277,7 +277,7 @@ test("onBecome(Un)Observed simple", () => { expect(events.length).toBe(0) // nothing happened yet x.get() expect(events.length).toBe(0) // nothing happened yet - x.set(4) + m._allowStateChanges(true, () => x.set(4)) expect(events.length).toBe(0) // nothing happened yet const d5 = mobx.reaction( diff --git a/packages/mobx/__tests__/v4/base/tojs.js b/packages/mobx/__tests__/v4/base/tojs.js index 81bd93f8b..92c728406 100644 --- a/packages/mobx/__tests__/v4/base/tojs.js +++ b/packages/mobx/__tests__/v4/base/tojs.js @@ -25,10 +25,14 @@ test("json1", function () { }) .join(", ") }) - - todos[1].title = "improve coverage" // prints: write blog, improve coverage + mobx._allowStateChanges(true, () => { + todos[1].title = "improve coverage" // prints: write blog, improve coverage + }) expect(output).toBe("write blog, improve coverage") - todos.push({ title: "take a nap" }) // prints: write blog, improve coverage, take a nap + mobx._allowStateChanges(true, () => { + todos.push({ title: "take a nap" }) // prints: write blog, improve coverage, take a nap + }) + expect(output).toBe("write blog, improve coverage, take a nap") }) @@ -86,10 +90,12 @@ test("json2", function () { true ) - o.todos[0].details.url = "boe" - o.todos[1].details.url = "ba" - o.todos[0].tags[0] = "reactjs" - o.todos[1].tags.push("pff") + mobx._allowStateChanges(true, () => { + o.todos[0].details.url = "boe" + o.todos[1].details.url = "ba" + o.todos[0].tags[0] = "reactjs" + o.todos[1].tags.push("pff") + }) expect(mobx.toJS(o)).toEqual({ todos: [ @@ -117,12 +123,14 @@ test("json2", function () { ab = [] tb = [] - o.todos.push( - mobx.observable({ - title: "test", - tags: ["x"] - }) - ) + mobx._allowStateChanges(true, () => { + o.todos.push( + mobx.observable({ + title: "test", + tags: ["x"] + }) + ) + }) expect(mobx.toJS(o)).toEqual({ todos: [ @@ -151,13 +159,16 @@ test("json2", function () { ab = [] tb = [] - o.todos[1] = mobx.observable({ - title: "clean the attic", - tags: ["needs sabbatical"], - details: { - url: "booking.com" - } + mobx._allowStateChanges(true, () => { + o.todos[1] = mobx.observable({ + title: "clean the attic", + tags: ["needs sabbatical"], + details: { + url: "booking.com" + } + }) }) + expect(JSON.parse(JSON.stringify(o))).toEqual({ todos: [ { diff --git a/packages/mobx/__tests__/v4/mobx4.ts b/packages/mobx/__tests__/v4/mobx4.ts index f6c0134ee..6719a5aa2 100644 --- a/packages/mobx/__tests__/v4/mobx4.ts +++ b/packages/mobx/__tests__/v4/mobx4.ts @@ -1,8 +1,7 @@ import { configure } from "../../src/mobx" configure({ - useProxies: "never", - enforceActions: "never" + useProxies: "never" }) export * from "../../src/mobx" diff --git a/packages/mobx/__tests__/v5/base/action.js b/packages/mobx/__tests__/v5/base/action.js index cfe782d11..ea87c39be 100644 --- a/packages/mobx/__tests__/v5/base/action.js +++ b/packages/mobx/__tests__/v5/base/action.js @@ -600,7 +600,9 @@ test("auto action can be used to update and is batched", () => { d() }) -test("auto action should not update state from inside a derivation", async () => { +test("auto action does not act as action in regards to enforceActions inside derivation", async () => { + mobx.configure({ enforceActions: "observed" }) + const a = mobx.observable(1) const d = mobx.autorun(() => a.get()) // observe @@ -612,6 +614,7 @@ test("auto action should not update state from inside a derivation", async () => await mobx.when(() => { expect( utils.grabConsole(() => { + console.log(mobx._getGlobalState()) double() }) ).toMatchInlineSnapshot( @@ -622,7 +625,7 @@ test("auto action should not update state from inside a derivation", async () => d() }) -test("auto action should not update state from inside a derivation", async () => { +test("auto action acts as action when outside derivation", async () => { const a = mobx.observable(1) const d = mobx.autorun(() => a.get()) // observe diff --git a/packages/mobx/__tests__/v5/base/extras.js b/packages/mobx/__tests__/v5/base/extras.js index 11d09bf43..3adba57f5 100644 --- a/packages/mobx/__tests__/v5/base/extras.js +++ b/packages/mobx/__tests__/v5/base/extras.js @@ -155,9 +155,9 @@ test("spy 1", function () { lines.push(line) }) - a.set(4) + m._allowStateChanges(true, () => a.set(4)) stop() - a.set(5) + m._allowStateChanges(true, () => a.set(5)) expect(stripTrackerOutput(lines)).toMatchSnapshot() }) @@ -333,7 +333,7 @@ test("onBecome(Un)Observed simple", () => { expect(events.length).toBe(0) // nothing happened yet x.get() expect(events.length).toBe(0) // nothing happened yet - x.set(4) + m._allowStateChanges(true, () => x.set(4)) expect(events.length).toBe(0) // nothing happened yet const d5 = mobx.reaction( diff --git a/packages/mobx/__tests__/v5/base/strict-mode.js b/packages/mobx/__tests__/v5/base/strict-mode.js index 2fbb885e8..817f42b26 100644 --- a/packages/mobx/__tests__/v5/base/strict-mode.js +++ b/packages/mobx/__tests__/v5/base/strict-mode.js @@ -180,21 +180,20 @@ test("strict mode checks", function () { const x = mobx.observable.box(3) const d = mobx.autorun(() => x.get()) - mobx._allowStateChanges(false, function () { - x.get() - }) - - mobx._allowStateChanges(true, function () { - x.set(7) - }) - expect( utils.grabConsole(function () { + // enforceActions are "never", therefore it doesn't matter whether state changes are allowed or not mobx._allowStateChanges(false, function () { x.set(4) }) + mobx._allowStateChanges(false, function () { + x.get() + }) + mobx._allowStateChanges(true, function () { + x.set(7) + }) }) - ).toMatch(/Side effects like changing state are not allowed at this point/) + ).toMatch("") mobx._resetGlobalState() d() diff --git a/packages/mobx/__tests__/v5/base/tojs.js b/packages/mobx/__tests__/v5/base/tojs.js index bee9e9e47..ad75e63c9 100644 --- a/packages/mobx/__tests__/v5/base/tojs.js +++ b/packages/mobx/__tests__/v5/base/tojs.js @@ -24,10 +24,14 @@ test("json1", function () { }) .join(", ") }) - - todos[1].title = "improve coverage" // prints: write blog, improve coverage + mobx._allowStateChanges(true, () => { + todos[1].title = "improve coverage" // prints: write blog, improve coverage + }) expect(output).toBe("write blog, improve coverage") - todos.push({ title: "take a nap" }) // prints: write blog, improve coverage, take a nap + mobx._allowStateChanges(true, () => { + todos.push({ title: "take a nap" }) // prints: write blog, improve coverage, take a nap + }) + expect(output).toBe("write blog, improve coverage, take a nap") }) @@ -85,10 +89,12 @@ test("json2", function () { true ) - o.todos[0].details.url = "boe" - o.todos[1].details.url = "ba" - o.todos[0].tags[0] = "reactjs" - o.todos[1].tags.push("pff") + mobx._allowStateChanges(true, () => { + o.todos[0].details.url = "boe" + o.todos[1].details.url = "ba" + o.todos[0].tags[0] = "reactjs" + o.todos[1].tags.push("pff") + }) expect(mobx.toJS(o)).toEqual({ todos: [ @@ -116,12 +122,14 @@ test("json2", function () { ab = [] tb = [] - o.todos.push( - mobx.observable({ - title: "test", - tags: ["x"] - }) - ) + mobx._allowStateChanges(true, () => { + o.todos.push( + mobx.observable({ + title: "test", + tags: ["x"] + }) + ) + }) expect(mobx.toJS(o)).toEqual({ todos: [ @@ -150,13 +158,16 @@ test("json2", function () { ab = [] tb = [] - o.todos[1] = mobx.observable({ - title: "clean the attic", - tags: ["needs sabbatical"], - details: { - url: "booking.com" - } + mobx._allowStateChanges(true, () => { + o.todos[1] = mobx.observable({ + title: "clean the attic", + tags: ["needs sabbatical"], + details: { + url: "booking.com" + } + }) }) + expect(JSON.parse(JSON.stringify(o))).toEqual({ todos: [ { diff --git a/packages/mobx/__tests__/v5/base/trace.ts b/packages/mobx/__tests__/v5/base/trace.ts index 5055fe8a5..2a3246d5d 100644 --- a/packages/mobx/__tests__/v5/base/trace.ts +++ b/packages/mobx/__tests__/v5/base/trace.ts @@ -47,7 +47,7 @@ describe("trace", () => { ) expectedLogCalls.push(["[mobx.trace] 'autorun' tracing enabled"]) - mobx.transaction(() => { + mobx.runInAction(() => { x.firstname = "John" expectedLogCalls.push([ "[mobx.trace] 'x.fullname' is invalidated due to a change in: 'x.firstname'" @@ -97,14 +97,14 @@ describe("trace", () => { ) expectedLogCalls.push(["[mobx.trace] 'autorun' tracing enabled"]) - mobx.transaction(() => { + mobx.runInAction(() => { x.foo = 1 expectedLogCalls.push([ "[mobx.trace] 'x.fooIsGreaterThan5' is invalidated due to a change in: 'x.foo'" ]) }) - mobx.transaction(() => { + mobx.runInAction(() => { x.foo = 6 expectedLogCalls.push([ "[mobx.trace] 'x.fooIsGreaterThan5' is invalidated due to a change in: 'x.foo'" @@ -141,8 +141,10 @@ describe("trace", () => { mobx.autorun(() => { x.fullname }) - expect(() => { - x.firstname += "!" - }).not.toThrow("Unexpected identifier") + expect( + mobx.action(() => { + x.firstname += "!" + }) + ).not.toThrow("Unexpected identifier") }) }) diff --git a/packages/mobx/src/api/configure.ts b/packages/mobx/src/api/configure.ts index 23e617b6e..6b5a5c149 100644 --- a/packages/mobx/src/api/configure.ts +++ b/packages/mobx/src/api/configure.ts @@ -38,9 +38,12 @@ export function configure(options: { globalState.verifyProxies = true } if (enforceActions !== undefined) { - const ea = enforceActions === ALWAYS ? ALWAYS : enforceActions === OBSERVED - globalState.enforceActions = ea - globalState.allowStateChanges = ea === true || ea === ALWAYS ? false : true + globalState.enforceActions = + enforceActions === ALWAYS ? ALWAYS : enforceActions === OBSERVED + // TODO + // const ea = enforceActions === ALWAYS ? ALWAYS : enforceActions === OBSERVED + // globalState.enforceActions = ea + // globalState.allowStateChanges = ea === true || ea === ALWAYS ? false : true } ;[ "computedRequiresReaction", diff --git a/packages/mobx/src/core/derivation.ts b/packages/mobx/src/core/derivation.ts index 70ee16959..73793de50 100644 --- a/packages/mobx/src/core/derivation.ts +++ b/packages/mobx/src/core/derivation.ts @@ -139,8 +139,10 @@ export function checkIfStateModificationsAreAllowed(atom: IAtom) { // Should not be possible to change observed state outside strict mode, except during initialization, see #563 if ( !globalState.allowStateChanges && - (hasObservers || globalState.enforceActions === "always") + (globalState.enforceActions === "always" || + (globalState.enforceActions === true && hasObservers)) ) { + // TODO the else part never occurs ATM console.warn( "[MobX] " + (globalState.enforceActions diff --git a/packages/mobx/src/core/globalstate.ts b/packages/mobx/src/core/globalstate.ts index 87efdb850..c47c2150f 100644 --- a/packages/mobx/src/core/globalstate.ts +++ b/packages/mobx/src/core/globalstate.ts @@ -11,7 +11,6 @@ const persistentKeys: (keyof MobXGlobals)[] = [ "computedRequiresReaction", "reactionRequiresObservable", "observableRequiresReaction", - "allowStateReads", "disableErrorBoundaries", "runId", "UNCHANGED", @@ -229,5 +228,6 @@ export function resetGlobalState() { globalState[key] = defaultGlobals[key] } } - globalState.allowStateChanges = !globalState.enforceActions + // TODO del + //globalState.allowStateChanges = !globalState.enforceActions }