Merge pull request #77 from PolymerLabs/fixes-74

Fixes #74. The `finalize` technique used in Polymer.Element is not am…

src/compat/class.html

@@ -38,7 +38,7 @@
       for (var i=0; i<behaviors.length; i++) {
         var b = behaviors[i];
         if (b) {
-          klass = Array.isArray(b) ? mixinBehaviors(b, klass) : 
+          klass = Array.isArray(b) ? mixinBehaviors(b, klass) :
             MixinBehavior(b, klass);
         }
       }
@@ -72,29 +72,30 @@
         properties: behavior.properties,
         observers: behavior.observers
       };
-      
+
       class PolymerGenerated extends Base {
 
         static get config() {
           return config;
         }
 
-        static finalize() {
-          if (!this.finalized) {
-            super.finalize();
-            if (behavior.registered) {
-              behavior.registered.call(this.prototype);
-            }
-            this.finalized = true;
-          }
-        }
-
         _invokeFunction(fn, args) {
           if (fn) {
             fn.apply(this, args);
           }
         }
 
+        constructor() {
+          super();
+          // call `registered` only if it was not called for *this* constructor
+          if (!PolymerGenerated.hasOwnProperty('__registered')) {
+            PolymerGenerated.__registered = true;
+            if (behavior.registered) {
+              behavior.registered.call(Object.getPrototypeOf(this));
+            }
+          }
+        }
+
         created() {
           super.created();
           this._invokeFunction(behavior.created);
@@ -128,7 +129,7 @@
       }
 
       for (var p in behavior) {
-        if (!(p in metaProps)) 
+        if (!(p in metaProps))
           utils.copyOwnProperty(p, behavior, PolymerGenerated.prototype);
       }
 
@@ -176,12 +177,12 @@
       // behaviors on prototype for BC...
       behaviors.reverse();
       klass.prototype.behaviors = behaviors;
-      // NOTE: while we could call `beforeRegister` here to maintain 
+      // NOTE: while we could call `beforeRegister` here to maintain
       // some BC, the state of the element at this point is not as it was in 1.0
       // In 1.0, the method was called *after* mixing prototypes together
       // but before processing of meta-objects. Since this is now done
       // in 1 step via `MixinBehavior`, this is no longer possible.
-      // However, *most* work (not setting `is`) that was previously done in 
+      // However, *most* work (not setting `is`) that was previously done in
       // `beforeRegister` should be possible in `registered`.
       return klass;
     }

src/elements/element.html

