fix(observable): handle descriptor with set

fixes #511
aurelia · Oct 4, 2016 · fa3dafb · fa3dafb
1 parent 521270b
commit fa3dafb
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 24 deletions.
diff --git a/src/decorator-observable.js b/src/decorator-observable.js
@@ -37,6 +37,7 @@ export function observable(targetOrConfig: any, key: string, descriptor?: Proper
 
     // we're adding a getter and setter which means the property descriptor
     // cannot have a "value" or "writable" attribute
+    delete descriptor.value;
     delete descriptor.writable;
     delete descriptor.initializer;
 

diff --git a/test/decorator-observable.spec.js b/test/decorator-observable.spec.js
@@ -72,45 +72,76 @@ describe('observable decorator', () => {
     expect(instance.value).toEqual(newValue);
   });
 
-  it('should work with decorators function', () => {
-    const instance = new (decorators(observable('value'))
-      .on(class {
+  describe('es2015 with decorators function', () => {
+    it('should work when decorating property', () => {
+      class MyClass {
         constructor() {
           this.value = oldValue;
         }
         valueChanged() { }
-      }));
-    spyOn(instance, 'valueChanged');
+      }
+      decorators(observable).on(MyClass.prototype, 'value');
+      const instance = new MyClass();
+      spyOn(instance, 'valueChanged');
 
-    instance.value = newValue;
-    expect(instance.valueChanged).toHaveBeenCalledWith(newValue, oldValue, 'value');
-  });
+      instance.value = newValue;
+      expect(instance.valueChanged).toHaveBeenCalledWith(newValue, oldValue, 'value');
+    });
 
-  it('should work with decorators function when property is undefined', () => {
-    const instance = new (decorators(observable('value'))
-      .on(class {
+    it('should work when decorating class', () => {
+      class MyClass {
+        constructor() {
+          this.value = oldValue;
+        }
         valueChanged() { }
-      }));
-    spyOn(instance, 'valueChanged');
+      }
+      decorators(observable('value')).on(MyClass);
+      const instance = new MyClass();
+      spyOn(instance, 'valueChanged');
 
-    instance.value = newValue;
-    expect(instance.valueChanged).toHaveBeenCalledWith(newValue, undefined, 'value');
-  });
+      instance.value = newValue;
+      expect(instance.valueChanged).toHaveBeenCalledWith(newValue, oldValue, 'value');
+    });
 
-  it('should work with decorators function and config', () => {
-    const instance = new (decorators(observable({ name: 'value', changeHandler: 'customHandler' }))
-      .on(class {
+    it('should work when property is undefined', () => {
+      class MyClass {
+        valueChanged() { }
+      }
+      decorators(observable).on(MyClass.prototype, 'value');
+      const instance = new MyClass();
+      spyOn(instance, 'valueChanged');
+
+      instance.value = newValue;
+      expect(instance.valueChanged).toHaveBeenCalledWith(newValue, undefined, 'value');
+    });
+
+    it('should work with customHandler', () => {
+      class MyClass {
         constructor() {
           this.value = oldValue;
         }
         customHandler() { }
-      }));
-    spyOn(instance, 'customHandler');
+      }
+      decorators(observable({ changeHandler: 'customHandler' })).on(MyClass.prototype, 'value');
+      const instance = new MyClass();
+      spyOn(instance, 'customHandler');
 
-    instance.value = newValue;
-    expect(instance.customHandler).toHaveBeenCalledWith(newValue, oldValue, 'value');
+      instance.value = newValue;
+      expect(instance.customHandler).toHaveBeenCalledWith(newValue, oldValue, 'value');
+    });
   });
-
+
+  it('should return a valid descriptor', () => {
+    const target = class { };
+    const descriptor = observable(target, 'value');
+
+    expect(typeof descriptor.value).toBe('undefined');
+    expect(typeof descriptor.writable).toBe('undefined');
+    expect(typeof descriptor.get).toBe('function');
+    expect(typeof descriptor.set).toBe('function');
+    expect(Reflect.defineProperty(target, 'value', descriptor)).toBe(true);
+  });
+
   it('should create an enumerable accessor', () => {
     const instance = new class {
       @observable value;