From e23560fcfb14a0559d7d0817a6845c3e57e416a5 Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@gmail.com>
Date: Thu, 29 Sep 2016 09:53:21 +0200
Subject: [PATCH] Fixed #540 by explicitly forbidding extending with observable
 objects

---
 CHANGELOG.md                |  4 ++++
 src/api/extendobservable.ts |  2 ++
 test/makereactive.js        | 25 +++++++++++++++++++++++--
 3 files changed, 29 insertions(+), 2 deletions(-)

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 d86c40c7f..9ac6fed89 100644
--- a/src/api/extendobservable.ts
+++ b/src/api/extendobservable.ts
@@ -1,6 +1,7 @@
 import {ValueMode} from "../types/modifiers";
 import {ObservableMap} 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<A extends Object, B extends Object>(target: A,
 	invariant(!(target instanceof ObservableMap), "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 <A & B> 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();
+})