Skip to content

Commit

Permalink
Warning severity config option
Browse files Browse the repository at this point in the history
  • Loading branch information
emereum committed Feb 24, 2023
1 parent faf075c commit cdc6ede
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 17 deletions.
5 changes: 5 additions & 0 deletions .changeset/green-socks-push.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"mobx": patch
---

Introduce warningSeverity configuration option. This allows consumers to promote specific warnings to exceptions.
16 changes: 16 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,19 @@ configure({
}
})
```

#### `warningSeverity: object`

MobX reports common development issues using `console.warn`. This configuration option allows you to promote specific warnings to `throw` rather than `warn`. **Default: `{ [key: string]: 'warn' }`**

```javascript
configure({
warningSeverity: {
computedRequiresReaction: "throw",
enforceActionsStrict: "throw",
enforceActionsNonStrict: "throw",
observableRequiresReaction: "throw",
derivationWithoutDependencies: "throw"
}
})
```
26 changes: 26 additions & 0 deletions packages/mobx/__tests__/v5/base/warnings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { onBecomeUnobserved, configure, computed, _resetGlobalState } from "../../../src/mobx"

describe("Configuring warning severity", () => {
let consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation()

beforeEach(() => _resetGlobalState())

it("Should default to console.warn when not configured", () => {
const a = computed(() => console.log("b"), { requiresReaction: true })
onBecomeUnobserved(a, () => console.log("c"))
a.get()

expect(consoleWarnSpy).toHaveBeenCalled()
})

it("Should throw an exception when configured with 'throw'", () => {
configure({ warningSeverity: { computedRequiresReaction: "throw" } })

const a = computed(() => console.log("b"), { requiresReaction: true })
onBecomeUnobserved(a, () => console.log("c"))

expect(() => a.get()).toThrow(
`[mobx] Computed value 'ComputedValue@2' is being read outside a reactive context. Doing a full recompute.`
)
})
})
5 changes: 5 additions & 0 deletions packages/mobx/src/api/configure.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { globalState, isolateGlobalState, setReactionScheduler } from "../internal"
import { WarningSeverity } from "../warnings"

const NEVER = "never"
const ALWAYS = "always"
Expand All @@ -16,6 +17,7 @@ export function configure(options: {
* Warn if observables are accessed outside a reactive context
*/
observableRequiresReaction?: boolean
warningSeverity?: WarningSeverity
isolateGlobalState?: boolean
disableErrorBoundaries?: boolean
safeDescriptors?: boolean
Expand Down Expand Up @@ -59,6 +61,9 @@ export function configure(options: {
"WARNING: Debug feature only. MobX will NOT recover from errors when `disableErrorBoundaries` is enabled."
)
}
if (options.warningSeverity) {
globalState.warningSeverity = { ...globalState.warningSeverity, ...options.warningSeverity }
}
if (options.reactionScheduler) {
setReactionScheduler(options.reactionScheduler)
}
Expand Down
5 changes: 2 additions & 3 deletions packages/mobx/src/core/computedvalue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
allowStateChangesStart,
allowStateChangesEnd
} from "../internal"
import { warn } from "../warnings"

export interface IComputedValue<T> {
get(): T
Expand Down Expand Up @@ -313,9 +314,7 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
? this.requiresReaction_
: globalState.computedRequiresReaction
) {
console.warn(
`[mobx] Computed value '${this.name_}' is being read outside a reactive context. Doing a full recompute.`
)
warn("computedRequiresReaction", this.name_)
}
}

Expand Down
19 changes: 6 additions & 13 deletions packages/mobx/src/core/derivation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
isComputedValue,
removeObserver
} from "../internal"
import { warn } from "../warnings"

export enum IDerivationState_ {
// before being run or (outside batch and not being observed)
Expand Down Expand Up @@ -141,21 +142,15 @@ export function checkIfStateModificationsAreAllowed(atom: IAtom) {
!globalState.allowStateChanges &&
(hasObservers || globalState.enforceActions === "always")
) {
console.warn(
"[MobX] " +
(globalState.enforceActions
? "Since strict-mode is enabled, changing (observed) observable values without using an action is not allowed. Tried to modify: "
: "Side effects like changing state are not allowed at this point. Are you trying to modify state from, for example, a computed value or the render function of a React component? You can wrap side effects in 'runInAction' (or decorate functions with 'action') if needed. Tried to modify: ") +
atom.name_
)
globalState.enforceActions
? warn("enforceActionsStrict", atom.name_)
: warn("enforceActionsNonStrict", atom.name_)
}
}

export function checkIfStateReadsAreAllowed(observable: IObservable) {
if (__DEV__ && !globalState.allowStateReads && globalState.observableRequiresReaction) {
console.warn(
`[mobx] Observable '${observable.name_}' being read outside a reactive context.`
)
warn("observableRequiresReaction", observable.name_)
}
}

Expand Down Expand Up @@ -208,9 +203,7 @@ function warnAboutDerivationWithoutDependencies(derivation: IDerivation) {
? derivation.requiresObservable_
: globalState.reactionRequiresObservable
) {
console.warn(
`[mobx] Derivation '${derivation.name_}' is created/updated without reading any observable value.`
)
warn("derivationWithoutDependencies", derivation.name_)
}
}

Expand Down
8 changes: 8 additions & 0 deletions packages/mobx/src/core/globalstate.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { IDerivation, IObservable, Reaction, die, getGlobal } from "../internal"
import { ComputedValue } from "./computedvalue"
import { WarningSeverity } from "../warnings"

/**
* These values will persist if global state is reset
Expand All @@ -11,6 +12,7 @@ const persistentKeys: (keyof MobXGlobals)[] = [
"computedRequiresReaction",
"reactionRequiresObservable",
"observableRequiresReaction",
"warningSeverity",
"allowStateReads",
"disableErrorBoundaries",
"runId",
Expand Down Expand Up @@ -126,6 +128,12 @@ export class MobXGlobals {
*/
observableRequiresReaction = false

/**
* Allows consumers to override the behaviour when a warning occurs. This can include promoting console warnings
* to errors.
*/
warningSeverity: WarningSeverity = {}

/*
* Don't catch and rethrow exceptions. This is useful for inspecting the state of
* the stack when an exception occurs while debugging.
Expand Down
3 changes: 2 additions & 1 deletion packages/mobx/src/types/observablemap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ export type IObservableMapInitialValues<K = any, V = any> =
// just extend Map? See also https://gist.github.com/nestharus/13b4d74f2ef4a2f4357dbd3fc23c1e54
// But: https://github.com/mobxjs/mobx/issues/1556
export class ObservableMap<K = any, V = any>
implements Map<K, V>, IInterceptable<IMapWillChange<K, V>>, IListenable {
implements Map<K, V>, IInterceptable<IMapWillChange<K, V>>, IListenable
{
[$mobx] = ObservableMapMarker
data_: Map<K, ObservableValue<V>>
hasMap_: Map<K, ObservableValue<boolean>> // hasMap, not hashMap >-).
Expand Down
33 changes: 33 additions & 0 deletions packages/mobx/src/warnings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { globalState } from "./internal"

const warnings = {
computedRequiresReaction: (name: string) =>
`[mobx] Computed value '${name}' is being read outside a reactive context. Doing a full recompute.`,

enforceActionsStrict: (name: string) =>
`[MobX] Since strict-mode is enabled, changing (observed) observable values without using an action is not ` +
`allowed. Tried to modify: ${name}`,

enforceActionsNonStrict: (name: string) =>
`[MobX] Side effects like changing state are not allowed at this point. Are you trying to modify state from, ` +
`for example, a computed value or the render function of a React component? You can wrap side effects in ` +
`'runInAction' (or decorate functions with 'action') if needed. Tried to modify: ${name}`,

observableRequiresReaction: (name: string) =>
`[mobx] Observable '${name}' being read outside a reactive context.`,

derivationWithoutDependencies: (name: string) =>
`[mobx] Derivation '${name}' is created/updated without reading any observable value.`
}
type Warnings = typeof warnings
export type WarningSeverity = { [k in keyof Warnings]?: "warn" | "throw" }

export function warn<K extends keyof Warnings>(warning: K, ...args: Parameters<Warnings[K]>) {
const message = (warnings[warning] as (...args: any[]) => string).call(null, args)

if (globalState.warningSeverity[warning] === "throw") {
throw new Error(message)
} else {
console.warn(message)
}
}

0 comments on commit cdc6ede

Please sign in to comment.