configure({ safeDescriptors: false }) (#2761)

* safeDescriptors configuration option

* fix headings

* fix headings #2

.changeset/cuddly-pets-sort.md

@@ -0,0 +1,5 @@
+---
+"mobx": patch
+---
+
+`configure({ safeDescriptors: false })` now forces all props of observable objects to be writable and configurable

docs/configuration.md

@@ -29,7 +29,7 @@ configure({
 
 Accepted values for the `useProxies` configuration are:
 
--   `"always"` (default): MobX expects to run only in environments with [`Proxy` support](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy) and it will error if such an environment is not available.
+-   `"always"` (**default**): MobX expects to run only in environments with [`Proxy` support](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy) and it will error if such an environment is not available.
 -   `"never"`: Proxies are not used and MobX falls back on non-proxy alternatives. This is compatible with all ES5 environments, but causes various [limitations](#limitations-without-proxy-support).
 -   `"ifavailable"` (experimental): Proxies are used if they are available, and otherwise MobX falls back to non-proxy alternatives. The benefit of this mode is that MobX will try to warn if APIs or language features that wouldn't work in ES5 environments are used, triggering errors when hitting an ES5 limitation running on a modern environment.
 
@@ -78,7 +78,7 @@ The goal of _enforceActions_ is that you don't forget to wrap event handlers in
 
 Possible options:
 
--   `"observed"` (default): All state that is observed _somewhere_ needs to be changed through actions. This is the default, and the recommended strictness mode in non-trivial applications.
+-   `"observed"` (**default**): All state that is observed _somewhere_ needs to be changed through actions. This is the default, and the recommended strictness mode in non-trivial applications.
 -   `"never"`: State can be changed from anywhere.
 -   `"always"`: State always needs to be changed through actions, which in practice also includes creation.
 
@@ -91,7 +91,9 @@ In the rare case where you create observables lazily, for example in a computed
 #### `computedRequiresReaction: boolean`
 
 Forbids the direct access of any unobserved computed value from outside an action or reaction.
-This guarantees you aren't using computed values in a way where MobX won't cache them. In the following example, MobX won't cache the computed value in the first code block, but will cache the result in the second and third block:
+This guarantees you aren't using computed values in a way where MobX won't cache them. **Default: `false`**.
+
+In the following example, MobX won't cache the computed value in the first code block, but will cache the result in the second and third block:
 
 ```javascript
 class Clock {
@@ -133,7 +135,7 @@ const clock = new Clock()
 
 Warns about any unobserved observable access.
 Use this if you want to check whether you are using observables without a "MobX context".
-This is a great way to find any missing `observer` wrappers, for example in React components. But it will find missing actions as well.
+This is a great way to find any missing `observer` wrappers, for example in React components. But it will find missing actions as well. **Default: `false`**
 
 ```javascript
 configure({ observableRequiresReaction: true })
@@ -144,7 +146,7 @@ configure({ observableRequiresReaction: true })
 #### `reactionRequiresObservable: boolean`
 
 Warns when a reaction (e.g. `autorun`) is created without accessing any observables.
-Use this to check whether you are unnecessarily wrapping React components with `observer`, wrapping functions with `action`, or find cases where you simply forgot to make some data structures or properties observable.
+Use this to check whether you are unnecessarily wrapping React components with `observer`, wrapping functions with `action`, or find cases where you simply forgot to make some data structures or properties observable. **Default: `false`**
 
 ```javascript
 configure({ reactionRequiresObservable: true })
@@ -154,7 +156,8 @@ configure({ reactionRequiresObservable: true })
 
 By default, MobX will catch and re-throw exceptions happening in your code to make sure that a reaction in one exception does not prevent the scheduled execution of other, possibly unrelated, reactions. This means exceptions are not propagated back to the original causing code and therefore you won't be able to catch them using try/catch.
 
-By disabling error boundaries, exceptions can escape derivations. This might ease debugging, but might leave MobX and by extension your application in an unrecoverable broken state.
+By disabling error boundaries, exceptions can escape derivations. This might ease debugging, but might leave MobX and by extension your application in an unrecoverable broken state. **Default: `false`**.
+
 This option is great for unit tests, but remember to call `_resetGlobalState` after each test, for example by using `afterEach` in jest, for example:
 
 ```js
@@ -177,13 +180,25 @@ afterEach(() => {
 })
 ```
 
+#### `safeDescriptors: boolean`
+
+MobX makes some fields **non-configurable** or **non-writable** to prevent you from doing things that are not supported or would most likely break your code. However this can also prevent **spying/mocking/stubbing** in your tests.
+`configure({ safeDescriptors: false })` disables this safety measure, making everything **configurable** and **writable**.
+Note it doesn't affect existing observables, only the ones created after it's been configured.
+<span style="color:red">**Use with caution**</span> and only when needed - do not turn this off globally for all tests, otherwise you risk false positives (passing tests with broken code). **Default: `true`**
+
+```javascript
+configure({ safeDescriptors: false })
+```
+
 ## Further configuration options
 
 #### `isolateGlobalState: boolean`
 
 Isolates the global state of MobX when there are multiple instances of MobX active in the same environment. This is useful when you have an encapsulated library that is using MobX, living in the same page as the app that is using MobX. The reactivity inside the library will remain self-contained when you call `configure({ isolateGlobalState: true })` from it.
 
 Without this option, if multiple MobX instances are active, their internal state will be shared. The benefit is that observables from both instances work together, the downside is that the MobX versions have to match.
+**Default: `false`**
 
 ```javascript
 configure({ isolateGlobalState: true })
@@ -193,7 +208,7 @@ configure({ isolateGlobalState: true })
 
 Sets a new function that executes all MobX reactions.
 By default `reactionScheduler` just runs the `f` reaction without any other behavior.
-This can be useful for basic debugging, or slowing down reactions to visualize application updates.
+This can be useful for basic debugging, or slowing down reactions to visualize application updates. **Default: `f => f()`**
 
 ```javascript
 configure({

packages/mobx/__tests__/v5/base/observables.js

@@ -2092,3 +2092,48 @@ test("observable ignores class instances #2579", () => {
     const c = new C()
     expect(observable(c)).toBe(c)
 })
+
+test("configure({ safeDescriptors: false })", () => {
+    function checkDescriptors(thing) {
+        Reflect.ownKeys(thing).forEach(key => {
+            const { get, set, writable, configurable } = Object.getOwnPropertyDescriptor(thing, key)
+            expect(get || set || writable).toBeTruthy()
+            expect(configurable).toBe(true)
+        })
+    }
+
+    const globalState = mobx._getGlobalState()
+    expect(globalState.safeDescriptors).toBe(true)
+    mobx.configure({ safeDescriptors: false })
+    expect(globalState.safeDescriptors).toBe(false)
+
+    class Clazz {
+        observable = 0
+        action() {}
+        get computed() {}
+        *flow() {}
+        constructor() {
+            mobx.makeObservable(this, {
+                observable: mobx.observable,
+                action: mobx.action,
+                computed: mobx.computed,
+                flow: mobx.flow
+            })
+        }
+    }
+    checkDescriptors(Clazz.prototype)
+    const clazz = new Clazz()
+    checkDescriptors(clazz)
+
+    const plain = mobx.observable({
+        observable: 0,
+        action() {},
+        get computed() {},
+        *flow() {}
+    })
+
+    checkDescriptors(plain)
+
+    mobx.configure({ safeDescriptors: true })
+    expect(globalState.safeDescriptors).toBe(true)
+})

packages/mobx/src/api/configure.ts

@@ -43,7 +43,8 @@ export function configure(options: {
         "computedRequiresReaction",
         "reactionRequiresObservable",
         "observableRequiresReaction",
-        "disableErrorBoundaries"
+        "disableErrorBoundaries",
+        "safeDescriptors"
     ].forEach(key => {
         if (key in options) globalState[key] = !!options[key]
     })

packages/mobx/src/core/globalstate.ts

@@ -143,6 +143,13 @@ export class MobXGlobals {
      * print warnings about code that would fail if proxies weren't available
      */
     verifyProxies = false
+
+    /**
+     * False forces all object's descriptors to
+     * writable: true
+     * configurable: true
+     */
+    safeDescriptors = true
 }
 
 let canMergeGlobalState = true

packages/mobx/src/types/actionannotation.ts

@@ -8,7 +8,8 @@ import {
     die,
     isFunction,
     Annotation,
-    recordAnnotationApplied
+    recordAnnotationApplied,
+    globalState
 } from "../internal"
 
 export function createActionAnnotation(name: string, options?: object): Annotation {
@@ -113,11 +114,11 @@ function createActionDescriptor(
         ),
         // Non-configurable for classes
         // prevents accidental field redefinition in subclass
-        configurable: adm.isPlainObject_,
+        configurable: globalState.safeDescriptors ? adm.isPlainObject_ : true,
         // https://github.com/mobxjs/mobx/pull/2641#issuecomment-737292058
         enumerable: false,
         // Non-obsevable, therefore non-writable
         // Also prevents rewriting in subclass constructor
-        writable: false
+        writable: globalState.safeDescriptors ? false : true
     }
 }

packages/mobx/src/types/flowannotation.ts

@@ -8,7 +8,8 @@ import {
     flow,
     isFlow,
     recordAnnotationApplied,
-    isFunction
+    isFunction,
+    globalState
 } from "../internal"
 
 export function createFlowAnnotation(name: string, options?: object): Annotation {
@@ -95,11 +96,11 @@ function createFlowDescriptor(
         value: flow(descriptor.value),
         // Non-configurable for classes
         // prevents accidental field redefinition in subclass
-        configurable: adm.isPlainObject_,
+        configurable: globalState.safeDescriptors ? adm.isPlainObject_ : true,
         // https://github.com/mobxjs/mobx/pull/2641#issuecomment-737292058
         enumerable: false,
         // Non-obsevable, therefore non-writable
         // Also prevents rewriting in subclass constructor
-        writable: false
+        writable: globalState.safeDescriptors ? false : true
     }
 }

packages/mobx/src/types/observableobject.ts

@@ -403,7 +403,7 @@ export class ObservableObjectAdministration
 
             const cachedDescriptor = getCachedObservablePropDescriptor(key)
             const descriptor = {
-                configurable: this.isPlainObject_,
+                configurable: globalState.safeDescriptors ? this.isPlainObject_ : true,
                 enumerable: true,
                 get: cachedDescriptor.get,
                 set: cachedDescriptor.set
@@ -465,7 +465,7 @@ export class ObservableObjectAdministration
             options.context = this.proxy_ || this.target_
             const cachedDescriptor = getCachedObservablePropDescriptor(key)
             const descriptor = {
-                configurable: this.isPlainObject_,
+                configurable: globalState.safeDescriptors ? this.isPlainObject_ : true,
                 enumerable: false,
                 get: cachedDescriptor.get,
                 set: cachedDescriptor.set