feat: Add additional optional dev time checks

backport of #2079

flow-typed/mobx.js

@@ -5,6 +5,8 @@ export type IObservableMapInitialValues<K, V> = IMapEntries<K, V> | KeyValueMap<
 export interface IMobxConfigurationOptions {
     +enforceActions?: boolean | "strict" | "never" | "always" | "observed";
     computedRequiresReaction?: boolean;
+    reactionRequiresObservable?: boolean;
+    observableRequiresReaction?: boolean;
     isolateGlobalState?: boolean;
     disableErrorBoundaries?: boolean;
     arrayBuffer?: number;
@@ -16,6 +18,10 @@ declare export function configure(options: IMobxConfigurationOptions): void
 export interface IAutorunOptions {
     delay?: number;
     name?: string;
+    /**
+     *  warn if the derivation has no dependencies after creation/update
+     */
+    requiresObservable?: boolean;
     scheduler?: (callback: () => void) => any;
     onError?: (error: any) => void;
 }

src/api/autorun.ts

@@ -16,6 +16,11 @@ import {
 export interface IAutorunOptions {
     delay?: number
     name?: string
+    /**
+     * Experimental.
+     * Warns if the view doesn't track observables
+     */
+    requiresObservable?: boolean
     scheduler?: (callback: () => void) => any
     onError?: (error: any) => void
 }
@@ -49,7 +54,8 @@ export function autorun(
             function(this: Reaction) {
                 this.track(reactionRunner)
             },
-            opts.onError
+            opts.onError,
+            opts.requiresObservable
         )
     } else {
         const scheduler = createSchedulerFromOptions(opts)
@@ -67,7 +73,8 @@ export function autorun(
                     })
                 }
             },
-            opts.onError
+            opts.onError,
+            opts.requiresObservable
         )
     }
 
@@ -138,7 +145,8 @@ export function reaction<T>(
                 scheduler!(reactionRunner)
             }
         },
