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,
@@ -339,7 +339,6 @@ class DomIfFast extends DomIfBase {
   constructor() {
     super();
     this.__instance = null;
-    this.__syncInfo = null;
   }
 
   /**
@@ -386,7 +385,8 @@ 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;
+      const instance = this.__instance;
+      let syncInfo = instance && instance.__syncInfo;
       if (this.if) {
         // Mix any props that changed while the `if` was false into `changedProps`
         if (syncInfo) {
@@ -397,25 +397,34 @@ class DomIfFast extends DomIfBase {
           // the next render. Clearing `__invalidProps` here ensures
           // `_showHideChildren`'s call to `__syncHostProperties` no-ops, so
           // that we don't call `runEffects` more often than necessary.
-          this.__syncInfo = null;
+          instance.__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 (instance) {
+          if (!syncInfo) {
+            syncInfo = instance.__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);
+          }
         }
       }
     };
@@ -431,10 +440,11 @@ class DomIfFast extends DomIfBase {
    * @return {void}
    */
   __syncHostProperties() {
-    const syncInfo = this.__syncInfo;
+    const instance = this.__instance;
+    const syncInfo = instance && instance.__syncInfo;
     if (syncInfo) {
-      this.__syncInfo = null;
-      syncInfo.runEffects(syncInfo.changedProps, syncInfo.hasPaths);
+      instance.__syncInfo = null;
+      syncInfo.runEffects(syncInfo.changedProps, false);
     }
   }
 

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);
     });