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.

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

Build computed graph from dependencies, rather than properties. #5568

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 19 additions & 9 deletions lib/mixins/property-effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ function getComputedOrder(inst) {
if (!ordered) {
ordered = new Map();
const effects = inst[TYPES.COMPUTE];
const {counts, ready} = dependencyCounts(inst);
let {counts, ready, total} = dependencyCounts(inst);
let curr;
while ((curr = ready.shift())) {
ordered.set(curr, ordered.size);
Expand All @@ -548,12 +548,16 @@ function getComputedOrder(inst) {
computedByCurr.forEach(fx => {
// Note `methodInfo` is where the computed property name is stored
const computedProp = fx.info.methodInfo;
--total;
if (--counts[computedProp] === 0) {
ready.push(computedProp);
}
});
}
}
if (total !== 0) {
console.warn(`Computed graph for ${inst.localName} incomplete; circular?`);
}
inst.constructor.__orderedComputedDeps = ordered;
}
return ordered;
Expand All @@ -563,7 +567,8 @@ function getComputedOrder(inst) {
* Generates a map of property-to-dependency count (`counts`, where "dependency
* count" is the number of dependencies a given property has assuming it is a
* computed property, otherwise 0). It also returns a pre-populated list of
* `ready` properties that have no dependencies.
* `ready` properties that have no dependencies and a `total` count, which is
* used for error-checking the graph.
*
* Used by `orderedComputed: true` computed property algorithm.
*
Expand All @@ -574,20 +579,25 @@ function getComputedOrder(inst) {
* dependencies.
*/
function dependencyCounts(inst) {
const props = inst.constructor._properties;
const infoForComputed = inst[COMPUTE_INFO];
const counts = {};
const computedDeps = inst[TYPES.COMPUTE];
const ready = [];
for (let p in props) {
let total = 0;
// Count dependencies for each computed property
for (let p in infoForComputed) {
const info = infoForComputed[p];
if (info) {
// Be sure to add the method name itself in case of "dynamic functions"
counts[p] = info.args.length + (info.dynamicFn ? 1 : 0);
} else {
// Be sure to add the method name itself in case of "dynamic functions"
total += counts[p] =
info.args.filter(a => !a.literal).length + (info.dynamicFn ? 1 : 0);
}
// Build list of ready properties (that aren't themselves computed)
for (let p in computedDeps) {
if (!infoForComputed[p]) {
ready.push(p);
}
}
return {counts, ready};
return {counts, ready, total};
}

/**
Expand Down
3 changes: 2 additions & 1 deletion test/unit/property-effects-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ customElements.define('x-computed-ordering', class extends PolymerElement {
static get properties() {
return {
a: {type: Number, value: 1000},
b: {type: Number, value: 100},
// b: {type: Number, value: 100}, // Intentionally undeclared; init in ctor
c: {type: Number, value: 10},
d: {type: Number, value: 1},
abbcd: {computed: 'computeABBCD(a, b, bcd)', observer: 'abbcdChanged'},
Expand All @@ -1034,6 +1034,7 @@ customElements.define('x-computed-ordering', class extends PolymerElement {
}
constructor() {
super();
this.b = 100;
sinon.spy(this, 'computeABBCD');
sinon.spy(this, 'computeBCD');
sinon.spy(this, 'computeBC');
Expand Down
2 changes: 1 addition & 1 deletion test/unit/property-effects.html
Original file line number Diff line number Diff line change
Expand Up @@ -2091,7 +2091,7 @@

if (orderedComputed) {

suite.only('computed property ordering', function() {
suite('computed property ordering', function() {
var el;

setup(function() {
Expand Down