diff --git a/lib/elements/dom-if.js b/lib/elements/dom-if.js index ad63809582..103e4e2691 100644 --- a/lib/elements/dom-if.js +++ b/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); } } diff --git a/test/unit/dom-if-elements.js b/test/unit/dom-if-elements.js index a173f77a2c..67ab066ec9 100644 --- a/test/unit/dom-if-elements.js +++ b/test/unit/dom-if-elements.js @@ -321,8 +321,10 @@ Polymer({ observer: 'propChanged' } }, + observers: ['pathChanged(obj.*)'], created() { this.propChanged = sinon.spy(); + this.pathChanged = sinon.spy(); } }); @@ -330,7 +332,7 @@ Polymer({ _template: html` `, diff --git a/test/unit/dom-if.html b/test/unit/dom-if.html index 09d578a3e7..6e7d0da445 100644 --- a/test/unit/dom-if.html +++ b/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 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); });