diff --git a/CHANGELOG.md b/CHANGELOG.md index 7158fe97b..4d5f18c8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 2.6.0 + +* Extending observable objects with other observable (objects) is now explicitly forbidden, fixes #540. + # 2.5.2 * Introduced `isComputed` diff --git a/src/api/extendobservable.ts b/src/api/extendobservable.ts index 5a1fc6a97..7d0e2273a 100644 --- a/src/api/extendobservable.ts +++ b/src/api/extendobservable.ts @@ -1,6 +1,7 @@ import {ValueMode} from "../types/modifiers"; import {isObservableMap} from "../types/observablemap"; import {asObservableObject, setObservableObjectInstanceProperty} from "../types/observableobject"; +import {isObservable} from "../api/isobservable"; import {invariant, isPropertyConfigurable, hasOwnProperty} from "../utils/utils"; /** @@ -15,6 +16,7 @@ export function extendObservable(target: A, invariant(!(isObservableMap(target)), "extendObservable should not be used on maps, use map.merge instead"); properties.forEach(propSet => { invariant(typeof propSet === "object", "all arguments of extendObservable should be objects"); + invariant(!isObservable(propSet), "extending an object with another observable (object) is not supported. Please construct an explicit propertymap, using `toJS` if need. See issue #540"); extendObservableHelper(target, propSet, ValueMode.Recursive, null); }); return target; diff --git a/test/makereactive.js b/test/makereactive.js index 7fd37b445..3946c7b1b 100644 --- a/test/makereactive.js +++ b/test/makereactive.js @@ -473,10 +473,10 @@ test('ES5 non reactive props', function (t) { }); const d2 = Object.getOwnPropertyDescriptor(te2, 'notWritable') t.equal(d2.value, 'static') - + // should not throw for other props t.equal(m.extendObservable(te, { 'bla' : 3}).bla, 3); - + t.end(); }) @@ -513,3 +513,24 @@ test('exceptions', function(t) { return t.end(); }) + +test("540 - extendobservable should not report cycles", function(t) { + var objWrapper = mobx.observable({ + value: null, + }); + + var obj = { + name: "Hello", + }; + + objWrapper.value = obj; + t.throws( + () => mobx.extendObservable(objWrapper, obj), + /extending an object with another observable \(object\) is not supported/ + ); + + mobx.autorun(() => { + console.log(objWrapper.name); + }); + t.end(); +})