diff --git a/lib/elements/dom-if.js b/lib/elements/dom-if.js index ad63809582..729833a32c 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, @@ -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; } } 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..a12dc689cc 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 (#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); });