-        opts.onError
+        opts.onError,
+        opts.requiresObservable
     )
 
     function reactionRunner() {

src/api/configure.ts

@@ -10,6 +10,16 @@ import {
 export function configure(options: {
     enforceActions?: boolean | "strict" | "never" | "always" | "observed"
     computedRequiresReaction?: boolean
+    /**
+     * (Experimental)
+     * Warn if you try to create to derivation / reactive context without accessing any observable.
+     */
+    reactionRequiresObservable?: boolean
+    /**
+     * (Experimental)
+     * Warn if observables are accessed outside a reactive context
+     */
+    observableRequiresReaction?: boolean
     computedConfigurable?: boolean
     isolateGlobalState?: boolean
     disableErrorBoundaries?: boolean
@@ -22,7 +32,9 @@ export function configure(options: {
         computedConfigurable,
         disableErrorBoundaries,
         arrayBuffer,
-        reactionScheduler
+        reactionScheduler,
+        reactionRequiresObservable,
+        observableRequiresReaction
     } = options
     if (options.isolateGlobalState === true) {
         isolateGlobalState()
@@ -57,6 +69,14 @@ export function configure(options: {
     if (computedRequiresReaction !== undefined) {
         globalState.computedRequiresReaction = !!computedRequiresReaction
     }
+    if (reactionRequiresObservable !== undefined) {
+        globalState.reactionRequiresObservable = !!reactionRequiresObservable
+    }
+    if (observableRequiresReaction !== undefined) {
+        globalState.observableRequiresReaction = !!observableRequiresReaction
+
+        globalState.allowStateReads = !globalState.observableRequiresReaction
+    }
     if (computedConfigurable !== undefined) {
         globalState.computedConfigurable = !!computedConfigurable
     }

src/api/when.ts

@@ -3,6 +3,11 @@ import { Lambda, fail, getNextId, IReactionDisposer, createAction, autorun } fro
 export interface IWhenOptions {
     name?: string
     timeout?: number
+    /**
+     * Experimental.
+     * Warns if the view doesn't track observables
+     */
+    requiresObservable?: boolean
     onError?: (error: any) => void
 }
 

src/core/action.ts

@@ -11,6 +11,7 @@ import {
     untrackedEnd,
     spyReportEnd
 } from "../internal"
+import { allowStateReadsStart, allowStateReadsEnd } from "./derivation"
 
 export interface IAction {
     isMobxAction: boolean
@@ -44,6 +45,7 @@ export function executeAction(actionName: string, fn: Function, scope?: any, arg
 export interface IActionRunInfo {
     prevDerivation: IDerivation | null
     prevAllowStateChanges: boolean
+    prevAllowStateReads: boolean
     notifySpy: boolean
     startTime: number
     error?: any
@@ -69,9 +71,11 @@ export function _startAction(actionName: string, scope: any, args?: IArguments):
     const prevDerivation = untrackedStart()
     startBatch()
     const prevAllowStateChanges = allowStateChangesStart(true)
+    const prevAllowStateReads = allowStateReadsStart(true)
     const runInfo = {
         prevDerivation,
         prevAllowStateChanges,
+        prevAllowStateReads,
         notifySpy,
         startTime,
         actionId: globalState.nextActionId++,
@@ -91,6 +95,7 @@ export function _endAction(runInfo: IActionRunInfo) {
         globalState.suppressReactionErrors = true
     }
     allowStateChangesEnd(runInfo.prevAllowStateChanges)
+    allowStateReadsEnd(runInfo.prevAllowStateReads)
     endBatch()
     untrackedEnd(runInfo.prevDerivation)
     if (runInfo.notifySpy) {

src/core/derivation.ts

@@ -55,6 +55,11 @@ export interface IDerivation extends IDepTreeNode {
     __mapid: string
     onBecomeStale(): void
     isTracing: TraceMode
+
+    /**
+     *  warn if the derivation has no dependencies after creation/update
+     */
+    requiresObservable?: boolean
 }
 
 export class CaughtException {
@@ -155,12 +160,23 @@ export function checkIfStateModificationsAreAllowed(atom: IAtom) {
         )
 }
 
+export function checkIfStateReadsAreAllowed(observable: IObservable) {
+    if (
+        process.env.NODE_ENV !== "production" &&
+        !globalState.allowStateReads &&
+        globalState.observableRequiresReaction
+    ) {
+        console.warn(`[mobx] Observable ${observable.name} being read outside a reactive context`)
+    }
+}
+
 /**
  * Executes the provided function `f` and tracks which observables are being accessed.
  * The tracking information is stored on the `derivation` object and the derivation is registered
  * as observer of any of the accessed observables.
  */
 export function trackDerivedFunction<T>(derivation: IDerivation, f: () => T, context: any) {
+    const prevAllowStateReads = allowStateReadsStart(true)
     // pre allocate array allocation + room for variation in deps
     // array will be trimmed by bindDependencies
     changeDependenciesStateTo0(derivation)
@@ -181,9 +197,27 @@ export function trackDerivedFunction<T>(derivation: IDerivation, f: () => T, con
     }
     globalState.trackingDerivation = prevTracking
     bindDependencies(derivation)
+
+    if (derivation.observing.length === 0) {
+        warnAboutDerivationWithoutDependencies(derivation)
+    }
+
+    allowStateReadsEnd(prevAllowStateReads)
+
     return result
 }
 
+function warnAboutDerivationWithoutDependencies(derivation: IDerivation) {
+    if (process.env.NODE_ENV === "production") return
+    if (globalState.reactionRequiresObservable || derivation.requiresObservable) {
+        console.warn(
+            `[mobx] Derivation ${
+                derivation.name
+            } is created/updated without reading any observable value`
+        )
+    }
+}
+
 /**
  * diffs newObserving with observing.
  * update observing to be newObserving with unique observables
@@ -276,6 +310,16 @@ export function untrackedEnd(prev: IDerivation | null) {
     globalState.trackingDerivation = prev
 }
 
+export function allowStateReadsStart(allowStateReads: boolean) {
+    const prev = globalState.allowStateReads
+    globalState.allowStateReads = allowStateReads
+    return prev
+}
+
+export function allowStateReadsEnd(prev: boolean) {
+    globalState.allowStateReads = prev
+}
+
 /**
  * needed to keep `lowestObserverState` correct. when changing from (2 or 1) to 0
  *

src/core/globalstate.ts

@@ -8,6 +8,9 @@ const persistentKeys: (keyof MobXGlobals)[] = [
     "spyListeners",
     "enforceActions",
     "computedRequiresReaction",
+    "reactionRequiresObservable",
+    "observableRequiresReaction",
+    "allowStateReads",
     "disableErrorBoundaries",
     "runId",
     "UNCHANGED"
@@ -81,6 +84,12 @@ export class MobXGlobals {
      */
     allowStateChanges = true
 
+    /**
+     * Is it allowed to read observables at this point?
+     * Used to hold the state needed for `observableRequiresReaction`
+     */
+    allowStateReads = true
+
     /**
      * If strict mode is enabled, state changes are by default not allowed
      */
@@ -101,6 +110,18 @@ export class MobXGlobals {
      */
     computedRequiresReaction = false
 
+    /**
+     * (Experimental)
+     * Warn if you try to create to derivation / reactive context without accessing any observable.
+     */
+    reactionRequiresObservable = false
+
+    /**
+     * (Experimental)
+     * Warn if observables are accessed outside a reactive context
+     */
+    observableRequiresReaction = 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

src/core/observable.ts

@@ -6,7 +6,8 @@ import {
     runReactions,
     ComputedValue,
     getDependencyTree,
-    IDependencyTree
+    IDependencyTree,
+    checkIfStateReadsAreAllowed
 } from "../internal"
 
 export interface IDepTreeNode {
@@ -153,6 +154,8 @@ export function endBatch() {
 }
 
 export function reportObserved(observable: IObservable): boolean {
+    checkIfStateReadsAreAllowed(observable)
+
     const derivation = globalState.trackingDerivation
     if (derivation !== null) {
         /**
@@ -172,6 +175,7 @@ export function reportObserved(observable: IObservable): boolean {
     } else if (observable.observers.length === 0 && globalState.inBatch > 0) {
         queueForUnobservation(observable)
     }
+
     return false
 }
 

src/core/reaction.ts

@@ -66,7 +66,8 @@ export class Reaction implements IDerivation, IReactionPublic {
     constructor(
         public name: string = "Reaction@" + getNextId(),
         private onInvalidate: () => void,
-        private errorHandler?: (error: any, derivation: IDerivation) => void
+        private errorHandler?: (error: any, derivation: IDerivation) => void,
+        public requiresObservable = false
     ) {}
 
     onBecomeStale() {