extendObservable: remove constraint for existing property

CHANGELOG.md

@@ -1,3 +1,7 @@
+# 5.15.3 / 4.15.3 [NOT PUBLISHED YET]
+
+-   Fixes #2250 - extendObservable: allows it to be used on an existing property [#2250](https://github.com/mobxjs/mobx/pull/2250)
+
 # 5.15.2 / 4.15.2 [NOT PUBLISHED YET]
 
 -   Fixes #2230 - computedvalue: throw error object instead of string when options is empty [#2243](https://github.com/mobxjs/mobx/pull/2243)

docs/refguide/api.md

@@ -116,7 +116,7 @@ The api is further the same as the [ES6 set api](https://developer.mozilla.org/e
 
 Usage: `extendObservable(target, properties, decorators?, options?)`.
 
-For each key/value pair in each `properties` a (new) observable property will be introduced on the target object.
+For each key/value pair in each `properties` a (new) observable property will be introduced on the target object if the property does not already exist on the target object. If the property does exist, it will be made observable.
 This can be used in constructor functions to introduce observable properties without using decorators.
 If a value of the `properties` is a getter function, a _computed_ property will be introduced.
 

src/api/extendobservable.ts

@@ -86,12 +86,6 @@ export function extendObservableObjectWithProperties(
             if (process.env.NODE_ENV !== "production") {
                 if (!isPlainObject(properties))
                     fail(`'extendObservabe' only accepts plain objects as second argument`)
-                if (Object.getOwnPropertyDescriptor(target, key))
-                    fail(
-                        `'extendObservable' can only be used to introduce new properties. Use 'set' or 'decorate' instead. The property '${stringifyKey(
-                            key
-                        )}' already exists on '${target}'`
-                    )
                 if (isComputed(descriptor.value))
                     fail(
                         `Passing a 'computed' as initial property value is no longer supported by extendObservable. Use a getter or decorator instead`

test/base/extendObservable.js

@@ -0,0 +1,119 @@
+// @ts-check
+
+import {
+    computed,
+    autorun,
+    isObservable,
+    isObservableProp,
+    isComputedProp,
+    extendObservable
+} from "../../src/mobx"
+
+test("extendObservable should work", function() {
+    class Box {
+        // @ts-ignore
+        uninitialized
+        height = 20
+        sizes = [2]
+        someFunc = function() {
+            return 2
+        }
+        get width() {
+            return (
+                this.undeclared *
+                this.height *
+                this.sizes.length *
+                this.someFunc() *
+                (this.uninitialized ? 2 : 1)
+            )
+        }
+        addSize() {
+            // @ts-ignore
+            this.sizes.push([3])
+            // @ts-ignore
+            this.sizes.push([4])
+        }
+        constructor() {
+            this.undeclared = 1
+        }
+    }
+
+    const box = new Box()
+
+    extendObservable(box, {
+        height: 20,
+        sizes: [2],
+        get someFunc() {
+            return 2
+        },
+        width: 40
+    })
+
+    expect(isObservableProp(box, "height")).toBe(true)
+    expect(isObservableProp(box, "sizes")).toBe(true)
+    expect(isObservable(box.sizes)).toBe(true)
+    expect(isObservableProp(box, "someFunc")).toBe(true)
+    expect(isComputedProp(box, "someFunc")).toBe(true)
+    expect(isObservableProp(box, "width")).toBe(true)
+
+    const ar = []
+
+    autorun(() => {
+        ar.push(box.width)
+    })
+
+    expect(ar.slice()).toEqual([40])
+})
+
+test("extendObservable should work with plain object", function() {
+    const box = {
+        /** @type {boolean | undefined} */
+        uninitialized: undefined,
+        height: 20,
+        sizes: [2],
+        someFunc: function() {
+            return 2
+        },
+        get width() {
+            return (
+                this.undeclared *
+                this.height *
+                this.sizes.length *
+                this.someFunc() *
+                (this.uninitialized ? 2 : 1)
+            )
+        },
+        addSize() {
+            // @ts-ignore
+            this.sizes.push([3])
+            // @ts-ignore
+            this.sizes.push([4])
+        }
+    }
+
+    box.undeclared = 1
+
+    extendObservable(box, {
+        height: 20,
+        sizes: [2],
+        get someFunc() {
+            return 2
+        },
+        width: 40
+    })
+
+    expect(isObservableProp(box, "height")).toBe(true)
+    expect(isObservableProp(box, "sizes")).toBe(true)
+    expect(isObservable(box.sizes)).toBe(true)
+    expect(isObservableProp(box, "someFunc")).toBe(true)
+    expect(isComputedProp(box, "someFunc")).toBe(true)
+    expect(isObservableProp(box, "width")).toBe(true)
+
+    const ar = []
+
+    autorun(() => {
+        ar.push(box.width)
+    })
+
+    expect(ar.slice()).toEqual([40])
+})

test/base/makereactive.js

@@ -228,7 +228,11 @@ test("observable5", function() {
         return 3
     }
     x.nonReactive = three
-    expect(b.toArray()).toEqual([[17, f, 17], [18, f, 18], [18, three, 3]])
+    expect(b.toArray()).toEqual([
+        [17, f, 17],
+        [18, f, 18],
+        [18, three, 3]
+    ])
 })
 
 test("flat array", function() {
@@ -650,11 +654,6 @@ test("double declare property", () => {
     mobx.extendObservable(o, {
         a: 5
     })
-    expect(() => {
-        mobx.extendObservable(o, {
-            a: 3
-        })
-    }).toThrow(/can only be used to introduce new properties/)
     expect(() => {
         mobx.extendObservable(
             o,

test/base/observables.js

@@ -1344,7 +1344,11 @@ test("atom events #427", () => {
     let stop = 0
     let runs = 0
 
-    const a = mobx.createAtom("test", () => start++, () => stop++)
+    const a = mobx.createAtom(
+        "test",
+        () => start++,
+        () => stop++
+    )
     expect(a.reportObserved()).toEqual(false)
 
     expect(start).toBe(0)
@@ -1682,12 +1686,18 @@ test("computed equals function only invoked when necessary", () => {
 
         // Another value change will cause a comparison
         right.set("F")
-        expect(comparisons).toEqual([{ from: "ab", to: "cb" }, { from: "de", to: "df" }])
+        expect(comparisons).toEqual([
+            { from: "ab", to: "cb" },
+            { from: "de", to: "df" }
+        ])
 
         // Becoming unobserved, then observed won't cause a comparison
         disposeAutorun()
         disposeAutorun = mobx.autorun(() => values.push(combinedToLowerCase.get()))
-        expect(comparisons).toEqual([{ from: "ab", to: "cb" }, { from: "de", to: "df" }])
+        expect(comparisons).toEqual([
+            { from: "ab", to: "cb" },
+            { from: "de", to: "df" }
+        ])
 
         expect(values).toEqual(["ab", "cb", "de", "df", "df"])
 
@@ -1754,23 +1764,6 @@ test("Issue 1120 - isComputed should return false for a non existing property",
     expect(mobx.isComputedProp(observable({}), "x")).toBe(false)
 })
 
-test("It should not be possible to redefine a computed property", () => {
-    const a = observable({
-        width: 10,
-        get surface() {
-            return this.width
-        }
-    })
-
-    expect(() => {
-        mobx.extendObservable(a, {
-            get surface() {
-                return this.width * 2
-            }
-        })
-    }).toThrow(/'extendObservable' can only be used to introduce new properties/)
-})
-
 test("extendObservable should not be able to set a computed property", () => {
     expect(() => {
         observable({
@@ -1817,7 +1810,10 @@ test("computed comparer works with decorate (plain)", () => {
     time.minute = 0
     expect(changes).toEqual([{ hour: 9, minute: 0 }])
     time.hour = 10
-    expect(changes).toEqual([{ hour: 9, minute: 0 }, { hour: 10, minute: 0 }])
+    expect(changes).toEqual([
+        { hour: 9, minute: 0 },
+        { hour: 10, minute: 0 }
+    ])
     time.minute = 30
     expect(changes).toEqual([
         { hour: 9, minute: 0 },
@@ -1856,7 +1852,10 @@ test("computed comparer works with decorate (plain) - 2", () => {
     time.minute = 0
     expect(changes).toEqual([{ hour: 9, minute: 0 }])
     time.hour = 10
-    expect(changes).toEqual([{ hour: 9, minute: 0 }, { hour: 10, minute: 0 }])
+    expect(changes).toEqual([
+        { hour: 9, minute: 0 },
+        { hour: 10, minute: 0 }
+    ])
     time.minute = 30
     expect(changes).toEqual([
         { hour: 9, minute: 0 },
@@ -1891,7 +1890,10 @@ test("computed comparer works with decorate (plain) - 3", () => {
     time.minute = 0
     expect(changes).toEqual([{ hour: 9, minute: 0 }])
     time.hour = 10
-    expect(changes).toEqual([{ hour: 9, minute: 0 }, { hour: 10, minute: 0 }])
+    expect(changes).toEqual([
+        { hour: 9, minute: 0 },
+        { hour: 10, minute: 0 }
+    ])
     time.minute = 30
     expect(changes).toEqual([
         { hour: 9, minute: 0 },
@@ -2073,7 +2075,10 @@ test("tuples", () => {
     const myStuff = tuple(1, 3)
     const events = []
 
-    mobx.reaction(() => myStuff[0], val => events.push(val))
+    mobx.reaction(
+        () => myStuff[0],
+        val => events.push(val)
+    )
     myStuff[1] = 17 // should not react
     myStuff[0] = 2 // should react
     expect(events).toEqual([2])

test/base/proxies.js

@@ -20,7 +20,11 @@ test("should react to key removal (unless reconfiguraing to empty) - 1", () => {
         z: 1
     })
 
-    reaction(() => Object.keys(x), keys => events.push(keys.join(",")), { fireImmediately: true })
+    reaction(
+        () => Object.keys(x),
+        keys => events.push(keys.join(",")),
+        { fireImmediately: true }
+    )
     expect(events).toEqual(["y,z"])
     delete x.y
     expect(events).toEqual(["y,z", "z"])
@@ -36,7 +40,10 @@ test("should react to key removal (unless reconfiguraing to empty) - 2", () => {
         z: 1
     })
 
-    reaction(() => x.z, v => events.push(v))
+    reaction(
+        () => x.z,
+        v => events.push(v)
+    )
 
     delete x.z
     expect(events).toEqual([undefined])
@@ -49,7 +56,10 @@ test("should react to key removal (unless reconfiguraing to empty) - 2", () => {
         z: undefined
     })
 
-    reaction(() => x.z, v => events.push(v))
+    reaction(
+        () => x.z,
+        v => events.push(v)
+    )
 
     delete x.z
     expect(events).toEqual([])
@@ -59,7 +69,10 @@ test("should react to future key additions - 1", () => {
     const events = []
     const x = observable.object({})
 
-    reaction(() => Object.keys(x), keys => events.push(keys.join(",")))
+    reaction(
+        () => Object.keys(x),
+        keys => events.push(keys.join(","))
+    )
 
     x.y = undefined
     expect(events).toEqual(["y"])
@@ -88,18 +101,6 @@ test("should react to future key additions - 2", () => {
     expect(events).toEqual([4])
 })
 
-test("should throw clear warning if trying to add computed to already reserved key", () => {
-    const x = observable.object({ z: 3 })
-
-    expect(() => {
-        extendObservable(x, {
-            get z() {
-                return 3
-            }
-        })
-    }).toThrow(/can only be used to introduce new properties/)
-})
-
 test("correct keys are reported", () => {
     const x = observable.object({
         x: 1,
@@ -119,7 +120,11 @@ test("correct keys are reported", () => {
 
     expect(Object.keys(x)).toEqual(["x", "z", "a"])
     expect(Object.values(x)).toEqual([1, 3, 4])
-    expect(Object.entries(x)).toEqual([["x", 1], ["z", 3], ["a", 4]])
+    expect(Object.entries(x)).toEqual([
+        ["x", 1],
+        ["z", 3],
+        ["a", 4]
+    ])
 
     expect(Object.getOwnPropertyNames(x)).toEqual(["x", "y", "z", "a", "b"])
     expect(keys(x)).toEqual(["x", "z", "a"])