Skip to content

Commit

Permalink
Ensure hasPaths is also accumulated as part of info necessary to sync.
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinpschaaf committed Jul 9, 2019
1 parent 3d09455 commit 89d7055
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 20 deletions.
38 changes: 23 additions & 15 deletions lib/elements/dom-if.js
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,7 @@ class DomIfFast extends DomIfBase {
constructor() {
super();
this.__instance = null;
this.__invalidProps = null;
this.__squelchedRunEffects = null;
this.__syncInfo = null;
}

/**
Expand Down Expand Up @@ -378,29 +377,38 @@ 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
// pre-hidden values if `_showHideChildren` were to be called later at
// 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"
Expand All @@ -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;
}
}

Expand Down
6 changes: 3 additions & 3 deletions lib/mixins/property-effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -1978,17 +1978,17 @@ 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) {
this._runEffectsForTemplate(info, changedProps, oldProps, hasPaths);
}
};
if (templateInfo.runEffects) {
templateInfo.runEffects(baseRunEffects, changedProps);
templateInfo.runEffects(baseRunEffects, changedProps, hasPaths);
} else {
baseRunEffects(changedProps);
baseRunEffects(changedProps, hasPaths);
}
}

Expand Down
18 changes: 17 additions & 1 deletion test/unit/dom-if-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,25 @@ Polymer({
return val;
}
});

Polymer({
is: 'prop-observer',
properties: {
prop: {
observer: 'propChanged'
}
},
created() {
this.propChanged = sinon.spy();
}
});

Polymer({
_template: html`
<template is="dom-if" if="{{b}}" restamp="{{restamp}}">{{guarded(a)}}</template>
<template is="dom-if" if="{{b}}" restamp="{{restamp}}">
{{guarded(a)}}
<prop-observer id="observer" prop="[[c.d]]"></prop-observer>
</template>
`,

is: 'x-guard-separate-props',
Expand Down
10 changes: 9 additions & 1 deletion test/unit/dom-if.html
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down

0 comments on commit 89d7055

Please sign in to comment.