From 3abe0e6fa0062bbdc50ae0bc73b749d8105a755e Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@gmail.com>
Date: Wed, 31 Aug 2016 20:30:44 +0200
Subject: [PATCH] implemented getters for plain objects, see #421

---
 src/api/extendobservable.ts   |  3 ++-
 src/types/observableobject.ts | 15 ++++++++++-----
 test/babel/babel-tests.js     | 19 +++++++++++++++++++
 test/observables.js           | 24 ++++++++++++++++++++++++
 test/typescript-tests.ts      | 19 +++++++++++++++++++
 5 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/src/api/extendobservable.ts b/src/api/extendobservable.ts
index aaa8fffc4..d86c40c7f 100644
--- a/src/api/extendobservable.ts
+++ b/src/api/extendobservable.ts
@@ -25,7 +25,8 @@ export function extendObservableHelper(target, properties, mode: ValueMode, name
 	for (let key in properties) if (hasOwnProperty(properties, key)) {
 		if (target === properties && !isPropertyConfigurable(target, key))
 			continue; // see #111, skip non-configurable or non-writable props for `observable(object)`.
-		setObservableObjectInstanceProperty(adm, key, properties[key]);
+		const descriptor = Object.getOwnPropertyDescriptor(properties, key);
+		setObservableObjectInstanceProperty(adm, key, descriptor);
 	}
 	return target;
 }
diff --git a/src/types/observableobject.ts b/src/types/observableobject.ts
index 171ff6756..6f0cba6f3 100644
--- a/src/types/observableobject.ts
+++ b/src/types/observableobject.ts
@@ -65,11 +65,16 @@ export function asObservableObject(target, name: string, mode: ValueMode = Value
 	return adm;
 }
 
-export function setObservableObjectInstanceProperty(adm: ObservableObjectAdministration, propName: string, value) {
-	if (adm.values[propName])
-		adm.target[propName] = value; // the property setter will make 'value' reactive if needed.
-	else
-		defineObservableProperty(adm, propName, value, true, undefined);
+export function setObservableObjectInstanceProperty(adm: ObservableObjectAdministration, propName: string, descriptor: PropertyDescriptor) {
+	if (adm.values[propName]) {
+		invariant("value" in descriptor, "cannot redefine property " + propName);
+		adm.target[propName] = descriptor.value; // the property setter will make 'value' reactive if needed.
+	} else {
+		if ("value" in descriptor)
+			defineObservableProperty(adm, propName, descriptor.value, true, undefined);
+		else
+			defineObservableProperty(adm, propName, descriptor.get, true, descriptor.set);
+	}
 }
 
 export function defineObservableProperty(adm: ObservableObjectAdministration, propName: string, newValue, asInstanceProperty: boolean, setter) {
diff --git a/test/babel/babel-tests.js b/test/babel/babel-tests.js
index e615732df..c948aebf1 100644
--- a/test/babel/babel-tests.js
+++ b/test/babel/babel-tests.js
@@ -726,3 +726,22 @@ test('computed setter should succeed (babel)', function(t) {
 
 	t.end();
 });
+
+test('computed getter / setter for plan objects should succeed (babel)', function(t) {
+	const b = observable({
+		a: 3,
+		get propX() { return this.a * 2 },
+		set propX(v) { this.a = v }
+	})
+
+	const values = []
+	mobx.autorun(() => values.push(b.propX))
+	t.equal(b.propX, 6);
+	b.propX = 4;
+	t.equal(b.propX, 8);
+
+	t.deepEqual(values, [6, 8])
+
+	t.end();
+});
+
diff --git a/test/observables.js b/test/observables.js
index a3745e043..2aa5d43fd 100644
--- a/test/observables.js
+++ b/test/observables.js
@@ -1852,3 +1852,27 @@ test("support computed property getters / setters", t => {
 	d()
 	t.end()
 })
+
+test('computed getter / setter for plan objects should succeed', function (t) {
+	var b = observable({
+		a: 3,
+		get propX() {
+			return this.a * 2;
+		},
+		set propX(v) {
+			this.a = v;
+		}
+	});
+
+	var values = [];
+	mobx.autorun(function () {
+		return values.push(b.propX);
+	});
+	t.equal(b.propX, 6);
+	b.propX = 4;
+	t.equal(b.propX, 8);
+
+	t.deepEqual(values, [6, 8]);
+
+	t.end();
+});
diff --git a/test/typescript-tests.ts b/test/typescript-tests.ts
index 82abbd2db..695b2af47 100644
--- a/test/typescript-tests.ts
+++ b/test/typescript-tests.ts
@@ -943,3 +943,22 @@ test("505, don't throw when accessing subclass fields in super constructor (type
 	t.deepEqual(values, { a: 1, b: undefined}) // undefined, as A constructor runs before B constructor
 	t.end()
 })
+
+
+test('computed getter / setter for plan objects should succeed (typescript)', function(t) {
+	const b = observable({
+		a: 3,
+		get propX() { return this.a * 2 },
+		set propX(v) { this.a = v }
+	})
+
+	const values: number[] = []
+	mobx.autorun(() => values.push(b.propX))
+	t.equal(b.propX, 6);
+	b.propX = 4;
+	t.equal(b.propX, 8);
+
+	t.deepEqual(values, [6, 8])
+
+	t.end();
+});