Merge pull request #4608 from Polymer/fix-4601

Ensure data flows correctly when enabling properties when client elements have already enabled

lib/mixins/property-accessors.html

@@ -138,7 +138,7 @@
         this.__serializing = false;
         this.__dataCounter = 0;
         this.__dataEnabled = false;
-        this.__dataInitialized = false;
+        this.__dataReady = false;
         this.__dataInvalid = false;
         // initialize data with prototype values saved when creating accessors
         this.__data = {};
@@ -465,7 +465,7 @@
        * @protected
        */
       _invalidateProperties() {
-        if (!this.__dataInvalid && this.__dataInitialized) {
+        if (!this.__dataInvalid && this.__dataReady) {
           this.__dataInvalid = true;
           microtask.run(() => {
             if (this.__dataInvalid) {
@@ -529,7 +529,7 @@
        * @public
        */
       ready() {
-        this.__dataInitialized = true;
+        this.__dataReady = true;
         // Run normal flush
         this._flushProperties();
       }

lib/mixins/property-effects.html

@@ -1087,7 +1087,7 @@
       _initializeProperties() {
         super._initializeProperties();
         hostStack.registerHost(this);
-        this.__dataClientsInitialized = false;
+        this.__dataClientsReady = false;
         this.__dataPendingClients = null;
         this.__dataToNotify = null;
         this.__dataLinkedPaths = null;
@@ -1395,14 +1395,14 @@
 
       /**
        * Overrides `PropertyAccessor`'s default async queuing of
-       * `_propertiesChanged`: if `__dataInitialized` is false (has not yet been
+       * `_propertiesChanged`: if `__dataReady` is false (has not yet been
        * manually flushed), the function no-ops; otherwise flushes
        * `_propertiesChanged` synchronously.
        *
        * @override
        */
       _invalidateProperties() {
-        if (this.__dataInitialized) {
+        if (this.__dataReady) {
           this._flushProperties();
         }
       }
@@ -1429,48 +1429,56 @@
        * @protected
        */
       _flushClients() {
-        if (!this.__dataClientsInitialized) {
-          this.__dataClientsInitialized = true;
+        if (!this.__dataClientsReady) {
+          this.__dataClientsReady = true;
           this._readyClients();
           // Override point where accessors are turned on; importantly,
           // this is after clients have fully readied, providing a guarantee
           // that any property effects occur only after all clients are ready.
-          this.__dataInitialized = true;
+          this.__dataReady = true;
         } else {
-          // Flush all clients
-          let clients = this.__dataPendingClients;
-          if (clients) {
-            this.__dataPendingClients = null;
-            for (let i=0; i < clients.length; i++) {
-              let client = clients[i];
-              if (client.__dataPending) {
-                client._flushProperties();
-              }
-            }
-          }
+          this.__enableOrFlushClients();
         }
       }
 
-      /**
-       * Perform any initial setup on client dom. Called before the first
-       * `_flushProperties` call on client dom and before any element
-       * observers are called.
-       *
-       * @protected
-       */
-      _readyClients() {
+      // NOTE: We ensure clients either enable or flush as appropriate. This
+      // handles two corner cases:
+      // (1) clients flush properly when connected/enabled before the host
+      // enables; e.g.
+      //   (a) Templatize stamps with no properties and does not flush and
+      //   (b) the instance is inserted into dom and
+      //   (c) then the instance flushes.
+      // (2) clients enable properly when not connected/enabled when the host
+      // flushes; e.g.
+      //   (a) a template is runtime stamped and not yet connected/enabled
+      //   (b) a host sets a property, causing stamped dom to flush
+      //   (c) the stamped dom enables.
+      __enableOrFlushClients() {
         let clients = this.__dataPendingClients;
         if (clients) {
           this.__dataPendingClients = null;
           for (let i=0; i < clients.length; i++) {
             let client = clients[i];
             if (!client.__dataEnabled) {
               client._enableProperties();
+            } else if (client.__dataPending) {
+              client._flushProperties();
             }
           }
         }
       }
 
+      /**
+       * Perform any initial setup on client dom. Called before the first
+       * `_flushProperties` call on client dom and before any element
+       * observers are called.
+       *
+       * @protected
+       */
+      _readyClients() {
+        this.__enableOrFlushClients();
+      }
+
       /**
        * Sets a bag of property changes to this instance, and
        * synchronously processes all effects of the properties as a batch.
@@ -1514,7 +1522,7 @@
         this._flushProperties();
         // If no data was pending, `_flushProperties` will not `flushClients`
         // so ensure this is done.
-        if (!this.__dataClientsInitialized) {
+        if (!this.__dataClientsReady) {
           this._flushClients();
         }
         // Before ready, client notifications do not trigger _flushProperties.
@@ -2245,7 +2253,7 @@
         // Setup compound storage, 2-way listeners, and dataHost for bindings
         setupBindings(this, templateInfo);
         // Flush properties into template nodes if already booted
-        if (this.__dataInitialized) {
+        if (this.__dataReady) {
           runEffects(this, templateInfo.propertyEffects, this.__data, null,
             false, templateInfo.nodeList);
         }

test/unit/property-effects-template.html

@@ -78,7 +78,7 @@
     </template>
     <!-- Nested template in Shadow DOM for runtime stamping -->
     <template id="templateFromShadowDom">
-      <x-element id="first"></x-element>
+      <x-element early="[[earlyProp]]" id="first"></x-element>
       <x-element id="events" on-click="handleClick" on-tap="handleTap"></x-element>
       <template id="domIf" is="dom-if" if="[[prop]]">
         <x-element id="ifElementSD" prop="{{prop}}" path="{{obj.path}}"></x-element>
@@ -131,6 +131,12 @@
         this.shadowRoot.appendChild(dom);
         return dom;
       }
+      stampTemplateAndSetPropFromShadow() {
+        let dom = this._stampTemplate(this.$.templateFromShadowDom);
+        this.earlyProp = 'early';
+        this.shadowRoot.appendChild(dom);
+        return dom;
+      }
       stampTemplateFromLight() {
         let dom = this._stampTemplate(Polymer.DomModule.import(this.localName, '#templateFromLightDom'));
         this.shadowRoot.appendChild(dom);
@@ -430,6 +436,90 @@
     assert.equal(stamped[0], el.$.first);
   });
 
+  test('runtime stamp template and set prop before attaching (from shadow dom)', () => {
+    let dom = el.stampTemplateAndSetPropFromShadow();
+    assertStampingCorrect(el, el.$);
+    assertStampingCorrect(el, dom.$, 'SD');
+    let stamped = el.shadowRoot.querySelectorAll('x-element#first');
+    assert.equal(stamped.length, 2);
+    assert.equal(stamped[0], el.$.first);
+    assert.equal(stamped[1], dom.$.first);
+    assert.deepEqual(Polymer.lifecycleOrder.ready, [
+      'x-runtime|x-element#proto|x-element-child#noBinding',
+      'x-runtime|x-element#proto|x-element-child#hasBinding',
+      'x-runtime|x-element#proto',
+      'x-runtime',
+      'x-element#shadow|x-element-child#noBinding',
+      'x-element#shadow|x-element-child#hasBinding',
+      'x-element#shadow'
+    ]);
+  });
+
+  test('runtime stamp and remove multiple templates and set prop before attaching (from shadow dom)', () => {
+    let stamped;
+    // Stamp template
+    let dom1 = el.stampTemplateAndSetPropFromShadow();
+    assertStampingCorrect(el, el.$);
+    assertStampingCorrect(el, dom1.$, 'SD');
+    stamped = el.shadowRoot.querySelectorAll('x-element#first');
+    assert.equal(stamped.length, 2);
+    assert.equal(stamped[0], el.$.first);
+    assert.equal(stamped[1], dom1.$.first);
+    // Unstamp
+    el._removeBoundDom(dom1);
+    for (let n in dom1.$) {
+      assert.notOk(dom1.$[n].parentNode, null);
+    }
+    stamped = el.shadowRoot.querySelectorAll('x-element#first');
+    assert.equal(stamped.length, 1);
+    assert.equal(stamped[0], el.$.first);
+    // Stamp again
+    let dom2 = el.stampTemplateAndSetPropFromShadow();
+    assertStampingCorrect(el, el.$);
+    assertStampingCorrect(el, dom2.$, 'SD');
+    stamped = el.shadowRoot.querySelectorAll('x-element#first');
+    assert.equal(stamped.length, 2);
+    assert.equal(stamped[0], el.$.first);
+    assert.equal(stamped[1], dom2.$.first);
+    // Stamp again
+    let dom3 = el.stampTemplateAndSetPropFromShadow();
+    assertStampingCorrect(el, el.$);
+    assertStampingCorrect(el, dom2.$, 'SD');
+    assertStampingCorrect(el, dom3.$, 'SD');
+    stamped = el.shadowRoot.querySelectorAll('x-element#first');
+    assert.equal(stamped.length, 3);
+    assert.equal(stamped[0], el.$.first);
+    assert.equal(stamped[1], dom2.$.first);
+    assert.equal(stamped[2], dom3.$.first);
+    assert.deepEqual(Polymer.lifecycleOrder.ready, [
+      'x-runtime|x-element#proto|x-element-child#noBinding',
+      'x-runtime|x-element#proto|x-element-child#hasBinding',
+      'x-runtime|x-element#proto',
+      'x-runtime',
+      'x-element#shadow|x-element-child#noBinding',
+      'x-element#shadow|x-element-child#hasBinding',
+      'x-element#shadow',
+      'x-runtime|x-element#shadow|x-element-child#noBinding',
+      'x-runtime|x-element#shadow|x-element-child#hasBinding',
+      'x-runtime|x-element#shadow',
+      'x-runtime|x-element#shadow|x-element-child#noBinding',
+      'x-runtime|x-element#shadow|x-element-child#hasBinding',
+      'x-runtime|x-element#shadow'
+    ]);
+    // Unstamp
+    el._removeBoundDom(dom2);
+    el._removeBoundDom(dom3);
+    for (let n in dom2.$) {
+      assert.notOk(dom1.$[n].parentNode, null);
+    }
+    for (let n in dom3.$) {
+      assert.notOk(dom1.$[n].parentNode, null);
+    }
+    stamped = el.shadowRoot.querySelectorAll('x-element#first');
+    assert.equal(stamped.length, 1);
+    assert.equal(stamped[0], el.$.first);
+  });
+
   test('runtime stamp template (from light dom)', () => {
     let dom = el.stampTemplateFromLight();
     assertStampingCorrect(el, el.$);

test/unit/templatize.html

@@ -181,6 +181,20 @@
       assert.isUndefined(childB.computedFromLiteral);
     });
 
+    test('templatize with no props, instance manually flushes', function() {
+      let templatizeA = host.shadowRoot.querySelector('[id=templatizeA]');
+      templatizeA.go(false);
+      let childA = host.shadowRoot.querySelector('#childA');
+      assert.ok(childA);
+      sinon.spy(childA, 'objChanged');
+      assert.isUndefined(childA.obj);
+      let o = {foo: true};
+      templatizeA.instance._setPendingProperty('obj', o);
+      templatizeA.instance._flushProperties();
+      assert.equal(childA.obj, o);
+      assert.isTrue(childA.objChanged.calledOnce);
+    });
+
   });
 
   suite('templatizer behavior', function() {