@@ -36,13 +36,23 @@
 
     class PolymerElement extends data.extendWithAPI(Base) {
 
+      // returns the config object on specifically on `this` class (not super)
+      // config is used for:
+      // (1) super chain mixes togther to make `flattenedProperties` which is
+      // then used to make observedAttributes and set property defaults
+      // (2) properties effects and observers are created from it at `finalize` time.
       static get _ownConfig() {
         if (!this.hasOwnProperty('__ownConfig')) {
           this.__ownConfig = this.hasOwnProperty('config') ? this.config : {};
         }
         return this.__ownConfig;
       }
 
+      // a flattened list of properties mixed together from the chain of all
+      // constructor's `config.properties`
+      // This list is used to create
+      // (1) observedAttributes,
+      // (2) element default values
       static get _flattenedProperties() {
         if (!this.hasOwnProperty('__flattenedProperties')) {
           this.__flattenedProperties = this._ownConfig.properties || {};
@@ -58,15 +68,17 @@
 
       static get observedAttributes() {
         if (!this.hasOwnProperty('_observedAttributes')) {
+          // observedAttributes must be finalized at registration time
+          this._observedAttributes = this.addPropertiesToAttributes(
+            this._flattenedProperties, []);
           // TODO(kschaaf): revisit: capture import document, to aid finding dom-module
-        var currentScript = document._currentScript || document.currentScript;
-        this.__importDoc = currentScript && currentScript.ownerDocument;
-        // observedAttributes must be finalized at registration time
-        this._observedAttributes = this.addPropertiesToAttributes(
-          this._flattenedProperties, []);
-        }
-        if (!Polymer.StyleLib.nativeShadow) {
-          this.__placeholder = Polymer.StyleLib.applyStylePlaceHolder(this.is);
+          var currentScript = document._currentScript || document.currentScript;
+          this.__importDoc = currentScript && currentScript.ownerDocument;
+          // TODO(sorvell): define time cheat. Should we patch
+          // customElements.define instead?
+          if (!Polymer.StyleLib.nativeShadow) {
+            this.__placeholder = Polymer.StyleLib.applyStylePlaceHolder(this.is);
+          }
         }
         return this._observedAttributes;
       }
@@ -78,49 +90,61 @@
         return attrs;
       }
 
-      static createProperties(properties) {
-        if (properties) {
-          this.data.createProperties(this.prototype, properties);
-        }
-      }
-
-      static createMethodObservers(observers) {
-        if (observers) {
-          this.data.createMethodObservers(this.prototype, observers);
-        }
-      }
-
-      static get finalized() {
+      static get _finalized() {
         return this.hasOwnProperty('__finalized');
       }
 
-      static set finalized(value) {
+      static set _finalized(value) {
         this.__finalized = value;
       }
 
-      // bikeshed this name... we have `prepare` below which is not good.
-      static finalize() {
+      // TODO(sorvell): need to work on public api surrouding `finalize`.
+      // Due to meta-programming, it's awkward to make a subclass impl of this.
+      // However, a user might want to call `finalize` prior to define to do
+      // this work eagerly. Need to also decide on `finalizeConfig(config)` and
+      // `finalizeTemplate(template)`. Both are public but have simiarly
+      // awkward subclassing characteristics.
+      static _finalize() {
         var proto = this.prototype;
-        if (!this.finalized) {
-          if (this.hasOwnProperty('is') && this.is) {
-            Polymer.telemetry.register(proto);
-          }
-          this.finalized = true;
+        if (!this._finalized) {
+
           var superProto = Object.getPrototypeOf(proto);
           var superCtor = superProto && superProto.constructor;
           if (superCtor.prototype instanceof PolymerElement) {
-            superCtor.finalize();
+            superCtor._finalize();
+          }
+          this._finalized = true;
+          if (this.hasOwnProperty('is') && this.is) {
+            Polymer.telemetry.register(proto);
           }
           var config = this._ownConfig;
-          this.createProperties(config.properties);
-          this.createMethodObservers(config.observers);
+          if (config) {
+            this.finalizeConfig(config);
+          }
           if (this.template) {
             var template = this.template.cloneNode(true);
             this.finalizeTemplate(template);
           }
         }
       }
 
+      static createProperties(properties) {
+        if (properties) {
+          this.data.createProperties(this.prototype, properties);
+        }
+      }
+
+      static createMethodObservers(observers) {
+        if (observers) {
+          this.data.createMethodObservers(this.prototype, observers);
+        }
+      }
+
+      static finalizeConfig(config) {
+        this.createProperties(config.properties);
+        this.createMethodObservers(config.observers);
+      }
+
       static get template() {
         if (!this.hasOwnProperty('_template')) {
           // TODO(sorvell): `__importDoc` may not be set if super class
@@ -167,8 +191,8 @@
         super();
         // note: `this.constructor.prototype` is wrong in Safari so make sure to
         // use `__proto__`
-        if (!this.constructor.finalized) {
-          this.constructor.finalize();
+        if (!this.constructor._finalized) {
+          this.constructor._finalize();
         }
         Polymer.StyleLib.prepareHost(this, this._template);
         Polymer.telemetry.instanceCount++;

test/smoke/behaviors-registered.html

@@ -0,0 +1,45 @@
+<!doctype html>
+<!--
+@license
+Copyright (c) 2014 The Polymer Project Authors. All rights reserved.
+This code may only be used under the BSD style license found at http://polymer.github.io/LICENSE.txt
+The complete set of authors may be found at http://polymer.github.io/AUTHORS.txt
+The complete set of contributors may be found at http://polymer.github.io/CONTRIBUTORS.txt
+Code distributed by Google as part of the polymer project is also
+subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt
+-->
+<html>
+<head>
+  <title>Polymer - behaviors</title>
+  <meta charset="utf-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <script src="../../../webcomponentsjs/webcomponents-lite.js"></script>
+  <link rel="import" href="../../polymer.html">
+</head>
+<body>
+  <dom-module id="my-element">
+  <template>
+    Check console output
+  </template>
+  <script>
+    let count = 0;
+    let TestBehavior = {
+      // Invoked once in Polymer 1.0, twice in alacarte.
+      registered: function() {
+        console.log(++count);
+        console.log(this.behaviors);
+      }
+    };
+    Polymer({
+      is: 'my-element',
+      behaviors: [TestBehavior]
+    });
+  </script>
+</dom-module>
+
+</head>
+<body>
+  <my-element></my-element>
+</body>
+</body>
+</html>

test/unit/behaviors.html

@@ -210,7 +210,7 @@
       <div id="content"></div>
     </template>
     <script>
-      Polymer.registerBehavior ={
+      Polymer.registerBehavior1 ={
         registered: function() {
           this.constructor.createProperties({
             prop: {
@@ -220,6 +220,8 @@
           this.constructor.createMethodObservers([
             'propChanged2(prop)'
           ]);
+          this.registeredCount++;
+          this.registeredThis = this;
         },
         ready: function() {
           this.ensureAttributes({
@@ -234,61 +236,37 @@
         }
       };
 
-      Polymer({
-        behaviors: [
-          Polymer.registerBehavior
-        ],
-
-        is: 'behavior-registered'
-      });
-    </script>
-  </dom-module>
-
-  <!--
-  <dom-module id="behavior-as-is">
-    <template><div id="content"></div></template>
-    <script>
-      Polymer.registerBehavior ={
-        beforeRegister: function() {
-          this.is = this.as;
-          this.properties = {
-            prop: {
-              observer: 'propChanged1'
-            }
-          };
-          this.hostAttributes = {
-            attr: true
-          };
-          this.observers = [
-            'propChanged2(prop)'
-          ];
-        },
-        propChanged1: function() {
-          this.propChanged1Called = true;
-        },
-        propChanged2: function() {
-          this.propChanged2Called = true;
+      Polymer.registerBehavior2 ={
+        registered: function() {
+          this.registeredCount++;
         }
       };
 
-      try {
+      Polymer.registerBehavior3 ={
+        registered: function() {
+          this.registeredCount++;
+        }
+      };
 
-        Polymer({
+      Polymer({
+        behaviors: [
+          Polymer.registerBehavior1,
+          Polymer.registerBehavior2,
+          Polymer.registerBehavior3
+        ],
 
-          behaviors: [
-            Polymer.registerBehavior
-          ],
+        registered: function() {
+          this.registeredCount++;
+        },
 
-          as: 'behavior-as-is'
+        registeredCount: 0,
 
-        });
-      } catch(e) {
-        window.behaviorAsIsFail = true;
-      }
+        is: 'behavior-registered'
+      });
     </script>
   </dom-module>
-   -->
-</script>
+
+  </script>
 
   <test-fixture id="single">
     <template>
@@ -357,17 +335,25 @@
   });
 });
 
-suite('registered behavior', function() {
-  test('registered behavior', function() {
+suite('behavior.registered', function() {
+  test('can install dynamic properties', function() {
     var el = fixture('registered');
     assert.ok(el.$.content);
     el.prop = 42;
     assert.isTrue(el.propChanged1Called);
     assert.isTrue(el.propChanged2Called);
     assert.isTrue(el.hasAttribute('attr'));
   });
+
+  test('called once for each behavior with access to element prototype', function() {
+    var el = fixture('registered');
+    assert.equal(el.registeredCount, 4);
+    assert.equal(el.registeredThis, Object.getPrototypeOf(el));
+  });
+
 });
 
+
 suite('multi-behaviors element', function() {
   var el;