extendObservable: remove constraint for existing property

CHANGELOG.md

@@ -3,6 +3,8 @@
 -   Fixes #2230 - computedvalue: throw error object instead of string when options is empty [#2243](https://github.com/mobxjs/mobx/pull/2243)
 -   supports ES6 Sets and Maps in shallow comparer. [#2238](https://github.com/mobxjs/mobx/pull/2238)
 
+-   Fixes #2250 - extendObservable: allows it to be used on an existing property [#2252](https://github.com/mobxjs/mobx/pull/2252)
+
 # 5.15.1 / 4.15.1
 
 -   Make initial values of observable set accept readonly array [#2202](https://github.com/mobxjs/mobx/pull/2202)

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,162 @@
+// @ts-check
+
+import {
+    action,
+    autorun,
+    isObservable,
+    isObservableProp,
+    isComputedProp,
+    isAction,
+    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("extendObservable should apply specified decorators", 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,
+        {
+            someFunc: function() {
+                return 2
+            }
+        },
+        { someFunc: action }
+    )
+
+    expect(isAction(box.someFunc)).toBe(true)
+    expect(box.someFunc()).toEqual(2)
+})

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])