From 89d70557bd7563647c75d95d9af87e3ad5b01796 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Mon, 8 Jul 2019 17:27:06 -0700 Subject: [PATCH] Ensure hasPaths is also accumulated as part of info necessary to sync. --- lib/elements/dom-if.js | 38 ++++++++++++++++++++-------------- lib/mixins/property-effects.js | 6 +++--- test/unit/dom-if-elements.js | 18 +++++++++++++++- test/unit/dom-if.html | 10 ++++++++- 4 files changed, 52 insertions(+), 20 deletions(-) diff --git a/lib/elements/dom-if.js b/lib/elements/dom-if.js index 2661d2e7b6..ba794a77b4 100644 --- a/lib/elements/dom-if.js +++ b/lib/elements/dom-if.js @@ -331,8 +331,7 @@ class DomIfFast extends DomIfBase { constructor() { super(); this.__instance = null; - this.__invalidProps = null; - this.__squelchedRunEffects = null; + this.__syncInfo = null; } /** @@ -378,11 +377,11 @@ class DomIfFast extends DomIfBase { /** @type {!HTMLTemplateElement} */ (this.__template), true); // Install runEffects hook that prevents running property effects // (and any nested template effects) when the `if` is false - templateInfo.runEffects = (runEffects, changedProps) => { + templateInfo.runEffects = (runEffects, changedProps, hasPaths) => { + const syncInfo = this.__syncInfo; if (this.if) { - const invalidProps = this.__invalidProps; // Mix any props that changed while the `if` was false into `changedProps` - if (invalidProps) { + if (syncInfo) { // If there were properties received while the `if` was false, it is // important to sync the hidden state with the element _first_, so that // new bindings to e.g. `textContent` do not get stomped on by @@ -390,17 +389,26 @@ 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.__squelchedRunEffects = this.__invalidProps = null; + this.__syncInfo = null; this._showHideChildren(); - changedProps = Object.assign(invalidProps, changedProps); + changedProps = Object.assign(syncInfo.changedProps, changedProps); + hasPaths = hasPaths || syncInfo.hasPaths; } - runEffects(changedProps); + runEffects(changedProps, hasPaths); } else { - // Store off any values changed while `if` was false, along with the + // 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 - this.__invalidProps = Object.assign(this.__invalidProps || {}, changedProps); - this.__squelchedRunEffects = runEffects; + if (syncInfo) { + syncInfo.hasPaths = syncInfo.hasPaths || hasPaths; + Object.assign(syncInfo.changedProps, changedProps); + } else { + this.__syncInfo = { + runEffects, + changedProps: Object.assign({}, changedProps), + hasPaths + }; + } } }; // Stamp the template, and set its DocumentFragment to the "instance" @@ -415,10 +423,10 @@ class DomIfFast extends DomIfBase { * @return {void} */ __syncHostProperties() { - const props = this.__invalidProps; - if (props) { - this.__squelchedRunEffects(this.__invalidProps); - this.__invalidProps = this.__squelchedRunEffects = null; + const syncInfo = this.__syncInfo; + if (syncInfo) { + syncInfo.runEffects(syncInfo.changedProps, syncInfo.hasPaths); + this.__syncInfo = null; } } diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index a939d2ac0d..19c8cec33f 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -1978,7 +1978,7 @@ export const PropertyEffects = dedupingMixin(superClass => { } _runEffectsForTemplate(templateInfo, changedProps, oldProps, hasPaths) { - const baseRunEffects = (changedProps) => { + const baseRunEffects = (changedProps, hasPaths) => { runEffects(this, templateInfo.propertyEffects, changedProps, oldProps, hasPaths, templateInfo.nodeList); for (let info=templateInfo.firstChild; info; info=info.nextSibling) { @@ -1986,9 +1986,9 @@ export const PropertyEffects = dedupingMixin(superClass => { } }; if (templateInfo.runEffects) { - templateInfo.runEffects(baseRunEffects, changedProps); + templateInfo.runEffects(baseRunEffects, changedProps, hasPaths); } else { - baseRunEffects(changedProps); + baseRunEffects(changedProps, hasPaths); } } diff --git a/test/unit/dom-if-elements.js b/test/unit/dom-if-elements.js index 5f851548ca..2cebd4e6e4 100644 --- a/test/unit/dom-if-elements.js +++ b/test/unit/dom-if-elements.js @@ -301,9 +301,25 @@ Polymer({ return val; } }); + +Polymer({ + is: 'prop-observer', + properties: { + prop: { + observer: 'propChanged' + } + }, + created() { + this.propChanged = sinon.spy(); + } +}); + Polymer({ _template: html` - + `, is: 'x-guard-separate-props', diff --git a/test/unit/dom-if.html b/test/unit/dom-if.html index 8e45135e65..4a1b5cf75d 100644 --- a/test/unit/dom-if.html +++ b/test/unit/dom-if.html @@ -872,18 +872,26 @@ let el = document.createElement('x-guard-separate-props'); el.restamp = restamp; document.body.appendChild(el); - el.a = 'ok'; el.b = true; flush(); + el.a = 'ok'; + el.c = {d: 'ok'}; assert.equal(el.shadowRoot.textContent.trim(), 'ok'); + assert.equal(el.shadowRoot.querySelector('#observer').propChanged.callCount, 1); 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); + } + 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); document.body.removeChild(el); });