From eab77c93031bd038fc72b62ef4bcbd39934ac4bd Mon Sep 17 00:00:00 2001
From: bigopon <bigopon.777@gmail.com>
Date: Thu, 31 Aug 2017 02:12:00 +1000
Subject: [PATCH 1/9] feat(BindableProperty): Prop to attribute

---
 src/bindable-property.js   |  6 +++-
 src/html-behavior.js       | 58 ++++++++++++++++++++++++++++++++++++++
 test/html-behavior.spec.js | 23 +++++++++++++++
 3 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/src/bindable-property.js b/src/bindable-property.js
index 7d516058..214bb7a4 100644
--- a/src/bindable-property.js
+++ b/src/bindable-property.js
@@ -35,7 +35,7 @@ export class BindableProperty {
   * Creates an instance of BindableProperty.
   * @param nameOrConfig The name of the property or a cofiguration object.
   */
-  constructor(nameOrConfig: string | Object) {
+  constructor(nameOrConfig) {
     if (typeof nameOrConfig === 'string') {
       this.name = nameOrConfig;
     } else {
@@ -62,6 +62,10 @@ export class BindableProperty {
     behavior.attributes[this.attribute] = this;
     this.owner = behavior;
 
+    if (this.reflect) {
+      behavior.registerReflection(this.name, this.reflect);
+    }
+
     if (descriptor) {
       this.descriptor = descriptor;
       return this._configureDescriptor(descriptor);
diff --git a/src/html-behavior.js b/src/html-behavior.js
index 50547782..d3f2aaa9 100644
--- a/src/html-behavior.js
+++ b/src/html-behavior.js
@@ -323,6 +323,7 @@ export class HtmlBehaviorResource {
 
     if (element !== null) {
       element.au = au = element.au || {};
+      this._setupReflections(element, this.target);
     }
 
     let viewModel = instruction.viewModel || container.get(this.target);
@@ -401,6 +402,52 @@ export class HtmlBehaviorResource {
     return controller;
   }
 
+  /**
+   * Allow a bindable property on custom element to register how to reflect prop value to attribute
+   * @param {string} propertyName 
+   * @param {boolean | {(element: Element, newVal: any, oldVal: any) => any}} reflection 
+   */
+  registerReflection(propertyName, reflection) {
+    let reflections = this.reflections || (this.reflections = {});
+    if (propertyName in reflections) {
+      throw new Error(`Reflection for ${propertyName} was already registered`);
+    }
+    if (typeof reflection === 'function') {
+      reflections[propertyName] = reflection;
+    } else if (reflection) {
+      reflections[propertyName] = propToAttr;
+    }
+  }
+  
+  /**
+   * @param {Element} element 
+   * @param {Function} target 
+   */
+  _setupReflections(element, target) {
+    if (!this.reflections) return;
+
+    let {reflections} = this;
+    let method = 'propertyChanged';
+    let onChanged = target.prototype[method];
+    let hasHandler = !!onChanged;
+    
+    let alteredHandler = hasHandler
+      ? function propertyChanged(name, newVal, oldVal) {
+          onChanged.call(this, name, newVal, oldVal);
+          reflections[name].call(this, element, name, newVal, oldVal)
+        }
+      : function propertyChanged(name, newVal, oldVal) {
+          reflections[name].call(this, element, name, newVal, oldVal);
+        }
+
+    if (!Reflect.defineProperty(target.prototype, method, {
+      configurable: true,
+      value: alteredHandler
+    })) {
+      throw new Error(`Cannot setup property reflection on <${this.elementName}/> for ${target.name}`);
+    };
+  }
+
   _ensurePropertiesDefined(instance: Object, lookup: Object) {
     let properties;
     let i;
@@ -454,3 +501,14 @@ export class HtmlBehaviorResource {
     }    
   }
 }
+
+/**
+ * @private
+ * Used for avoid creating mapping function multiple times
+ * @param {Element} element 
+ * @param {string} propertyName 
+ * @param {any} newValue 
+ */
+function propToAttr(element, propertyName, newValue) {
+  element.setAttribute(propertyName, newValue);
+}
diff --git a/test/html-behavior.spec.js b/test/html-behavior.spec.js
index 26395015..afb93aaa 100644
--- a/test/html-behavior.spec.js
+++ b/test/html-behavior.spec.js
@@ -3,6 +3,7 @@ import {Container} from 'aurelia-dependency-injection';
 import {ObserverLocator, bindingMode} from 'aurelia-binding';
 import {TaskQueue} from 'aurelia-task-queue';
 import {HtmlBehaviorResource} from '../src/html-behavior';
+import {BindableProperty} from '../src/bindable-property';
 import {ViewResources} from '../src/view-resources';
 
 describe('html-behavior', () => {
@@ -58,4 +59,26 @@ describe('html-behavior', () => {
 
     expect(resource.attributes['test'].defaultBindingMode).toBe(bindingMode.twoWay);
   });
+
+  describe('Prop to attribute reflection', () => {
+    it('should have reflections when bindable properties are registered with `reflect`', () => {
+      let resource = new HtmlBehaviorResource();
+      let Target = class {};
+
+      var prop1 = new BindableProperty({
+        reflect: true,
+        name: 'prop1'
+      });
+      prop1.registerWith(Target, resource);
+
+      var prop2 = new BindableProperty({
+        reflect() {},
+        name: 'prop2'
+      });
+      prop2.registerWith(Target, resource);
+
+      expect(typeof resource.reflections.prop1).toBe('function');
+      expect(resource.reflections.prop2).toBe(prop2.reflect);
+    });
+  });
 });

From bd63798a5a81af461520e5bd4f337f2a392eb050 Mon Sep 17 00:00:00 2001
From: bigopon <bigopon.777@gmail.com>
Date: Thu, 31 Aug 2017 02:18:16 +1000
Subject: [PATCH 2/9] Fix: Add back types for Bindable property constructor

---
 src/bindable-property.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/bindable-property.js b/src/bindable-property.js
index 214bb7a4..280ed51d 100644
--- a/src/bindable-property.js
+++ b/src/bindable-property.js
@@ -35,7 +35,13 @@ export class BindableProperty {
   * Creates an instance of BindableProperty.
   * @param nameOrConfig The name of the property or a cofiguration object.
   */
-  constructor(nameOrConfig) {
+  constructor(nameOrConfig: string | {
+    defaultBindingMode?: number,
+    reflect?: boolean | {(el: Element, newVal, oldVal): any},
+    name?: string,
+    attribute?: any,
+    changeHandler?: string
+  }) {
     if (typeof nameOrConfig === 'string') {
       this.name = nameOrConfig;
     } else {

From 006b7998e1986f7e5469d3aafeb4c1c79570df9b Mon Sep 17 00:00:00 2001
From: bigopon <bigopon.777@gmail.com>
Date: Thu, 31 Aug 2017 02:21:14 +1000
Subject: [PATCH 3/9] Fix method typings

---
 src/bindable-property.js | 2 +-
 src/html-behavior.js     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bindable-property.js b/src/bindable-property.js
index 280ed51d..dcb9cc15 100644
--- a/src/bindable-property.js
+++ b/src/bindable-property.js
@@ -37,7 +37,7 @@ export class BindableProperty {
   */
   constructor(nameOrConfig: string | {
     defaultBindingMode?: number,
-    reflect?: boolean | {(el: Element, newVal, oldVal): any},
+    reflect?: boolean | {(el: Element, name: string, newVal, oldVal): any},
     name?: string,
     attribute?: any,
     changeHandler?: string
diff --git a/src/html-behavior.js b/src/html-behavior.js
index d3f2aaa9..845bd262 100644
--- a/src/html-behavior.js
+++ b/src/html-behavior.js
@@ -405,7 +405,7 @@ export class HtmlBehaviorResource {
   /**
    * Allow a bindable property on custom element to register how to reflect prop value to attribute
    * @param {string} propertyName 
-   * @param {boolean | {(element: Element, newVal: any, oldVal: any) => any}} reflection 
+   * @param {boolean | {(element: Element, name: string, newVal, oldVal) => any}} reflection 
    */
   registerReflection(propertyName, reflection) {
     let reflections = this.reflections || (this.reflections = {});

From 7f8770ebb69ae3d834c372e17f4a5bb7642d19e7 Mon Sep 17 00:00:00 2001
From: bigopon <bigopon.777@gmail.com>
Date: Thu, 31 Aug 2017 02:32:34 +1000
Subject: [PATCH 4/9] Fix: Ignore null/ void by default

---
 src/html-behavior.js | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/html-behavior.js b/src/html-behavior.js
index 845bd262..334bbd77 100644
--- a/src/html-behavior.js
+++ b/src/html-behavior.js
@@ -510,5 +510,9 @@ export class HtmlBehaviorResource {
  * @param {any} newValue 
  */
 function propToAttr(element, propertyName, newValue) {
-  element.setAttribute(propertyName, newValue);
+  if (newVal == null) {
+    element.removeAttribute(propertyName)
+  } else {
+    element.setAttribute(propertyName, newValue);
+  }
 }

From 8c0d71876f1c87ac80cb1c1d3792a5cdd52a51ed Mon Sep 17 00:00:00 2001
From: bigopon <bigopon.777@gmail.com>
Date: Thu, 31 Aug 2017 02:47:31 +1000
Subject: [PATCH 5/9] Fix: Ensure reflect each VM Class once

---
 src/html-behavior.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/html-behavior.js b/src/html-behavior.js
index 334bbd77..f45e3283 100644
--- a/src/html-behavior.js
+++ b/src/html-behavior.js
@@ -425,6 +425,7 @@ export class HtmlBehaviorResource {
    */
   _setupReflections(element, target) {
     if (!this.reflections) return;
+    if (this._reflectedTarget === target) return;
 
     let {reflections} = this;
     let method = 'propertyChanged';
@@ -446,6 +447,7 @@ export class HtmlBehaviorResource {
     })) {
       throw new Error(`Cannot setup property reflection on <${this.elementName}/> for ${target.name}`);
     };
+    this._reflectedTarget = target;
   }
 
   _ensurePropertiesDefined(instance: Object, lookup: Object) {

From 76296059b75b3033f0cb57a1610ee227727bf078 Mon Sep 17 00:00:00 2001
From: bigopon <bigopon.777@gmail.com>
Date: Thu, 31 Aug 2017 03:08:56 +1000
Subject: [PATCH 6/9] Fix param name! ...

---
 src/html-behavior.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/html-behavior.js b/src/html-behavior.js
index f45e3283..74f39f15 100644
--- a/src/html-behavior.js
+++ b/src/html-behavior.js
@@ -512,7 +512,7 @@ export class HtmlBehaviorResource {
  * @param {any} newValue 
  */
 function propToAttr(element, propertyName, newValue) {
-  if (newVal == null) {
+  if (newValue == null) {
     element.removeAttribute(propertyName)
   } else {
     element.setAttribute(propertyName, newValue);

From ab7dab4ef5eeaed4be98630788ca78cbac1604d8 Mon Sep 17 00:00:00 2001
From: bigopon <bigopon.777@gmail.com>
Date: Thu, 31 Aug 2017 11:55:35 +1000
Subject: [PATCH 7/9] Fix: Initiate reflection on each instance

---
 src/bindable-property.js | 53 ++++++++++++++++++++++++++++++++--
 src/html-behavior.js     | 61 +++++++++-------------------------------
 src/view-factory.js      |  5 ++++
 3 files changed, 70 insertions(+), 49 deletions(-)

diff --git a/src/bindable-property.js b/src/bindable-property.js
index dcb9cc15..acd22ab2 100644
--- a/src/bindable-property.js
+++ b/src/bindable-property.js
@@ -64,12 +64,14 @@ export class BindableProperty {
   * @param descriptor The property descriptor for this property.
   */
   registerWith(target: Function, behavior: HtmlBehaviorResource, descriptor?: Object): void {
+    let { reflect } = this;
     behavior.properties.push(this);
     behavior.attributes[this.attribute] = this;
     this.owner = behavior;
 
-    if (this.reflect) {
-      behavior.registerReflection(this.name, this.reflect);
+    if (reflect) {
+      behavior._registerReflection(this.name, typeof reflect === 'function' ? reflect : propToAttr);
+      this._configureReflection(target);
     }
 
     if (descriptor) {
@@ -79,6 +81,38 @@ export class BindableProperty {
 
     return undefined;
   }
+  
+  _configureReflection(target) {
+    if (target.__reflectionConfigured__) return;
+
+    let method = 'propertyChanged';
+    let onChanged = target.prototype[method];
+    let hasHandler = !!onChanged;
+    
+    let alteredHandler;
+    if (hasHandler) { // avoid ternary to make it consisten with the rest ?
+      alteredHandler = function propertyChanged(name, newVal, oldVal) {
+        onChanged.call(this, name, newVal, oldVal);
+        let { __element__, __reflections__ } = this.__observers__;
+        if (!__element__) return;
+        __reflections__[name].call(this, __element__, name, newVal, oldVal)
+      }
+    } else {
+      alteredHandler = function propertyChanged(name, newVal, oldVal) {
+        let { __element__, __reflections__ } = this.__observers__;
+        if (!__element__) return;
+        __reflections__[name].call(this, __element__, name, newVal, oldVal);
+      }
+    }
+  
+    if (!Reflect.defineProperty(target.prototype, method, {
+      configurable: true,
+      value: alteredHandler
+    })) {
+      throw new Error(`Cannot setup property reflection on <${this.name}/> for ${target.name}`);
+    };
+    target.__reflectionConfigured__ = true;
+  }
 
   _configureDescriptor(descriptor: Object): Object {
     let name = this.name;
@@ -258,3 +292,18 @@ export class BindableProperty {
     observer.selfSubscriber = selfSubscriber;
   }
 }
+
+/**
+ * @private
+ * Used for avoid creating mapping function multiple times
+ * @param {Element} element 
+ * @param {string} propertyName 
+ * @param {any} newValue 
+ */
+function propToAttr(element, propertyName, newValue) {
+  if (newValue == null) {
+    element.removeAttribute(propertyName);
+  } else {
+    element.setAttribute(propertyName, newValue);
+  }
+}
diff --git a/src/html-behavior.js b/src/html-behavior.js
index 74f39f15..5222a6a4 100644
--- a/src/html-behavior.js
+++ b/src/html-behavior.js
@@ -323,7 +323,6 @@ export class HtmlBehaviorResource {
 
     if (element !== null) {
       element.au = au = element.au || {};
-      this._setupReflections(element, this.target);
     }
 
     let viewModel = instruction.viewModel || container.get(this.target);
@@ -331,6 +330,10 @@ export class HtmlBehaviorResource {
     let childBindings = this.childBindings;
     let viewFactory;
 
+    if (element !== null) {
+      this._setupReflections(element, viewModel);
+    }
+
     if (this.liftsContent) {
       //template controller
       au.controller = controller;
@@ -405,49 +408,28 @@ export class HtmlBehaviorResource {
   /**
    * Allow a bindable property on custom element to register how to reflect prop value to attribute
    * @param {string} propertyName 
-   * @param {boolean | {(element: Element, name: string, newVal, oldVal) => any}} reflection 
+   * @param {{(element: Element, name: string, newVal, oldVal) => any}} instruction A function with suitable parameters to react for setting attribute on the element
    */
-  registerReflection(propertyName, reflection) {
+  _registerReflection(propertyName, instruction) {
     let reflections = this.reflections || (this.reflections = {});
     if (propertyName in reflections) {
       throw new Error(`Reflection for ${propertyName} was already registered`);
     }
-    if (typeof reflection === 'function') {
-      reflections[propertyName] = reflection;
-    } else if (reflection) {
-      reflections[propertyName] = propToAttr;
+    if (typeof instruction !== 'function') {
+      throw new Error('Invalid reflection instruction');
     }
+    reflections[propertyName] = instruction;
   }
   
   /**
    * @param {Element} element 
-   * @param {Function} target 
+   * @param {object} viewModel 
    */
-  _setupReflections(element, target) {
+  _setupReflections(element, viewModel) {
     if (!this.reflections) return;
-    if (this._reflectedTarget === target) return;
-
-    let {reflections} = this;
-    let method = 'propertyChanged';
-    let onChanged = target.prototype[method];
-    let hasHandler = !!onChanged;
-    
-    let alteredHandler = hasHandler
-      ? function propertyChanged(name, newVal, oldVal) {
-          onChanged.call(this, name, newVal, oldVal);
-          reflections[name].call(this, element, name, newVal, oldVal)
-        }
-      : function propertyChanged(name, newVal, oldVal) {
-          reflections[name].call(this, element, name, newVal, oldVal);
-        }
-
-    if (!Reflect.defineProperty(target.prototype, method, {
-      configurable: true,
-      value: alteredHandler
-    })) {
-      throw new Error(`Cannot setup property reflection on <${this.elementName}/> for ${target.name}`);
-    };
-    this._reflectedTarget = target;
+    let lookup = this.observerLocator.getOrCreateObserversLookup(viewModel);
+    lookup.__reflections__ = this.reflections;
+    lookup.__element__ = element;
   }
 
   _ensurePropertiesDefined(instance: Object, lookup: Object) {
@@ -503,18 +485,3 @@ export class HtmlBehaviorResource {
     }    
   }
 }
-
-/**
- * @private
- * Used for avoid creating mapping function multiple times
- * @param {Element} element 
- * @param {string} propertyName 
- * @param {any} newValue 
- */
-function propToAttr(element, propertyName, newValue) {
-  if (newValue == null) {
-    element.removeAttribute(propertyName)
-  } else {
-    element.setAttribute(propertyName, newValue);
-  }
-}
diff --git a/src/view-factory.js b/src/view-factory.js
index 0900c694..d2534867 100644
--- a/src/view-factory.js
+++ b/src/view-factory.js
@@ -104,6 +104,10 @@ function setAttribute(name, value) {
   this._element.setAttribute(name, value);
 }
 
+function removeAttribute(name) {
+  this._element.removeAttribute(name);
+}
+
 function makeElementIntoAnchor(element, elementInstruction) {
   let anchor = DOM.createComment('anchor');
 
@@ -119,6 +123,7 @@ function makeElementIntoAnchor(element, elementInstruction) {
     anchor.hasAttribute = hasAttribute;
     anchor.getAttribute = getAttribute;
     anchor.setAttribute = setAttribute;
+    anchor.removeAttribute = removeAttribute;
   }
 
   DOM.replaceNode(anchor, element);

From 746f194bca7f4bc06298557f4b53df2d35de661a Mon Sep 17 00:00:00 2001
From: bigopon <bigopon.777@gmail.com>
Date: Sat, 2 Sep 2017 00:36:03 +1000
Subject: [PATCH 8/9] Add bindable typing, extract refelection to separate fn

---
 src/bindable-property.js   | 85 ++++++++++++++++++++++----------------
 src/html-behavior.js       | 25 +++++------
 test/html-behavior.spec.js |  6 +--
 3 files changed, 63 insertions(+), 53 deletions(-)

diff --git a/src/bindable-property.js b/src/bindable-property.js
index acd22ab2..9514b2c7 100644
--- a/src/bindable-property.js
+++ b/src/bindable-property.js
@@ -4,15 +4,17 @@ import {bindingMode} from 'aurelia-binding';
 import {Container} from 'aurelia-dependency-injection';
 import {metadata} from 'aurelia-metadata';
 
+const reflectionConfigured = Symbol('reflection');
+
 function getObserver(instance, name) {
   let lookup = instance.__observers__;
 
   if (lookup === undefined) {
-    // We need to lookup the actual behavior for this instance, 
-    // as it might be a derived class (and behavior) rather than 
+    // We need to lookup the actual behavior for this instance,
+    // as it might be a derived class (and behavior) rather than
     // the class (and behavior) that declared the property calling getObserver().
-    // This means we can't capture the behavior in property get/set/getObserver and pass it here. 
-    // Note that it's probably for the best, as passing the behavior is an overhead 
+    // This means we can't capture the behavior in property get/set/getObserver and pass it here.
+    // Note that it's probably for the best, as passing the behavior is an overhead
     // that is only useful in the very first call of the first property of the instance.
     let ctor = Object.getPrototypeOf(instance).constructor; // Playing safe here, user could have written to instance.constructor.
     let behavior = metadata.get(metadata.resource, ctor);
@@ -27,6 +29,14 @@ function getObserver(instance, name) {
   return lookup[name];
 }
 
+export type BindablePropertyConfig = {
+  defaultBindingMode?: bindingMode,
+  reflectToAttribute?: boolean | {(el: Element, name: string, newVal, oldVal): any},
+  name?: string,
+  attribute?: any,
+  changeHandler?: string
+}
+
 /**
 * Represents a bindable property on a behavior.
 */
@@ -35,13 +45,7 @@ export class BindableProperty {
   * Creates an instance of BindableProperty.
   * @param nameOrConfig The name of the property or a cofiguration object.
   */
-  constructor(nameOrConfig: string | {
-    defaultBindingMode?: number,
-    reflect?: boolean | {(el: Element, name: string, newVal, oldVal): any},
-    name?: string,
-    attribute?: any,
-    changeHandler?: string
-  }) {
+  constructor(nameOrConfig: string | BindablePropertyConfig) {
     if (typeof nameOrConfig === 'string') {
       this.name = nameOrConfig;
     } else {
@@ -64,13 +68,14 @@ export class BindableProperty {
   * @param descriptor The property descriptor for this property.
   */
   registerWith(target: Function, behavior: HtmlBehaviorResource, descriptor?: Object): void {
-    let { reflect } = this;
+    let { reflectToAttribute } = this;
+
     behavior.properties.push(this);
     behavior.attributes[this.attribute] = this;
     this.owner = behavior;
 
-    if (reflect) {
-      behavior._registerReflection(this.name, typeof reflect === 'function' ? reflect : propToAttr);
+    if (reflectToAttribute) {
+      behavior._registerReflection(this.name, typeof reflectToAttribute === 'function' ? reflectToAttribute : propToAttr);
       this._configureReflection(target);
     }
 
@@ -81,37 +86,36 @@ export class BindableProperty {
 
     return undefined;
   }
-  
+
   _configureReflection(target) {
-    if (target.__reflectionConfigured__) return;
+    if (target[reflectionConfigured]) return;
 
     let method = 'propertyChanged';
     let onChanged = target.prototype[method];
     let hasHandler = !!onChanged;
-    
+
     let alteredHandler;
     if (hasHandler) { // avoid ternary to make it consisten with the rest ?
       alteredHandler = function propertyChanged(name, newVal, oldVal) {
         onChanged.call(this, name, newVal, oldVal);
-        let { __element__, __reflections__ } = this.__observers__;
-        if (!__element__) return;
-        __reflections__[name].call(this, __element__, name, newVal, oldVal)
-      }
+        callRefelection(this, name, newVal, oldVal);
+      };
     } else {
       alteredHandler = function propertyChanged(name, newVal, oldVal) {
-        let { __element__, __reflections__ } = this.__observers__;
-        if (!__element__) return;
-        __reflections__[name].call(this, __element__, name, newVal, oldVal);
-      }
+        callRefelection(this, name, newVal, oldVal);
+      };
     }
-  
+
+    // Reflect has better performance on chrome https://jsfiddle.net/bm792m4e/7/
+    // Also helps to avoid put try catch in the scope.
+    // Though Chrome no longer suffers much from it, it still impacts
     if (!Reflect.defineProperty(target.prototype, method, {
       configurable: true,
       value: alteredHandler
     })) {
-      throw new Error(`Cannot setup property reflection on <${this.name}/> for ${target.name}`);
-    };
-    target.__reflectionConfigured__ = true;
+      throw new Error(`Cannot setup property [${this.name}] reflection for ${target.name}`);
+    }
+    target[reflectionConfigured] = true;
   }
 
   _configureDescriptor(descriptor: Object): Object {
@@ -293,17 +297,26 @@ export class BindableProperty {
   }
 }
 
+/**
+ * @param instance the view model instance
+ * @param propertyName name of property changed
+ * @param newValue
+ * @param oldValue
+ */
+function callRefelection(instance: Object, propertyName: string, newValue, oldValue) {
+  let { __element__, __reflections__ } = instance.__observers__;
+  if (!__element__) return;
+  __reflections__[propertyName].call(instance, __element__, propertyName, newVal, oldVal);
+}
+
 /**
  * @private
  * Used for avoid creating mapping function multiple times
- * @param {Element} element 
- * @param {string} propertyName 
- * @param {any} newValue 
  */
-function propToAttr(element, propertyName, newValue) {
-  if (newValue == null) {
-    element.removeAttribute(propertyName);
+function propToAttr(element: Element, propertyName: string, newValue: any) {
+  if (newValue === null || newValue === void 0) {
+    element.removeAttribute(_hyphenate(propertyName));
   } else {
-    element.setAttribute(propertyName, newValue);
+    element.setAttribute(_hyphenate(propertyName), newValue);
   }
 }
diff --git a/src/html-behavior.js b/src/html-behavior.js
index 5222a6a4..4000c633 100644
--- a/src/html-behavior.js
+++ b/src/html-behavior.js
@@ -155,8 +155,8 @@ export class HtmlBehaviorResource {
       for (i = 0, ii = properties.length; i < ii; ++i) {
         properties[i].defineOn(target, this);
       }
-      // Because how inherited properties would interact with the default 'value' property 
-      // in a custom attribute is not well defined yet, we only inherit properties on 
+      // Because how inherited properties would interact with the default 'value' property
+      // in a custom attribute is not well defined yet, we only inherit properties on
       // custom elements, where it's not a problem.
       this._copyInheritedProperties(container, target);
     }
@@ -407,7 +407,7 @@ export class HtmlBehaviorResource {
 
   /**
    * Allow a bindable property on custom element to register how to reflect prop value to attribute
-   * @param {string} propertyName 
+   * @param {string} propertyName
    * @param {{(element: Element, name: string, newVal, oldVal) => any}} instruction A function with suitable parameters to react for setting attribute on the element
    */
   _registerReflection(propertyName, instruction) {
@@ -415,15 +415,12 @@ export class HtmlBehaviorResource {
     if (propertyName in reflections) {
       throw new Error(`Reflection for ${propertyName} was already registered`);
     }
-    if (typeof instruction !== 'function') {
-      throw new Error('Invalid reflection instruction');
-    }
     reflections[propertyName] = instruction;
   }
-  
+
   /**
-   * @param {Element} element 
-   * @param {object} viewModel 
+   * @param {Element} element
+   * @param {object} viewModel
    */
   _setupReflections(element, viewModel) {
     if (!this.reflections) return;
@@ -455,9 +452,9 @@ export class HtmlBehaviorResource {
   }
 
   _copyInheritedProperties(container: Container, target: Function) {
-    // This methods enables inherited @bindable properties.    
-    // We look for the first base class with metadata, make sure it's initialized 
-    // and copy its properties. 
+    // This methods enables inherited @bindable properties.
+    // We look for the first base class with metadata, make sure it's initialized
+    // and copy its properties.
     // We don't need to walk further than the first parent with metadata because
     // it had also inherited properties during its own initialization.
     let behavior, derived = target;
@@ -472,7 +469,7 @@ export class HtmlBehaviorResource {
         break;
       }
     }
-    behavior.initialize(container, target);    
+    behavior.initialize(container, target);
     for (let i = 0, ii = behavior.properties.length; i < ii; ++i) {
       let prop = behavior.properties[i];
       // Check that the property metadata was not overriden or re-defined in this class
@@ -482,6 +479,6 @@ export class HtmlBehaviorResource {
       // We don't need to call .defineOn() for those properties because it was done
       // on the parent prototype during initialization.
       new BindableProperty(prop).registerWith(derived, this);
-    }    
+    }
   }
 }
diff --git a/test/html-behavior.spec.js b/test/html-behavior.spec.js
index afb93aaa..244b9ff6 100644
--- a/test/html-behavior.spec.js
+++ b/test/html-behavior.spec.js
@@ -66,19 +66,19 @@ describe('html-behavior', () => {
       let Target = class {};
 
       var prop1 = new BindableProperty({
-        reflect: true,
+        reflectToAttribute: true,
         name: 'prop1'
       });
       prop1.registerWith(Target, resource);
 
       var prop2 = new BindableProperty({
-        reflect() {},
+        reflectToAttribute() {},
         name: 'prop2'
       });
       prop2.registerWith(Target, resource);
 
       expect(typeof resource.reflections.prop1).toBe('function');
-      expect(resource.reflections.prop2).toBe(prop2.reflect);
+      expect(resource.reflections.prop2).toBe(prop2.reflectToAttribute);
     });
   });
 });

From 47987e0cf55c9f6eb7795491cbbdcbe76c515809 Mon Sep 17 00:00:00 2001
From: bigopon <bigopon.777@gmail.com>
Date: Sun, 10 Sep 2017 11:18:40 +1000
Subject: [PATCH 9/9] move reflection to selfSubscriber, test to bindable
 property

---
 src/bindable-property.js       | 112 +++++++++++++++++----------------
 src/html-behavior.js           |  29 +--------
 test/bindable-property.spec.js |  37 +++++++++++
 test/html-behavior.spec.js     |  22 -------
 4 files changed, 98 insertions(+), 102 deletions(-)

diff --git a/src/bindable-property.js b/src/bindable-property.js
index 9514b2c7..2e9f0a68 100644
--- a/src/bindable-property.js
+++ b/src/bindable-property.js
@@ -75,8 +75,7 @@ export class BindableProperty {
     this.owner = behavior;
 
     if (reflectToAttribute) {
-      behavior._registerReflection(this.name, typeof reflectToAttribute === 'function' ? reflectToAttribute : propToAttr);
-      this._configureReflection(target);
+      behavior.hasReflections = true;
     }
 
     if (descriptor) {
@@ -87,37 +86,6 @@ export class BindableProperty {
     return undefined;
   }
 
-  _configureReflection(target) {
-    if (target[reflectionConfigured]) return;
-
-    let method = 'propertyChanged';
-    let onChanged = target.prototype[method];
-    let hasHandler = !!onChanged;
-
-    let alteredHandler;
-    if (hasHandler) { // avoid ternary to make it consisten with the rest ?
-      alteredHandler = function propertyChanged(name, newVal, oldVal) {
-        onChanged.call(this, name, newVal, oldVal);
-        callRefelection(this, name, newVal, oldVal);
-      };
-    } else {
-      alteredHandler = function propertyChanged(name, newVal, oldVal) {
-        callRefelection(this, name, newVal, oldVal);
-      };
-    }
-
-    // Reflect has better performance on chrome https://jsfiddle.net/bm792m4e/7/
-    // Also helps to avoid put try catch in the scope.
-    // Though Chrome no longer suffers much from it, it still impacts
-    if (!Reflect.defineProperty(target.prototype, method, {
-      configurable: true,
-      value: alteredHandler
-    })) {
-      throw new Error(`Cannot setup property [${this.name}] reflection for ${target.name}`);
-    }
-    target[reflectionConfigured] = true;
-  }
-
   _configureDescriptor(descriptor: Object): Object {
     let name = this.name;
 
@@ -182,27 +150,64 @@ export class BindableProperty {
     let defaultValue = this.defaultValue;
     let changeHandlerName = this.changeHandler;
     let name = this.name;
+    let reflectToAttribute = this.reflectToAttribute;
     let initialValue;
+    let attrName;
+    let reflectFunction;
 
     if (this.hasOptions) {
       return undefined;
     }
 
+    if (reflectToAttribute) {
+      attrName = this.attribute === undefined ? _hyphenate(name) : this.attribute;
+      reflectFunction = typeof reflectToAttribute === 'function' ? reflectToAttribute : reflectFunctions[reflectToAttribute];
+    }
+
     if (changeHandlerName in viewModel) {
       if ('propertyChanged' in viewModel) {
+        if (reflectFunction !== undefined) {
+          selfSubscriber = (newValue, oldValue) => {
+            callReflection(viewModel, reflectFunction, attrName, newValue);
+            viewModel[changeHandlerName](newValue, oldValue);
+            viewModel.propertyChanged(name, newValue, oldValue);
+          };
+        } else {
+          selfSubscriber = (newValue, oldValue) => {
+            viewModel[changeHandlerName](newValue, oldValue);
+            viewModel.propertyChanged(name, newValue, oldValue);
+          };
+        }
+      } else {
+        if (reflectFunction !== undefined) {
+          selfSubscriber = (newValue, oldValue) => {
+            callReflection(viewModel, reflectFunction, attrName, newValue);
+            viewModel[changeHandlerName](newValue, oldValue);
+          };
+        } else {
+          selfSubscriber = (newValue, oldValue) => viewModel[changeHandlerName](newValue, oldValue);
+        }
+      }
+    } else if ('propertyChanged' in viewModel) {
+      if (reflectFunction !== undefined) {
         selfSubscriber = (newValue, oldValue) => {
-          viewModel[changeHandlerName](newValue, oldValue);
+          callReflection(viewModel, reflectFunction, attrName, newValue);
           viewModel.propertyChanged(name, newValue, oldValue);
         };
       } else {
-        selfSubscriber = (newValue, oldValue) => viewModel[changeHandlerName](newValue, oldValue);
+        selfSubscriber = (newValue, oldValue) => viewModel.propertyChanged(name, newValue, oldValue);
       }
-    } else if ('propertyChanged' in viewModel) {
-      selfSubscriber = (newValue, oldValue) => viewModel.propertyChanged(name, newValue, oldValue);
     } else if (changeHandlerName !== null) {
       throw new Error(`Change handler ${changeHandlerName} was specified but not declared on the class.`);
     }
 
+    /**
+     * When view model doesn't have change handler but this property has reflection
+     */
+    if (selfSubscriber === null && reflectFunction !== undefined) {
+      selfSubscriber = (newValue, oldValue) => callReflection(viewModel, reflectFunction, attrName, newValue);
+    }
+
     if (defaultValue !== undefined) {
       initialValue = typeof defaultValue === 'function' ? defaultValue.call(viewModel) : defaultValue;
     }
@@ -298,25 +303,24 @@ export class BindableProperty {
 }
 
 /**
- * @param instance the view model instance
- * @param propertyName name of property changed
+ * @param viewModel the view model instance
+ * @param attrName name of attribute will be set on the element
  * @param newValue
- * @param oldValue
  */
-function callRefelection(instance: Object, propertyName: string, newValue, oldValue) {
-  let { __element__, __reflections__ } = instance.__observers__;
-  if (!__element__) return;
-  __reflections__[propertyName].call(instance, __element__, propertyName, newVal, oldVal);
+function callReflection(viewModel: Object, reflectFunction: (element: Element, attrName: string, val) => any, attrName: string, newValue) {
+  let { __element__ } = viewModel.__observers__;
+  reflectFunction(__element__, attrName, newValue);
 }
 
-/**
- * @private
- * Used for avoid creating mapping function multiple times
- */
-function propToAttr(element: Element, propertyName: string, newValue: any) {
-  if (newValue === null || newValue === void 0) {
-    element.removeAttribute(_hyphenate(propertyName));
-  } else {
-    element.setAttribute(_hyphenate(propertyName), newValue);
+const reflectFunctions = {
+  string(element, attrName, newValue) {
+    element.setAttribute(attrName, newValue);
+  },
+  boolean(element, attrName, newValue) {
+    if (newValue) {
+      element.setAttribute(attrName, '');
+    } else {
+      element.removeAttribute(attrName);
+    }
   }
-}
+};
diff --git a/src/html-behavior.js b/src/html-behavior.js
index 4000c633..2aebc587 100644
--- a/src/html-behavior.js
+++ b/src/html-behavior.js
@@ -46,6 +46,7 @@ export class HtmlBehaviorResource {
     this.attributes = {};
     this.isInitialized = false;
     this.primaryProperty = null;
+    this.hasReflections = false;
   }
 
   /**
@@ -330,8 +331,8 @@ export class HtmlBehaviorResource {
     let childBindings = this.childBindings;
     let viewFactory;
 
-    if (element !== null) {
-      this._setupReflections(element, viewModel);
+    if (element !== null && this.hasReflections) {
+      this.observerLocator.getOrCreateObserversLookup(viewModel).__element__ = element;
     }
 
     if (this.liftsContent) {
@@ -405,30 +406,6 @@ export class HtmlBehaviorResource {
     return controller;
   }
 
-  /**
-   * Allow a bindable property on custom element to register how to reflect prop value to attribute
-   * @param {string} propertyName
-   * @param {{(element: Element, name: string, newVal, oldVal) => any}} instruction A function with suitable parameters to react for setting attribute on the element
-   */
-  _registerReflection(propertyName, instruction) {
-    let reflections = this.reflections || (this.reflections = {});
-    if (propertyName in reflections) {
-      throw new Error(`Reflection for ${propertyName} was already registered`);
-    }
-    reflections[propertyName] = instruction;
-  }
-
-  /**
-   * @param {Element} element
-   * @param {object} viewModel
-   */
-  _setupReflections(element, viewModel) {
-    if (!this.reflections) return;
-    let lookup = this.observerLocator.getOrCreateObserversLookup(viewModel);
-    lookup.__reflections__ = this.reflections;
-    lookup.__element__ = element;
-  }
-
   _ensurePropertiesDefined(instance: Object, lookup: Object) {
     let properties;
     let i;
diff --git a/test/bindable-property.spec.js b/test/bindable-property.spec.js
index 4fd3932e..b130e16d 100644
--- a/test/bindable-property.spec.js
+++ b/test/bindable-property.spec.js
@@ -13,4 +13,41 @@ describe('BindableProperty', () => {
     expect(new BindableProperty({ name: 'test', defaultBindingMode: null }).defaultBindingMode).toBe(oneWay);
     expect(new BindableProperty({ name: 'test', defaultBindingMode: undefined }).defaultBindingMode).toBe(oneWay);
   });
+
+  describe('reflects to attribute', () => {
+    let viewModel;
+    let element;
+    let observer;
+    beforeEach(() => {
+      element = document.createElement('div');
+      viewModel = { __observers__: { __element__: element } };
+    });
+
+    it('should reflect prop as string', () => {
+      let prop = new BindableProperty({
+        reflectToAttribute: 'string',
+        name: 'prop'
+      });
+      prop.owner = {};
+      observer = prop.createObserver(viewModel);
+      observer.selfSubscriber('Hello');
+      expect(element.getAttribute('prop')).toBe('Hello');
+    });
+
+    it('should reflect prop as boolean', () => {
+      let prop = new BindableProperty({
+        reflectToAttribute: 'boolean',
+        name: 'prop'
+      });
+      prop.owner = {};
+      observer = prop.createObserver(viewModel);
+      observer.selfSubscriber('Hello');
+      expect(element.getAttribute('prop')).toBe('');
+      ['', NaN, 0, false, null, undefined].forEach(v => {
+        observer.selfSubscriber(v);
+        expect(element.hasAttribute('prop')).toBe(false);
+        element.setAttribute('prop', 'Hello');
+      });
+    });
+  });
 });
diff --git a/test/html-behavior.spec.js b/test/html-behavior.spec.js
index 244b9ff6..2cb08432 100644
--- a/test/html-behavior.spec.js
+++ b/test/html-behavior.spec.js
@@ -59,26 +59,4 @@ describe('html-behavior', () => {
 
     expect(resource.attributes['test'].defaultBindingMode).toBe(bindingMode.twoWay);
   });
-
-  describe('Prop to attribute reflection', () => {
-    it('should have reflections when bindable properties are registered with `reflect`', () => {
-      let resource = new HtmlBehaviorResource();
-      let Target = class {};
-
-      var prop1 = new BindableProperty({
-        reflectToAttribute: true,
-        name: 'prop1'
-      });
-      prop1.registerWith(Target, resource);
-
-      var prop2 = new BindableProperty({
-        reflectToAttribute() {},
-        name: 'prop2'
-      });
-      prop2.registerWith(Target, resource);
-
-      expect(typeof resource.reflections.prop1).toBe('function');
-      expect(resource.reflections.prop2).toBe(prop2.reflectToAttribute);
-    });
-  });
 });