Skip to content

Commit

Permalink
feat: Add additional optional dev time checks
Browse files Browse the repository at this point in the history
  • Loading branch information
Bnaya committed Aug 19, 2019
1 parent 2bce97d commit bc95364
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 4 deletions.
2 changes: 2 additions & 0 deletions flow-typed/mobx.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ export type IObservableMapInitialValues<K, V> = IMapEntries<K, V> | KeyValueMap<
export interface IMobxConfigurationOptions {
+enforceActions?: boolean | "strict" | "never" | "always" | "observed";
computedRequiresReaction?: boolean;
derivationRequiresObservable?: boolean;
observablesRequiresReaction?: boolean;
isolateGlobalState?: boolean;
disableErrorBoundaries?: boolean;
arrayBuffer?: number;
Expand Down
12 changes: 11 additions & 1 deletion src/api/configure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
export function configure(options: {
enforceActions?: boolean | "strict" | "never" | "always" | "observed"
computedRequiresReaction?: boolean
derivationRequiresObservable?: boolean
observablesRequiresReaction?: boolean
computedConfigurable?: boolean
isolateGlobalState?: boolean
disableErrorBoundaries?: boolean
Expand All @@ -19,7 +21,9 @@ export function configure(options: {
computedRequiresReaction,
computedConfigurable,
disableErrorBoundaries,
reactionScheduler
reactionScheduler,
derivationRequiresObservable,
observablesRequiresReaction
} = options
if (options.isolateGlobalState === true) {
isolateGlobalState()
Expand Down Expand Up @@ -54,6 +58,12 @@ export function configure(options: {
if (computedRequiresReaction !== undefined) {
globalState.computedRequiresReaction = !!computedRequiresReaction
}
if (derivationRequiresObservable !== undefined) {
globalState.derivationRequiresObservable = !!derivationRequiresObservable
}
if (observablesRequiresReaction !== undefined) {
globalState.observablesRequiresReaction = !!observablesRequiresReaction
}
if (computedConfigurable !== undefined) {
globalState.computedConfigurable = !!computedConfigurable
}
Expand Down
12 changes: 12 additions & 0 deletions src/core/derivation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,21 @@ export function trackDerivedFunction<T>(derivation: IDerivation, f: () => T, con
}
globalState.trackingDerivation = prevTracking
bindDependencies(derivation)

if (derivation.observing.length === 0) {
warnAboutDerivationWithoutDependencies(derivation.name)
}

return result
}

function warnAboutDerivationWithoutDependencies(name: string) {
if (process.env.NODE_ENV === "production") return
if (globalState.derivationRequiresObservable) {
console.warn(`[mobx] Derivation ${name} is created without reading any observable value`)
}
}

/**
* diffs newObserving with observing.
* update observing to be newObserving with unique observables
Expand Down
10 changes: 10 additions & 0 deletions src/core/globalstate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,16 @@ export class MobXGlobals {
*/
computedRequiresReaction = false

/**
* Warn if you try to create to derivation / reactive context without accessing any observable.
*/
derivationRequiresObservable = false

/**
* Warn if observables are accessed outside a reactive context
*/
observablesRequiresReaction = false

/**
* Allows overwriting of computed properties, useful in tests but not prod as it can cause
* memory leaks. See https://github.com/mobxjs/mobx/issues/1867
Expand Down
14 changes: 12 additions & 2 deletions src/core/observable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,11 @@ export function reportObserved(observable: IObservable): boolean {
}
}
return true
} else if (observable.observers.size === 0 && globalState.inBatch > 0) {
queueForUnobservation(observable)
} else if (observable.observers.size === 0) {
if (globalState.inBatch > 0) {
queueForUnobservation(observable)
}
warnAboutUntrackedObservableRead(observable.name)
}
return false
}
Expand Down Expand Up @@ -266,3 +269,10 @@ function printDepTree(tree: IDependencyTree, lines: string[], depth: number) {
lines.push(`${new Array(depth).join("\t")}${tree.name}`) // MWE: not the fastest, but the easiest way :)
if (tree.dependencies) tree.dependencies.forEach(child => printDepTree(child, lines, depth + 1))
}

function warnAboutUntrackedObservableRead(name: string) {
if (process.env.NODE_ENV === "production") return
if (globalState.observablesRequiresReaction) {
console.warn(`[mobx] Observable ${name} being read outside a reactive context`)
}
}
46 changes: 45 additions & 1 deletion test/base/strict-mode.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/**
* @type {typeof import("../../src/mobx")}
*/
const mobx = require("../../src/mobx.ts")
const utils = require("../utils/test-utils")

Expand Down Expand Up @@ -224,7 +227,7 @@ test("enforceActions 'strict' should not throw exception while observable array
}
})

test("warn on unsafe reads", function() {
test("warn on unsafe reads of computed", function() {
try {
mobx.configure({ computedRequiresReaction: true })
const x = mobx.observable({
Expand All @@ -240,6 +243,47 @@ test("warn on unsafe reads", function() {
mobx.configure({ computedRequiresReaction: false })
}
})
test("warn on unsafe reads of observable 1", function() {
try {
mobx.configure({ observablesRequiresReaction: true })
const x = mobx.observable({
y: 3
})
utils.consoleWarn(() => {
x.yy
}, /being read outside a reactive context/)
} finally {
mobx.configure({ observablesRequiresReaction: false })
}
})

test("warn on unsafe reads of observable array", function() {
try {
mobx.configure({ observablesRequiresReaction: true })
const x = mobx.observable({
arr: [1, 2, 3]
})
utils.consoleWarn(() => {
x.arr[1]
}, /being read outside a reactive context/)
} finally {
mobx.configure({ observablesRequiresReaction: false })
}
})

test("warn on derivation creation without dependencies", function() {
try {
mobx.configure({ derivationRequiresObservable: true })

utils.consoleWarn(() => {
const dispose = mobx.reaction(() => "plain value", newValue => newValue)

dispose()
}, /is created without reading any observable value/)
} finally {
mobx.configure({ derivationRequiresObservable: false })
}
})

test("#1869", function() {
const x = mobx.observable.box(3)
Expand Down

0 comments on commit bc95364

Please sign in to comment.