Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Process paths regardless of accessor, & loop on computeLinkedPaths. #4546

Merged
merged 3 commits into from
Apr 19, 2017
Merged

Process paths regardless of accessor, & loop on computeLinkedPaths. #4546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e2d1702
Process paths regardless of accessor, & loop on computeLinkedPaths.
kevinpschaaf Apr 18, 2017
d722cb9
Move computeLinkedPaths out of hot path and into sync setter.
kevinpschaaf Apr 19, 2017
73df8c5
Add more tests.
kevinpschaaf Apr 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 34 additions & 44 deletions lib/mixins/property-effects.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,29 +409,22 @@
* API.
*
* @param {Element} inst The instance whose props are changing
* @param {Object} changedProps Bag of changed properties
* @param {boolean} hasPaths True with `props` contains one or more paths
* @param {string} path Path that has changed
* @param {*} value Value of changed path
* @private
*/
function computeLinkedPaths(inst, changedProps, hasPaths) {
let links;
if (hasPaths && (links = inst.__dataLinkedPaths)) {
const cache = inst.__dataTemp;
function computeLinkedPaths(inst, path, value) {
let links = inst.__dataLinkedPaths;
if (links) {
let link;
for (let a in links) {
let b = links[a];
for (let path in changedProps) {
if (Polymer.Path.isDescendant(a, path)) {
link = Polymer.Path.translate(a, b, path);
cache[link] = changedProps[link] = changedProps[path];
let notifyProps = inst.__dataToNotify || (inst.__dataToNotify = {});
notifyProps[link] = true;
} else if (Polymer.Path.isDescendant(b, path)) {
link = Polymer.Path.translate(b, a, path);
cache[link] = changedProps[link] = changedProps[path];
let notifyProps = inst.__dataToNotify || (inst.__dataToNotify = {});
notifyProps[link] = true;
}
if (Polymer.Path.isDescendant(a, path)) {
link = Polymer.Path.translate(a, b, path);
inst._setPendingPropertyOrPath(link, value, true, true);
} else if (Polymer.Path.isDescendant(b, path)) {
link = Polymer.Path.translate(b, a, path);
inst._setPendingPropertyOrPath(link, value, true, true);
}
}
}
Expand Down Expand Up @@ -1291,33 +1284,32 @@
* @protected
*/
_setPendingPropertyOrPath(path, value, shouldNotify, isPathNotification) {
let rootProperty = Polymer.Path.root(Array.isArray(path) ? path[0] : path);
let hasAccessor = this.__dataHasAccessor && this.__dataHasAccessor[rootProperty];
let isPath = (rootProperty !== path);
if (hasAccessor) {
if (isPath) {
if (!isPathNotification) {
// Dirty check changes being set to a path against the actual object,
// since this is the entry point for paths into the system; from here
// the only dirty checks are against the `__dataTemp` cache to prevent
// duplicate work in the same turn only. Note, if this was a notification
// of a change already set to a path (isPathNotification: true),
// we always let the change through and skip the `set` since it was
// already dirty checked at the point of entry and the underlying
// object has already been updated
let old = Polymer.Path.get(this, path);
path = /** @type {string} */ (Polymer.Path.set(this, path, value));
// Use property-accessor's simpler dirty check
if (!path || !super._shouldPropertyChange(path, value, old)) {
return false;
}
if (isPathNotification ||
Polymer.Path.root(Array.isArray(path) ? path[0] : path) !== path) {
// Dirty check changes being set to a path against the actual object,
// since this is the entry point for paths into the system; from here
// the only dirty checks are against the `__dataTemp` cache to prevent
// duplicate work in the same turn only. Note, if this was a notification
// of a change already set to a path (isPathNotification: true),
// we always let the change through and skip the `set` since it was
// already dirty checked at the point of entry and the underlying
// object has already been updated
if (!isPathNotification) {
let old = Polymer.Path.get(this, path);
path = /** @type {string} */ (Polymer.Path.set(this, path, value));
// Use property-accessor's simpler dirty check
if (!path || !super._shouldPropertyChange(path, value, old)) {
return false;
}
this.__dataHasPaths = true;
}
return this._setPendingProperty(path, value, shouldNotify);
this.__dataHasPaths = true;
if (this._setPendingProperty(path, value, shouldNotify)) {
computeLinkedPaths(this, path, value);
return true;
}
} else {
if (isPath) {
Polymer.Path.set(this, path, value);
if (this.__dataHasAccessor && this.__dataHasAccessor[path]) {
return this._setPendingProperty(path, value, shouldNotify);
} else {
this[path] = value;
}
Expand Down Expand Up @@ -1548,8 +1540,6 @@
this.__dataHasPaths = false;
// Compute properties
runComputedEffects(this, changedProps, oldProps, hasPaths);
// Compute linked paths
computeLinkedPaths(this, changedProps, hasPaths);
// Clear notify properties prior to possible reentry (propagate, observe),
// but after computing effects have a chance to add to them
let notifyProps = this.__dataToNotify;
Expand Down
6 changes: 6 additions & 0 deletions test/unit/path-effects-elements.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@
computedFromPaths: {
computed: 'computeFromPaths(a, nested.b, nested.obj.c)'
},
computedFromLinkedPaths: {
computed: 'computeFromLinkedPaths(a, linked1.prop, linked2.prop)'
},
computed: {
computed: 'compute(nested.obj.value)'
}
Expand Down Expand Up @@ -163,6 +166,9 @@
computeFromPaths: function(a, b, c) {
return a + b + c;
},
computeFromLinkedPaths: sinon.spy(function(a, b, c) {
return a + b + c;
}),
compute: function(val) {
return '[' + val + ']';
}
Expand Down
71 changes: 69 additions & 2 deletions test/unit/path-effects.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -898,11 +898,78 @@
assert.equal(el.xChanged.callCount, 1);
assert.equal(el.yChanged.callCount, 1);
assert.equal(el.aChanged.callCount, 1);
el.unlinkPaths('y');
el.set('a.foo', 2);
assert.equal(el.xChanged.callCount, 2);
assert.equal(el.yChanged.callCount, 1);
assert.equal(el.yChanged.callCount, 2);
assert.equal(el.aChanged.callCount, 2);
el.unlinkPaths('y');
el.set('a.foo', 3);
assert.equal(el.xChanged.callCount, 3);
assert.equal(el.yChanged.callCount, 2);
assert.equal(el.aChanged.callCount, 3);
});

test('multiple linked dependencies to computed property', function() {
let linkedObj = {prop: 'Linked'};
el.computeFromLinkedPaths.reset();
el.setProperties({
linked1: linkedObj,
linked2: linkedObj,
a: 'A'
});
assert.equal(el.computeFromLinkedPaths.callCount, 1);
assert.equal(el.computedFromLinkedPaths, 'ALinkedLinked');

el.linkPaths('linked1', 'linked2');
el.set('linked1.prop', 'Linked+');
assert.equal(el.computeFromLinkedPaths.callCount, 2);
assert.equal(el.computedFromLinkedPaths, 'ALinked+Linked+');
el.linked3 = el.linked2;
el.linkPaths('linked2', 'linked3');
el.set('linked3.prop', 'Linked++');
assert.equal(el.computeFromLinkedPaths.callCount, 3);
assert.equal(el.computedFromLinkedPaths, 'ALinked++Linked++');
el.set('linked2.prop', 'Linked+++');
assert.equal(el.computeFromLinkedPaths.callCount, 4);
assert.equal(el.computedFromLinkedPaths, 'ALinked+++Linked+++');
el.set('linked1.prop', 'Linked++++');
assert.equal(el.computeFromLinkedPaths.callCount, 5);
assert.equal(el.computedFromLinkedPaths, 'ALinked++++Linked++++');

el.linked4 = el.linked1;
el.linkPaths('linked4', 'linked1');
el.set('linked4.prop', 'Linked+++++');
assert.equal(el.computeFromLinkedPaths.callCount, 6);
assert.equal(el.computedFromLinkedPaths, 'ALinked+++++Linked+++++');
el.set('linked3.prop', 'Linked++++++');
assert.equal(el.computeFromLinkedPaths.callCount, 7);
assert.equal(el.computedFromLinkedPaths, 'ALinked++++++Linked++++++');
el.set('linked2.prop', 'Linked+++++++');
assert.equal(el.computeFromLinkedPaths.callCount, 8);
assert.equal(el.computedFromLinkedPaths, 'ALinked+++++++Linked+++++++');
el.set('linked1.prop', 'Linked++++++++');
assert.equal(el.computeFromLinkedPaths.callCount, 9);
assert.equal(el.computedFromLinkedPaths, 'ALinked++++++++Linked++++++++');

el.linked5 = el.linked3;
el.linkPaths('linked5', 'linked3');
el.set('linked4.prop', 'Linked+++++++++');
assert.equal(el.computeFromLinkedPaths.callCount, 10);
assert.equal(el.computedFromLinkedPaths, 'ALinked+++++++++Linked+++++++++');
el.set('linked3.prop', 'Linked++++++++++');
assert.equal(el.computeFromLinkedPaths.callCount, 11);
assert.equal(el.computedFromLinkedPaths, 'ALinked++++++++++Linked++++++++++');
el.set('linked2.prop', 'Linked+++++++++++');
assert.equal(el.computeFromLinkedPaths.callCount, 12);
assert.equal(el.computedFromLinkedPaths, 'ALinked+++++++++++Linked+++++++++++');
el.set('linked1.prop', 'Linked++++++++++++');
assert.equal(el.computeFromLinkedPaths.callCount, 13);
assert.equal(el.computedFromLinkedPaths, 'ALinked++++++++++++Linked++++++++++++');

el.unlinkPaths('linked4');
el.set('linked4.prop', 'Linked+++++++++++++');
assert.equal(el.computeFromLinkedPaths.callCount, 13);
assert.equal(el.computedFromLinkedPaths, 'ALinked++++++++++++Linked++++++++++++');
});

test('link two arrays', function() {
Expand Down