Skip to content

Commit

Permalink
Fixes #4585. Data notifications do not flush host if host has not ini…
Browse files Browse the repository at this point in the history
…tialized clients. This preserves the Polymer 1.x guarantee that client dom is fully “readied” when data observers run.
  • Loading branch information
Steven Orvell committed May 5, 2017
1 parent a284d77 commit 3b6981d
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 4 deletions.
6 changes: 5 additions & 1 deletion lib/mixins/element-mixin.html
Expand Up @@ -656,6 +656,10 @@
hostStack.endHosting(this);
this.$ = this.root.$;
}
// The super.ready here makes this element able to observe data changes.
// We must wait to do this until after client dom is created/attached
// so that any notifications fired during this process are not handled
// before all clients are ready.
super.ready();
}

Expand All @@ -669,10 +673,10 @@
* @override
*/
_readyClients() {
super._readyClients();
if (this._template) {
this.root = this._attachDom(this.root);
}
super._readyClients();
}


Expand Down
10 changes: 9 additions & 1 deletion lib/mixins/property-effects.html
Expand Up @@ -227,8 +227,11 @@
}
}
// Flush host if we actually notified and host was batching
// And the host has already initialized clients; this prevents
// an issue with a host observing data changes before clients are ready.
let host;
if (notified && (host = inst.__dataHost) && host._flushProperties) {
if (notified && (host = inst.__dataHost) && host._flushProperties
&& host.__dataClientsInitialized) {
host._flushProperties();
}
}
Expand Down Expand Up @@ -1509,6 +1512,11 @@
if (!this.__dataClientsInitialized) {
this._readyClients();
}
// Before ready, client notifications do not trigger _flushProperties.
// Therefore a flush is necessary here if data has been set.
if (this.__dataPending) {
this._flushProperties();
}
}

/**
Expand Down
25 changes: 23 additions & 2 deletions test/unit/property-effects-elements.html
Expand Up @@ -184,6 +184,9 @@
};
this.titleChanged = sinon.spy();
},
ready: function() {
this.isReady = true;
},
clearObserverCounts: function() {
for (var i in this.observerCounts) {
this.observerCounts[i] = 0;
Expand Down Expand Up @@ -344,7 +347,8 @@
'boundcomputedvalueChanged(boundcomputedvalue)',
'boundcomputednotifyingvalueChanged(boundcomputednotifyingvalue)',
'boundreadonlyvalueChanged(boundreadonlyvalue)',
'boundCustomNotifyingValueChanged(boundCustomNotifyingValue)'
'boundCustomNotifyingValueChanged(boundCustomNotifyingValue)',
'boundnotifyingvalueWithDefaultChanged(boundnotifyingvalueWithDefault)'
],
properties: {
a: {
Expand All @@ -367,7 +371,8 @@
boundcomputedvalueChanged: 0,
boundcomputednotifyingvalueChanged: 0,
boundreadonlyvalueChanged: 0,
boundCustomNotifyingValueChanged: 0
boundCustomNotifyingValueChanged: 0,
boundnotifyingvalueWithDefault: 0
};
},
computeComputedValue: function(a, b) {
Expand All @@ -378,23 +383,39 @@
this.observerCounts[i] = 0;
}
},
assertClientsReady: function() {
assert.isTrue(this.$.basic1.isReady, 'basic1 not `ready` by observer time');
assert.isTrue(this.$.basic2.isReady, 'basic2 element not `ready` by observer time');
assert.isTrue(this.$.basic3.isReady, 'basic3 element not `ready` by observer time');
assert.isTrue(this.$.basic4.isReady, 'basic4 element not `ready` by observer time');
},
boundvalueChanged: function() {
this.assertClientsReady();
this.observerCounts.boundvalueChanged++;
},
boundnotifyingvalueChanged: function() {
this.assertClientsReady();
this.observerCounts.boundnotifyingvalueChanged++;
},
boundcomputedvalueChanged: function() {
this.assertClientsReady();
this.observerCounts.boundcomputedvalueChanged++;
},
boundcomputednotifyingvalueChanged: function() {
this.assertClientsReady();
this.observerCounts.boundcomputednotifyingvalueChanged++;
},
boundreadonlyvalueChanged: function() {
this.assertClientsReady();
this.observerCounts.boundreadonlyvalueChanged++;
},
boundCustomNotifyingValueChanged: function() {
this.assertClientsReady();
this.observerCounts.boundCustomNotifyingValueChanged++;
},
boundnotifyingvalueWithDefaultChanged: function() {
this.assertClientsReady();
this.observerCounts.boundnotifyingvalueWithDefault++;
}
});
</script>
Expand Down

0 comments on commit 3b6981d

Please sign in to comment.