Merge pull request #5630 from Polymer/legacy-undefined-noBatch-5629

Store syncInfo on instance and don't sync paths. Fixes #5629

lib/elements/dom-if.js

@@ -252,7 +252,7 @@ class DomIfBase extends PolymerElement {
       this.__teardownInstance();
     }
     this._showHideChildren();
-    if ((!suppressTemplateNotifications || this.notifyDomChange) 
+    if ((!suppressTemplateNotifications || this.notifyDomChange)
         && this.if != this._lastIf) {
       this.dispatchEvent(new CustomEvent('dom-change', {
         bubbles: true,
@@ -386,7 +386,7 @@ class DomIfFast extends DomIfBase {
     // Install runEffects hook that prevents running property effects
     // (and any nested template effects) when the `if` is false
     templateInfo.runEffects = (runEffects, changedProps, hasPaths) => {
-      const syncInfo = this.__syncInfo;
+      let syncInfo = this.__syncInfo;
       if (this.if) {
         // Mix any props that changed while the `if` was false into `changedProps`
         if (syncInfo) {
@@ -400,22 +400,31 @@ class DomIfFast extends DomIfBase {
           this.__syncInfo = null;
           this._showHideChildren();
           changedProps = Object.assign(syncInfo.changedProps, changedProps);
-          hasPaths = hasPaths || syncInfo.hasPaths;
         }
         runEffects(changedProps, hasPaths);
       } else {
         // Accumulate any values changed while `if` was false, along with the
         // runEffects method to sync them, so that we can replay them once `if`
         // becomes true
-        if (syncInfo) {
-          syncInfo.hasPaths = syncInfo.hasPaths || hasPaths;
-          Object.assign(syncInfo.changedProps, changedProps);
-        } else {
-          this.__syncInfo = {
-            runEffects,
-            changedProps: Object.assign({}, changedProps),
-            hasPaths
-          };
+        if (this.__instance) {
+          if (!syncInfo) {
+            syncInfo = this.__syncInfo = { runEffects, changedProps: {} };
+          }
+          if (hasPaths) {
+            // Store root object of any paths; this will ensure direct bindings
+            // like [[obj.foo]] bindings run after a `set('obj.foo', v)`, but
+            // note that path notifications like `set('obj.foo.bar', v)` will
+            // not propagate. Since batched path notifications are not
+            // supported, we cannot simply accumulate path notifications. This
+            // is equivalent to the non-fastDomIf case, which stores root(p) in
+            // __invalidProps.
+            for (const p in changedProps) {
+              const rootProp = root(p);
+              syncInfo.changedProps[rootProp] = this.__dataHost[rootProp];
+            }
+          } else {
+            Object.assign(syncInfo.changedProps, changedProps);
+          }
         }
       }
     };
@@ -434,7 +443,7 @@ class DomIfFast extends DomIfBase {
     const syncInfo = this.__syncInfo;
     if (syncInfo) {
       this.__syncInfo = null;
-      syncInfo.runEffects(syncInfo.changedProps, syncInfo.hasPaths);
+      syncInfo.runEffects(syncInfo.changedProps, false);
     }
   }
 
@@ -452,6 +461,7 @@ class DomIfFast extends DomIfBase {
     if (this.__instance) {
       host._removeBoundDom(this.__instance);
       this.__instance = null;
+      this.__syncInfo = null;
     }
   }
 
@@ -598,11 +608,11 @@ class DomIfLegacy extends DomIfBase {
   __syncHostProperties() {
     let props = this.__invalidProps;
     if (props) {
+      this.__invalidProps = null;
       for (let prop in props) {
         this.__instance._setPendingProperty(prop, this.__dataHost[prop]);
       }
       this.__instance._flushProperties();
-      this.__invalidProps = null;
     }
   }
 

test/unit/dom-if-elements.js

@@ -321,16 +321,18 @@ Polymer({
       observer: 'propChanged'
     }
   },
+  observers: ['pathChanged(obj.*)'],
   created() {
     this.propChanged = sinon.spy();
+    this.pathChanged = sinon.spy();
   }
 });
 
 Polymer({
   _template: html`
     <template is="dom-if" if="{{b}}" restamp="{{restamp}}">
       {{guarded(a)}}
-      <prop-observer id="observer" prop="[[c.d]]"></prop-observer>
+      <prop-observer id="observer" prop="[[c.d]]" obj="[[c]]"></prop-observer>
     </template>
 `,
 

test/unit/dom-if.html

@@ -940,21 +940,33 @@
       el.a = 'ok';
       el.c = {d: 'ok'};
       assert.equal(el.shadowRoot.textContent.trim(), 'ok');
-      assert.equal(el.shadowRoot.querySelector('#observer').propChanged.callCount, 1);
+      let observer = el.shadowRoot.querySelector('#observer');
+      assert.equal(observer.propChanged.callCount, 1);
+      assert.equal(observer.pathChanged.callCount, 1);
+      assert.equal(observer.pathChanged.getCalls()[0].args[0].path, 'obj');
       el.b = false;
       el.a = 'notok';
       el.set('c.d', 'notok');
       flush();
       assert.equal(el.shadowRoot.textContent.trim(), '');
       if (!restamp) {
-        assert.equal(el.shadowRoot.querySelector('#observer').propChanged.callCount, 1);
+        const observer = el.shadowRoot.querySelector('#observer');
+        assert.equal(observer.propChanged.callCount, 1);
+        assert.equal(observer.pathChanged.callCount, 1);
       }
       el.set('c.d', 'changed');
       el.a = 'changed';
       el.b = true;
       flush();
       assert.equal(el.shadowRoot.textContent.trim(), 'changed');
-      assert.equal(el.shadowRoot.querySelector('#observer').propChanged.callCount, restamp ? 1 : 2);
+      observer = el.shadowRoot.querySelector('#observer');
+      // Note that path bindings in a falsey dom-if will be updated when the
+      // dom-if becomes truthy, however path notifications to other elements
+      // will be lost; this is a fundamental limitation (#4818) due to the fact
+      // that batched path notifications are not supported. Hence the
+      // `propChanged` count increments, but the `pathChanged` count does not.
+      assert.equal(observer.propChanged.callCount, restamp ? 1 : 2);
+      assert.equal(observer.pathChanged.callCount, 1);
       document.body.removeChild(el);
     });