Merged #111 by @capaj: bork on non configurable props

src/api/observabledecorator.ts

@@ -1,7 +1,7 @@
 import {ValueMode, asReference} from "../types/modifiers";
 import {allowStateChanges} from "../api/extras";
 import {asObservableObject, setObservableObjectProperty} from "../types/observableobject";
-import {invariant} from "../utils/utils";
+import {invariant, assertPropertyConfigurable} from "../utils/utils";
 
 /**
  * ES6 / Typescript decorator which can to make class properties and getter functions reactive.
@@ -16,6 +16,8 @@ import {invariant} from "../utils/utils";
  */
 export function observableDecorator(target: Object, key: string, baseDescriptor: PropertyDescriptor) {
 	invariant(arguments.length >= 2 && arguments.length <= 3, "Illegal decorator config", key);
+	assertPropertyConfigurable(target, key);
+
 	// - In typescript, observable annotations are invoked on the prototype, not on actual instances,
 	// so upon invocation, determine the 'this' instance, and define a property on the
 	// instance as well (that hides the propotype property)

src/types/observableobject.ts

@@ -6,7 +6,7 @@
 import {ObservableValue} from "./observablevalue";
 import {ComputedValue} from "../core/computedvalue";
 import {ValueMode, AsStructure} from "./modifiers";
-import {Lambda, invariant} from "../utils/utils";
+import {Lambda, invariant, assertPropertyConfigurable} from "../utils/utils";
 import {SimpleEventEmitter} from "../utils/simpleeventemitter";
 import {getNextId} from "../core/globalstate";
 
@@ -63,6 +63,8 @@ export function setObservableObjectProperty(adm: IObservableObjectAdministration
 }
 
 function defineObservableProperty(adm: IObservableObjectAdministration, propName: string, value) {
+	assertPropertyConfigurable(adm.target, propName);
+
 	let observable: ComputedValue<any>|ObservableValue<any>;
 	let name = `${adm.name}@${adm.id} / Prop "${propName}"`;
 

src/utils/utils.ts

@@ -70,6 +70,18 @@ export function makeNonEnumerable(object: any, props: string[]) {
 	}
 }
 
+export function isPropertyConfigurable(object: any, prop: string): boolean {
+	const descriptor = Object.getOwnPropertyDescriptor(object, prop);
+	return !descriptor || (descriptor.configurable !== false && descriptor.writable !== false);
+}
+
+export function assertPropertyConfigurable(object: any, prop: string) {
+	invariant(
+		isPropertyConfigurable(object, prop),
+		`Cannot make property '${prop}' observable, it is not configurable and writable in the target object`
+	);
+}
+
 /**
  * Naive deepEqual. Doesn't check for prototype, non-enumerable or out-of-range properties on arrays.
  * If you have such a case, you probably should use this function but something fancier :).

test/makereactive.js

@@ -222,41 +222,41 @@ test('flat array', function(t) {
             a: 1
         }])
     });
-    
+
     var result;
     var updates = 0;
     var dis = m.autorun(function() {
         updates++;
         result = mobservable.toJSON(x);
     });
-    
+
     t.deepEqual(result, { x: [{ a: 1 }]});
     t.equal(updates, 1);
-    
+
     x.x[0].a = 2; // not picked up; object is not made reactive
     t.deepEqual(result, { x: [{ a: 1 }]});
     t.equal(updates, 1);
-    
+
     x.x.push({ a: 3 }); // picked up, array is reactive
     t.deepEqual(result, { x: [{ a: 2}, { a: 3 }]});
     t.equal(updates, 2);
-    
+
     x.x[0] = { a: 4 }; // picked up, array is reactive
     t.deepEqual(result, { x: [{ a: 4 }, { a: 3 }]});
     t.equal(updates, 3);
 
-    x.x[1].a = 6; // not picked up    
+    x.x[1].a = 6; // not picked up
     t.deepEqual(result, { x: [{ a: 4 }, { a: 3 }]});
     t.equal(updates, 3);
-    
+
     t.end();
 })
 
 test('flat object', function(t) {
     var y = m.observable(m.asFlat({
         x : { z: 3 }
     }));
-    
+
     var result;
     var updates = 0;
     var dis = m.autorun(function() {
@@ -266,34 +266,34 @@ test('flat object', function(t) {
 
     t.deepEqual(result, { x: { z: 3 }});
     t.equal(updates, 1);
-    
+
     y.x.z = 4; // not picked up
     t.deepEqual(result, { x: { z: 3 }});
     t.equal(updates, 1);
-    
+
     y.x = { z: 5 };
     t.deepEqual(result, { x: { z: 5 }});
     t.equal(updates, 2);
 
     y.x.z = 6; // not picked up
     t.deepEqual(result, { x: { z: 5 }});
     t.equal(updates, 2);
-    
+
     t.end();
 })
 
 test('as structure', function(t) {
-    
+
     var x = m.observable({
         x: m.asStructure(null)
     });
-    
+
     var changed = 0;
     var dis = m.autorun(function() {
         changed++;
         JSON.stringify(x);
     });
-    
+
     function c() {
         t.equal(changed, 1, "expected a change");
         if (changed !== 1)
@@ -307,7 +307,7 @@ test('as structure', function(t) {
             console.trace();
         changed = 0;
     }
-    
+
     // nc = no change, c = changed.
     c();
     x.x = null;
@@ -400,7 +400,7 @@ test('as structure', function(t) {
     nc();
     x.x.a[0].b = 3;
     nc();
-    
+
     dis();
     t.end();
 })
@@ -425,7 +425,7 @@ test('as structure view', function(t) {
         bc++;
     });
     t.equal(bc, 1);
-    
+
     var cc = 0;
     var co = m.autorun(function() {
         x.c;
@@ -443,6 +443,38 @@ test('as structure view', function(t) {
     t.end();
 })
 
+test('ES5 non reactive props', function (t) {
+  var te = {}
+  Object.defineProperty(te, 'nonConfigurable', {
+    enumerable: true,
+    configurable: false,
+    writable: true,
+    value: 'static'
+  })
+  t.throws(function() {
+	 const a = m.observable(te)
+  });
+  const d1 = Object.getOwnPropertyDescriptor(te, 'nonConfigurable')
+  t.equal(d1.value, 'static')
+
+  var te2 = {};
+  Object.defineProperty(te2, 'notWritable', {
+    enumerable: true,
+    configurable: true,
+    writable: false,
+    value: 'static'
+  })
+  t.throws(function() {
+	 const a = m.observable(te2)
+  });
+  const d2 = Object.getOwnPropertyDescriptor(te2, 'notWritable')
+  t.equal(d2.value, 'static')
+
+  t.equal(m.extendObservable(te, { 'bla' : 3}).bla, 3);
+
+  t.end();
+})
+
 test('exceptions', function(t) {
     t.throws(function() {
         m.asReference(m.asFlat(3));
@@ -451,7 +483,7 @@ test('exceptions', function(t) {
     var x = m.observable({
         y: m.asReference(null)
     });
-    
+
     t.throws(function() {
         x.y = m.asStructure(3)
     });
@@ -469,7 +501,7 @@ test('exceptions', function(t) {
     t.throws(function() {
         ar[1] = m.asReference(3)
     });
-    
+
     t.throws(function() {
         ar = m.observable([m.asStructure(3)]);
     });