From 987ae2c4948ac180fe7dde48f07ee323ac7bb8fc Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 7 Feb 2019 12:24:16 -0800 Subject: [PATCH 001/146] Adds `legacyUndefined` setting If `legacyUndefined` is set, multi-property observers, computed properties, and bindings will not be executed unless none of their dependencies is `undefined`. This matches Polymer 1 behavior. --- lib/mixins/property-effects.js | 20 +++++++++++++++++--- lib/utils/settings.js | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index c92bea3fa6..dccf2a0051 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -19,12 +19,14 @@ import { camelToDashCase, dashToCamelCase } from '../utils/case-map.js'; import { PropertyAccessors } from './property-accessors.js'; /* for annotated effects */ import { TemplateStamp } from './template-stamp.js'; -import { sanitizeDOMValue } from '../utils/settings.js'; +import { sanitizeDOMValue, legacyUndefined } from '../utils/settings.js'; // Monotonically increasing unique ID used for de-duping effects triggered // from multiple properties in the same turn let dedupeId = 0; +const NOOP = {}; + /** * Property effect types; effects are stored on the prototype using these keys * @enum {string} @@ -432,6 +434,10 @@ function runComputedEffects(inst, changedProps, oldProps, hasPaths) { */ function runComputedEffect(inst, property, props, oldProps, info) { let result = runMethodEffect(inst, property, props, oldProps, info); + // Abort if method returns a no-op result + if (result === NOOP) { + return; + } let computedProp = info.methodInfo; if (inst.__dataHasAccessor && inst.__dataHasAccessor[computedProp]) { inst._setPendingProperty(computedProp, result, true); @@ -579,7 +585,10 @@ function runBindingEffect(inst, path, props, oldProps, info, hasPaths, nodeList) } else { let value = info.evaluator._evaluateBinding(inst, part, path, props, oldProps, hasPaths); // Propagate value to child - applyBindingValue(inst, node, binding, part, value); + // Abort if value is a no-op result + if (value !== NOOP) { + applyBindingValue(inst, node, binding, part, value); + } } } @@ -816,7 +825,7 @@ function runMethodEffect(inst, property, props, oldProps, info) { let fn = context[info.methodName]; if (fn) { let args = inst._marshalArgs(info.args, property, props); - return fn.apply(context, args); + return args === NOOP ? NOOP : fn.apply(context, args); } else if (!info.dynamicFn) { console.warn('method `' + info.methodName + '` not defined'); } @@ -2215,6 +2224,11 @@ export const PropertyEffects = dedupingMixin(superClass => { value = structured ? getArgValue(data, props, name) : data[name]; } } + // When the `legacyUndefined` flag is enabled, pass a no-op value + // so that the observer, computed property, or compound binding is aborted. + if (legacyUndefined && value === undefined) { + return NOOP; + } values[i] = value; } return values; diff --git a/lib/utils/settings.js b/lib/utils/settings.js index 6b878c4ed1..4d99402006 100644 --- a/lib/utils/settings.js +++ b/lib/utils/settings.js @@ -158,3 +158,22 @@ export let syncInitialRender = false; export const setSyncInitialRender = function(useSyncInitialRender) { syncInitialRender = useSyncInitialRender; }; + +/** + * Setting to retain the legacy Polymer 1 behavior for multi-property + * observers around undefined values. Observers and computed property methods + * are not called until no argument is undefined. + */ +export let legacyUndefined = false; + +/** + * Sets `legacyUndefined` globally for all elements to enable legacy + * multi-property behavior for undefined values. + * + * @param {boolean} useLegacyUndefined enable or disable legacy + * multi-property behavior for undefined. + * @return {void} + */ +export const setLegacyUndefined = function(useLegacyUndefined) { + legacyUndefined = useLegacyUndefined; +}; From 52a559fcec2a70f101a0d1473ff84792f21e3f94 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 7 Feb 2019 12:40:39 -0800 Subject: [PATCH 002/146] Add tests for `legacyUndefined` setting --- test/runner.html | 1 + test/unit/legacy-undefined.html | 343 ++++++++++++++++++++++++++++++++ 2 files changed, 344 insertions(+) create mode 100644 test/unit/legacy-undefined.html diff --git a/test/runner.html b/test/runner.html index 338dd11253..8f67390d67 100644 --- a/test/runner.html +++ b/test/runner.html @@ -87,6 +87,7 @@ 'unit/shady-unscoped-style.html', 'unit/html-tag.html', 'unit/legacy-data.html', + 'unit/legacy-undefined.html', // 'unit/multi-style.html' 'unit/class-properties.html' ]; diff --git a/test/unit/legacy-undefined.html b/test/unit/legacy-undefined.html new file mode 100644 index 0000000000..2afffd7cb8 --- /dev/null +++ b/test/unit/legacy-undefined.html @@ -0,0 +1,343 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 363bef2cc1d525f7b706dbaf04ee4cb8d5f3a16e Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 7 Feb 2019 17:17:46 -0800 Subject: [PATCH 003/146] Adds `legacyNoBatch` setting Avoids batching properties after startup to more closely match Polymer 1 behavior. --- lib/mixins/property-accessors.js | 10 +++++++++- lib/mixins/property-effects.js | 5 ++++- lib/utils/settings.js | 18 ++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/mixins/property-accessors.js b/lib/mixins/property-accessors.js index bfa7f33a7e..bd2cb77b8a 100644 --- a/lib/mixins/property-accessors.js +++ b/lib/mixins/property-accessors.js @@ -12,6 +12,7 @@ import '../utils/boot.js'; import { dedupingMixin } from '../utils/mixin.js'; import { camelToDashCase, dashToCamelCase } from '../utils/case-map.js'; import { PropertiesChanged } from './properties-changed.js'; +import { legacyNoBatch } from '../utils/settings.js'; // Save map of native properties; this forms a blacklist or properties // that won't have their values "saved" by `saveAccessorValue`, since @@ -289,7 +290,14 @@ export const PropertyAccessors = dedupingMixin(superClass => { * @override */ _definePropertyAccessor(property, readOnly) { - saveAccessorValue(this, property); + if (legacyNoBatch) { + const ready = this.__dataReady; + this.__dataReady = false; + saveAccessorValue(this, property); + this.__dataReady = ready; + } else { + saveAccessorValue(this, property); + } super._definePropertyAccessor(property, readOnly); } diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index dccf2a0051..e7412d47a4 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -19,7 +19,7 @@ import { camelToDashCase, dashToCamelCase } from '../utils/case-map.js'; import { PropertyAccessors } from './property-accessors.js'; /* for annotated effects */ import { TemplateStamp } from './template-stamp.js'; -import { sanitizeDOMValue, legacyUndefined } from '../utils/settings.js'; +import { sanitizeDOMValue, legacyUndefined, legacyNoBatch } from '../utils/settings.js'; // Monotonically increasing unique ID used for de-duping effects triggered // from multiple properties in the same turn @@ -1481,6 +1481,9 @@ export const PropertyEffects = dedupingMixin(superClass => { this.__dataToNotify = this.__dataToNotify || {}; this.__dataToNotify[property] = shouldNotify; } + if (legacyNoBatch) { + this._invalidateProperties(); + } return true; } return false; diff --git a/lib/utils/settings.js b/lib/utils/settings.js index 4d99402006..1f37b9e354 100644 --- a/lib/utils/settings.js +++ b/lib/utils/settings.js @@ -177,3 +177,21 @@ export let legacyUndefined = false; export const setLegacyUndefined = function(useLegacyUndefined) { legacyUndefined = useLegacyUndefined; }; + +/** + * Setting to retain the legacy Polymer 1 behavior for setting properties. Does + * not batch property sets. + */ +export let legacyNoBatch = false; + +/** + * Sets `legacyNoBatch` globally for all elements to enable legacy + * behavior for setting properties. + * + * @param {boolean} useLegacyNoBatch enable or disable legacy + * property setting behavior. + * @return {void} + */ +export const setLegacyNoBatch = function(useLegacyNoBatch) { + legacyNoBatch = useLegacyNoBatch; +}; From 6139e8893a281f45554337a8a8f059c98b7d58f0 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 13 Feb 2019 18:16:52 -0800 Subject: [PATCH 004/146] Add arg length check --- lib/mixins/property-effects.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index dccf2a0051..5e0e2b753d 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -2226,7 +2226,7 @@ export const PropertyEffects = dedupingMixin(superClass => { } // When the `legacyUndefined` flag is enabled, pass a no-op value // so that the observer, computed property, or compound binding is aborted. - if (legacyUndefined && value === undefined) { + if (legacyUndefined && value === undefined && args.length > 1) { return NOOP; } values[i] = value; From 32c308372c469a07a5c6f63ff87f280cca97201c Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 14 Feb 2019 09:27:15 -0800 Subject: [PATCH 005/146] Fix test With `legacyUndefined` only multi-property effects should not be run when an argument is undefined; single property effects should always run. --- test/unit/legacy-undefined.html | 117 ++++++++++++++++---------------- 1 file changed, 58 insertions(+), 59 deletions(-) diff --git a/test/unit/legacy-undefined.html b/test/unit/legacy-undefined.html index 2afffd7cb8..78d86304ce 100644 --- a/test/unit/legacy-undefined.html +++ b/test/unit/legacy-undefined.html @@ -211,7 +211,7 @@ setupElement({singleProp}); assertEffects({singlePropObserver: 1}); el.singleProp = undefined; - assertEffects({singlePropObserver: 1}); + assertEffects({singlePropObserver: 2}); }); test('one multiPropObserver arguments undefined', () => { setupElement({multiProp1, multiProp2}); @@ -277,65 +277,64 @@ teardown(() => console.warn.restore()); - suite('warn', () => { - test('no arguments defined', () => { - el = fixture('declarative-none'); - assertEffects({}); - }); - test('singlePropObserver argument defined', () => { - el = fixture('declarative-single'); - assertEffects({singlePropObserver: 1}); - }); - test('one multiPropObserver arguments defined', () => { - el = fixture('declarative-multi-one'); - assertEffects({multiPropObserver: 0}); - }); - test('all multiPropObserver defined', () => { - el = fixture('declarative-multi-all'); - assertEffects({multiPropObserver: 1}); - }); - test('computeSingle argument defined', () => { - el = fixture('declarative-single-computed'); - assertEffects({computeSingle: 1}); - assert.equal(el.computedSingle, '[a]'); - }); - test('one computeMulti arguments defined', () => { - el = fixture('declarative-multi-one-computed'); - assertEffects({computeMulti: 0}); - assert.equal(el.computedMulti, undefined); - }); - test('all computeMulti defined', () => { - el = fixture('declarative-multi-all-computed'); - assert.equal(el.computedMulti, '[b,c]'); - }); - test('inline computeSingle argument defined', () => { - el = fixture('declarative-single-computed-inline'); - assertEffects({computeSingle: 1}); - assert.equal(el.$.child.computedSingle, '[a]'); - }); - test('inline one computeMulti arguments defined', () => { - el = fixture('declarative-multi-one-computed-inline'); - assertEffects({computeMulti: 0}); - assert.equal(el.$.child.computedMulti, undefined); - }); - test('inline all computeMulti defined', () => { - el = fixture('declarative-multi-all-computed-inline'); - assertEffects({computeMulti: 1}); - assert.equal(el.$.child.computedMulti, '[b,c]'); - }); - test('one inline computeMulti argument defined in dom-if', () => { - el = fixture('declarative-multi-if-one-computed-inline'); - flush(); - assertEffects({computeMulti: 0}); - assert.equal(el.$$('#ifChild').computedMulti, undefined); - }); - test('all inline computeMulti argument defined in dom-if', () => { - el = fixture('declarative-multi-if-all-computed-inline'); - flush(); - assertEffects({computeMulti: 1}); - assert.equal(el.$$('#ifChild').computedMulti, '[b,c]'); - }); + test('no arguments defined', () => { + el = fixture('declarative-none'); + assertEffects({}); + }); + test('singlePropObserver argument defined', () => { + el = fixture('declarative-single'); + assertEffects({singlePropObserver: 1}); + }); + test('one multiPropObserver arguments defined', () => { + el = fixture('declarative-multi-one'); + assertEffects({multiPropObserver: 0}); }); + test('all multiPropObserver defined', () => { + el = fixture('declarative-multi-all'); + assertEffects({multiPropObserver: 1}); + }); + test('computeSingle argument defined', () => { + el = fixture('declarative-single-computed'); + assertEffects({computeSingle: 1}); + assert.equal(el.computedSingle, '[a]'); + }); + test('one computeMulti arguments defined', () => { + el = fixture('declarative-multi-one-computed'); + assertEffects({computeMulti: 0}); + assert.equal(el.computedMulti, undefined); + }); + test('all computeMulti defined', () => { + el = fixture('declarative-multi-all-computed'); + assert.equal(el.computedMulti, '[b,c]'); + }); + test('inline computeSingle argument defined', () => { + el = fixture('declarative-single-computed-inline'); + assertEffects({computeSingle: 1}); + assert.equal(el.$.child.computedSingle, '[a]'); + }); + test('inline one computeMulti arguments defined', () => { + el = fixture('declarative-multi-one-computed-inline'); + assertEffects({computeMulti: 0}); + assert.equal(el.$.child.computedMulti, undefined); + }); + test('inline all computeMulti defined', () => { + el = fixture('declarative-multi-all-computed-inline'); + assertEffects({computeMulti: 1}); + assert.equal(el.$.child.computedMulti, '[b,c]'); + }); + test('one inline computeMulti argument defined in dom-if', () => { + el = fixture('declarative-multi-if-one-computed-inline'); + flush(); + assertEffects({computeMulti: 0}); + assert.equal(el.$$('#ifChild').computedMulti, undefined); + }); + test('all inline computeMulti argument defined in dom-if', () => { + el = fixture('declarative-multi-if-all-computed-inline'); + flush(); + assertEffects({computeMulti: 1}); + assert.equal(el.$$('#ifChild').computedMulti, '[b,c]'); + }); + }); From e29a3150020b933790f78c8050d23f480f25bc7d Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Tue, 19 Feb 2019 12:31:04 -0800 Subject: [PATCH 006/146] Store splices directly on array when `legacyUndefined` is set Legacy behavior stored splices non-ephemerally. To most easily match this behavior, we store splices directly on the array. This was previously avoided because the property will show as an enumerable property on the array; however, this avoids the need to store and clear splices in a more bespoke way. --- lib/mixins/property-effects.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index e7412d47a4..626ace1e68 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -1008,8 +1008,18 @@ function getArgValue(data, props, path) { * @private */ function notifySplices(inst, array, path, splices) { - inst.notifyPath(path + '.splices', { indexSplices: splices }); + const splicesData = { indexSplices: splices }; + // Legacy behavior stored splices in `__data__` so it was *not* ephemeral. + // To match this behavior, we store splices directly on the array. + if (legacyUndefined) { + array.splices = splicesData; + } + inst.notifyPath(path + '.splices', splicesData); inst.notifyPath(path + '.length', array.length); + // Clear splice data only when it's stored on the array. + if (legacyUndefined) { + splicesData.indexSplices = []; + } } /** From c966eb1fa63c225f7b8f7d90e6e3e22a9b1f7594 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Tue, 19 Feb 2019 12:45:34 -0800 Subject: [PATCH 007/146] Avoid over-warning on templatizer props and "static" dynamicFns. --- lib/mixins/element-mixin.js | 5 ++++- lib/mixins/property-effects.js | 2 +- lib/mixins/template-stamp.js | 1 + test/unit/property-effects.html | 24 ++++++++++++++++++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/mixins/element-mixin.js b/lib/mixins/element-mixin.js index c931fb7fc0..415e2e2b3d 100644 --- a/lib/mixins/element-mixin.js +++ b/lib/mixins/element-mixin.js @@ -777,7 +777,10 @@ export const ElementMixin = dedupingMixin(base => { // The warning is only enabled in `legacyOptimizations` mode, since // we don't want to spam existing users who might have adopted the // shorthand when attribute deserialization is not important. - if (legacyOptimizations && !(prop in this._properties)) { + if (legacyOptimizations && !(prop in this._properties) && + effect.info && effect.info.part && + !(effect.info.part.signature && effect.info.part.signature.static) && + !effect.info.part.hostProp && !templateInfo.nestedTemplate) { console.warn(`Property '${prop}' used in template but not declared in 'properties'; ` + `attribute will not be observed.`); } diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index c92bea3fa6..65d76136be 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -2640,7 +2640,7 @@ export const PropertyEffects = dedupingMixin(superClass => { let hostProps = nodeInfo.templateInfo.hostProps; let mode = '{'; for (let source in hostProps) { - let parts = [{ mode, source, dependencies: [source] }]; + let parts = [{ mode, source, dependencies: [source], hostProp: true }]; addBinding(this, templateInfo, nodeInfo, 'property', '_host_' + source, parts); } return noted; diff --git a/lib/mixins/template-stamp.js b/lib/mixins/template-stamp.js index 167fbe530a..156bacee5f 100644 --- a/lib/mixins/template-stamp.js +++ b/lib/mixins/template-stamp.js @@ -204,6 +204,7 @@ export const TemplateStamp = dedupingMixin( if (!template._templateInfo) { let templateInfo = template._templateInfo = {}; templateInfo.nodeInfoList = []; + templateInfo.nestedTemplate = Boolean(outerTemplateInfo); templateInfo.stripWhiteSpace = (outerTemplateInfo && outerTemplateInfo.stripWhiteSpace) || template.hasAttribute('strip-whitespace'); diff --git a/test/unit/property-effects.html b/test/unit/property-effects.html index 087023914f..61a2a75083 100644 --- a/test/unit/property-effects.html +++ b/test/unit/property-effects.html @@ -1900,6 +1900,30 @@ assert.equal(console.warn.callCount, 3); }); + test('do not warn on valid non-property bindings', () => { + setLegacyOptimizations(true); + Polymer({ + is: 'x-nowarn-undeclared-binding', + properties: { + hostProp: String + }, + _template: html` +
[[noDeps()]]
+
[[hostProp]]
+ + `, + noDeps() { return 'static'; } + }); + const el = document.createElement('x-nowarn-undeclared-binding'); + document.body.appendChild(el); + document.body.removeChild(el); + assert.equal(console.warn.callCount, 0); + }); + test('warn when re-declaring a computed property', () => { Polymer({ is: 'x-warn-redeclared-computed', From 2874c86d8d85a2bf22d16340eb15d7ee79c75d71 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Tue, 19 Feb 2019 12:48:17 -0800 Subject: [PATCH 008/146] Remove unnecessary qualification. --- lib/mixins/element-mixin.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/mixins/element-mixin.js b/lib/mixins/element-mixin.js index 415e2e2b3d..976f18c755 100644 --- a/lib/mixins/element-mixin.js +++ b/lib/mixins/element-mixin.js @@ -778,7 +778,6 @@ export const ElementMixin = dedupingMixin(base => { // we don't want to spam existing users who might have adopted the // shorthand when attribute deserialization is not important. if (legacyOptimizations && !(prop in this._properties) && - effect.info && effect.info.part && !(effect.info.part.signature && effect.info.part.signature.static) && !effect.info.part.hostProp && !templateInfo.nestedTemplate) { console.warn(`Property '${prop}' used in template but not declared in 'properties'; ` + From add77842848226cff5c15d39e085a4e9f99be6f7 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Tue, 19 Feb 2019 23:53:16 -0800 Subject: [PATCH 009/146] Ensure properties are set one-by-one at startup. --- lib/mixins/property-effects.js | 48 ++++++++++-- test/runner.html | 2 + test/unit/property-effects-elements.js | 4 +- test/unit/property-effects.html | 102 +++++++++++++++++++------ 4 files changed, 126 insertions(+), 30 deletions(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index c7e68cff47..857e786fb9 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -622,7 +622,9 @@ function applyBindingValue(inst, node, binding, part, value) { inst._enqueueClient(node); } } - } else { + } else if (!legacyNoBatch || inst.__dataReady) { + // In legacy no-batching mode, bindings applied before dataReady are + // equivalent to the "apply config" phase, which only set managed props inst._setUnmanagedPropertyToNode(node, prop, value); } } @@ -1555,7 +1557,30 @@ export const PropertyEffects = dedupingMixin(superClass => { */ _flushProperties() { this.__dataCounter++; - super._flushProperties(); + if (legacyNoBatch && !this.__dataReady) { + // The first time this element is being flushed, propagate pending + // data down the tree first; this approximates 1.x's distributeConfig + this._propagatePropertyChanges(this.__dataPending, {}, false); + // Flushing clients causes this element to become __dataReady + this._flushClients(); + // Capture element data and flush properties one-by one to defeat + // Polymer 2.x's batched _propertiesChanged scheme; this approximates + // 1.x's applyConfig + const changedProps = this.__data; + const notify = this.__dataToNotify; + this.__data = {}; + this.__dataPending = this.__dataToNotify = null; + for (let prop in changedProps) { + const value = changedProps[prop]; + this.__data[prop] = value; + this.__dataPending = {[prop]: value}; + this.__dataOld = {}; + this.__dataToNotify = {[prop]: notify && notify[prop]}; + super._flushProperties(); + } + } else { + super._flushProperties(); + } this.__dataCounter--; } @@ -1698,12 +1723,21 @@ export const PropertyEffects = dedupingMixin(superClass => { // ---------------------------- let hasPaths = this.__dataHasPaths; this.__dataHasPaths = false; + let notifyProps; // Compute properties - runComputedEffects(this, changedProps, oldProps, 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; - this.__dataToNotify = null; + if (legacyNoBatch) { + // When not batching, computed effects may re-enter, so capture + // notifying properties early + notifyProps = this.__dataToNotify; + this.__dataToNotify = null; + runEffects(this, this[TYPES.COMPUTE], changedProps, oldProps, hasPaths); + } else { + runComputedEffects(this, changedProps, oldProps, hasPaths); + // Clear notify properties prior to possible reentry (propagate, observe), + // but after computing effects have a chance to add to them + notifyProps = this.__dataToNotify; + this.__dataToNotify = null; + } // Propagate properties to clients this._propagatePropertyChanges(changedProps, oldProps, hasPaths); // Flush clients diff --git a/test/runner.html b/test/runner.html index 8f67390d67..341b02899d 100644 --- a/test/runner.html +++ b/test/runner.html @@ -31,6 +31,8 @@ 'unit/template-stamp.html', 'unit/strict-template-policy.html', 'unit/property-effects.html', + 'unit/property-effects.html?legacyUndefined=true', + 'unit/property-effects.html?legacyUndefined=true&legacyNoBatch=true', 'unit/property-effects-template.html', 'unit/path-effects.html', 'unit/shady.html', diff --git a/test/unit/property-effects-elements.js b/test/unit/property-effects-elements.js index 7986f2a6f7..0e0f6094d6 100644 --- a/test/unit/property-effects-elements.js +++ b/test/unit/property-effects-elements.js @@ -298,7 +298,9 @@ Polymer({ }, $computeTrickyFunctionFromLiterals: function(num, str) { - return this.computeFromLiterals(num, str); + assert.equal(num, 3); + assert.equal(str, 'foo'); + return num + str; }, computeFromTrickyLiterals: function(a, b) { diff --git a/test/unit/property-effects.html b/test/unit/property-effects.html index 087023914f..37b211b85a 100644 --- a/test/unit/property-effects.html +++ b/test/unit/property-effects.html @@ -26,9 +26,13 @@ From 9ed318951cf39c4840af7ee352ff1e95ce9acbbe Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Mon, 4 Mar 2019 08:52:46 -0800 Subject: [PATCH 022/146] Minor comment updates --- lib/mixins/property-effects.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index c60a0341b0..c6164ffc38 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -82,7 +82,8 @@ let DataEffect; //eslint-disable-line no-unused-vars * * @param {Object} model Prototype or instance * @param {string} type Property effect type - * @param {boolean=} cloneArrays Clone any arrays mapped in the map + * @param {boolean=} cloneArrays Clone any arrays assigned to the map when + * extending a superclass map onto this subclass * @return {Object} The own-property map of effects for the given type * @private */ @@ -2356,7 +2357,11 @@ export const PropertyEffects = dedupingMixin(superClass => { throw new Error("Malformed computed expression '" + expression + "'"); } const info = createMethodEffect(this, sig, TYPES.COMPUTE, runComputedEffect, property, dynamicFn); - ensureOwnEffectMap(this, COMPUTE_INFO)[property] = info; + if (orderedComputed) { + // Effects are normally stored as map of dependency->effect, but for + // ordered computation, we also need tree of computedProp->dependencies + ensureOwnEffectMap(this, COMPUTE_INFO)[property] = info; + } } /** From ef0efa6ee6b990019741114486e949824ec88e89 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Tue, 5 Mar 2019 11:15:31 -0800 Subject: [PATCH 023/146] Add hasPaths optimziation --- lib/mixins/property-effects.js | 39 +++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index c6164ffc38..11b6ced9dc 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -439,20 +439,21 @@ const TRANSITIVE_DEPENDENCY = '~transitive~dependency~'; * @param {?Object} changedProps Bag of current property changes * @param {?Object} oldProps Bag of previous values for changed properties * @param {?} info Effect metadata + * @param {boolean} hasPaths True with `changedProps` contains one or more paths * @return {void} * @private */ -function runComputedEffect(inst, property, changedProps, oldProps, info) { +function runComputedEffect(inst, property, changedProps, oldProps, info, hasPaths) { if (orderedComputed) { // Compute any computed dependencies first; this recurses through the // dependency graph to ensure computed properties are never computed with // dependencies that may become invalidated later in this turn - computeDependencies(inst, info, changedProps, oldProps); + computeDependencies(inst, info, changedProps, oldProps, hasPaths); // If the dependency was not transitive, it's definitely dirty and needs to // be computed; if it is transitive, check its arguments against the current // changed props and only re-compute if it is dirty if (property === TRANSITIVE_DEPENDENCY && - !computedPropertyIsDirty(inst, info, changedProps)) { + !computedPropertyIsDirty(inst, info, changedProps, hasPaths)) { return; } } @@ -502,16 +503,17 @@ function runComputedEffect(inst, property, changedProps, oldProps, info) { * @param {Object} info Effect metadata * @param {Object} changedProps Bag of current property changes * @param {Object} oldProps Bag of previous values for changed properties + * @param {boolean} hasPaths True with `changedProps` contains one or more paths * @return {void} * @private */ - function computeDependencies(inst, info, changedProps, oldProps) { + function computeDependencies(inst, info, changedProps, oldProps, hasPaths) { let deps = computedDependenciesFor(inst, info); if (deps.length) { deps.forEach(depInfo => { if (depInfo.lastRun !== info.lastRun) { depInfo.lastRun = info.lastRun; - runComputedEffect(inst, TRANSITIVE_DEPENDENCY, changedProps, oldProps, depInfo); + runComputedEffect(inst, TRANSITIVE_DEPENDENCY, changedProps, oldProps, depInfo, hasPaths); } }); } @@ -523,12 +525,13 @@ function runComputedEffect(inst, property, changedProps, oldProps, info) { * @param {!Polymer_PropertyEffects} inst The instance to test * @param {?} info Effect metadata * @param {Object} changedProps Bag of current property changes + * @param {boolean} hasPaths True with `changedProps` contains one or more paths * @return {boolean} true when the computed effect should be re-calculated * @private */ -function computedPropertyIsDirty(inst, info, changedProps) { - return info.dynamicFn && propertyIsDirty(inst, {name: info.methodName}, changedProps) || - info.args.some(arg => propertyIsDirty(inst, arg, changedProps)); +function computedPropertyIsDirty(inst, info, changedProps, hasPaths) { + return info.dynamicFn && propertyIsDirty(inst, {rootProperty: info.methodName}, changedProps, hasPaths) || + info.args.some(arg => propertyIsDirty(inst, arg, changedProps, hasPaths)); } /** @@ -537,17 +540,23 @@ function computedPropertyIsDirty(inst, info, changedProps) { * @param {!Polymer_PropertyEffects} inst The instance to test * @param {DataTrigger} trigger Descriptor * @param {Object} changedProps Bag of current property changes + * @param {boolean} hasPaths True with `changedProps` contains one or more paths * @return {boolean} true when the property is dirty * @private */ -function propertyIsDirty(inst, trigger, changedProps) { - return [changedProps, inst.__dataPending].some(props => { - for (let p in props) { - if (pathMatchesTrigger(p, trigger)) { - return true; +function propertyIsDirty(inst, trigger, changedProps, hasPaths) { + if (hasPaths) { + [changedProps, inst.__dataPending].some(props => { + for (let p in props) { + if (pathMatchesTrigger(p, trigger)) { + return true; + } } - } - }); + }); + } else { + return changedProps && trigger.rootProperty in changedProps || + inst.__dataPending && trigger.rootProperty in inst.__dataPending; + } } /** From 42dd361f942fb57b700d2dac7b97bb49491aa0d9 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Thu, 7 Mar 2019 18:12:26 -0800 Subject: [PATCH 024/146] Fix closure issues --- externs/polymer-externs.js | 3 +++ lib/mixins/property-effects.js | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/externs/polymer-externs.js b/externs/polymer-externs.js index 16fe1eaf33..a45439ae94 100644 --- a/externs/polymer-externs.js +++ b/externs/polymer-externs.js @@ -147,6 +147,9 @@ Polymer.legacyWarnings; /** @type {boolean} */ Polymer.legacyNotifyOrder; +/** @type {boolean} */ +Polymer.orderedComputed; + // nb. This is explicitly 'var', as Closure Compiler checks that this is the case. /** * @constructor diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index 11b6ced9dc..de2d153cb4 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -553,9 +553,10 @@ function propertyIsDirty(inst, trigger, changedProps, hasPaths) { } } }); + return false; } else { - return changedProps && trigger.rootProperty in changedProps || - inst.__dataPending && trigger.rootProperty in inst.__dataPending; + return Boolean(changedProps && trigger.rootProperty in changedProps || + inst.__dataPending && trigger.rootProperty in inst.__dataPending); } } From fc49a9255972127335f8db2b8430afd7f4b16edc Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Mon, 11 Mar 2019 18:20:59 -0700 Subject: [PATCH 025/146] Dedupe against a single turn on only under orderedComputed --- lib/mixins/property-effects.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index de2d153cb4..f946014a10 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -421,8 +421,10 @@ function runComputedEffects(inst, changedProps, oldProps, hasPaths) { Object.assign(/** @type {!Object} */ (changedProps), inst.__dataPending); inputProps = inst.__dataPending; inst.__dataPending = null; - // Ensure all computed properties are de-duped against the same turn - dedupeId--; + if (orderedComputed) { + // Ensure all computed properties are de-duped against the same turn + dedupeId--; + } } } } From 7cda770e4cf304522bbba3e90d3015e9583c463d Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 10 Apr 2019 17:11:13 -0700 Subject: [PATCH 026/146] [wip] Add additional topo-sort based algorithm. --- lib/mixins/property-effects.js | 121 +++++++++++++++++++++++++++++---- 1 file changed, 107 insertions(+), 14 deletions(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index f946014a10..fc82d767db 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -415,18 +415,113 @@ function runReflectEffect(inst, property, props, oldProps, info) { function runComputedEffects(inst, changedProps, oldProps, hasPaths) { let computeEffects = inst[TYPES.COMPUTE]; if (computeEffects) { - let inputProps = changedProps; - while (runEffects(inst, computeEffects, inputProps, oldProps, hasPaths)) { + if (orderedComputed == 2) { + dedupeId++; + const order = orderedComputedDeps(inst); + const queue = []; + for (let p in changedProps) { + addEffectsFor(p, computeEffects, queue, order, hasPaths); + } + let info; + while ((info = queue.shift())) { + if (runComputedEffect(inst, '', changedProps, oldProps, info, hasPaths)) { + addEffectsFor(info.methodInfo, computeEffects, queue, order, hasPaths); + } + } Object.assign(/** @type {!Object} */ (oldProps), inst.__dataOld); Object.assign(/** @type {!Object} */ (changedProps), inst.__dataPending); - inputProps = inst.__dataPending; inst.__dataPending = null; - if (orderedComputed) { - // Ensure all computed properties are de-duped against the same turn - dedupeId--; + } else { + let inputProps = changedProps; + while (runEffects(inst, computeEffects, inputProps, oldProps, hasPaths)) { + Object.assign(/** @type {!Object} */ (oldProps), inst.__dataOld); + Object.assign(/** @type {!Object} */ (changedProps), inst.__dataPending); + inputProps = inst.__dataPending; + inst.__dataPending = null; + if (orderedComputed) { + // Ensure all computed properties are de-duped against the same turn + dedupeId--; + } + } + } + } +} + +const insertEffect = (info, effects, order) => { + let start = 0; + let end = effects.length - 1; + let idx = -1; + while (start <= end) { + const mid = (start + end) >> 1; + const cmp = order.get(effects[mid].methodInfo) - order.get(info.methodInfo); + if (cmp < 0) { + start = mid + 1; + } else if (cmp > 0) { + end = mid - 1; + } else { + idx = mid; + break; + } + } + if (idx < 0) { + idx = end + 1; + } + effects.splice(idx, 0, info); +}; + +const addEffectsFor = (prop, computeEffects, queue, order, hasPaths) => { + const rootProperty = hasPaths ? root(prop) : prop; + const fxs = computeEffects[rootProperty]; + if (fxs) { + for (let i=0; i { + const p = fx.info.methodInfo; + if (--counts[p] === 0) { + next.push(p); + } + }); } } + inst.constructor.__orderedComputedDeps = ordered; + } + return ordered; +} + +function edgeCounts(inst) { + const props = inst.constructor._properties; + const depsForComputed = inst[COMPUTE_INFO]; + const counts = {}; + const next = []; + for (let p in props) { + const deps = depsForComputed[p]; + if (deps) { + counts[p] = deps.args.length; + } else { + next.push(p); + } } + return {counts, next}; } const TRANSITIVE_DEPENDENCY = '~transitive~dependency~'; @@ -442,11 +537,11 @@ const TRANSITIVE_DEPENDENCY = '~transitive~dependency~'; * @param {?Object} oldProps Bag of previous values for changed properties * @param {?} info Effect metadata * @param {boolean} hasPaths True with `changedProps` contains one or more paths - * @return {void} + * @return {boolean} True when the property being computed changed * @private */ function runComputedEffect(inst, property, changedProps, oldProps, info, hasPaths) { - if (orderedComputed) { + if (orderedComputed == 1) { // Compute any computed dependencies first; this recurses through the // dependency graph to ensure computed properties are never computed with // dependencies that may become invalidated later in this turn @@ -467,7 +562,7 @@ function runComputedEffect(inst, property, changedProps, oldProps, info, hasPath } let computedProp = info.methodInfo; if (inst.__dataHasAccessor && inst.__dataHasAccessor[computedProp]) { - inst._setPendingProperty(computedProp, result, true); + return inst._setPendingProperty(computedProp, result, true); } else { inst[computedProp] = result; } @@ -2369,11 +2464,9 @@ export const PropertyEffects = dedupingMixin(superClass => { throw new Error("Malformed computed expression '" + expression + "'"); } const info = createMethodEffect(this, sig, TYPES.COMPUTE, runComputedEffect, property, dynamicFn); - if (orderedComputed) { - // Effects are normally stored as map of dependency->effect, but for - // ordered computation, we also need tree of computedProp->dependencies - ensureOwnEffectMap(this, COMPUTE_INFO)[property] = info; - } + // Effects are normally stored as map of dependency->effect, but for + // ordered computation, we also need tree of computedProp->dependencies + ensureOwnEffectMap(this, COMPUTE_INFO)[property] = info; } /** From b065d1450a4795d14633ece90510292229f6bc38 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Thu, 11 Apr 2019 10:45:50 -0700 Subject: [PATCH 027/146] Cleanup, add tests. * remove old implementation * add API docs * rename some API * add dynamicFn to dep count * add test for method as dependency --- lib/mixins/property-effects.js | 226 ++++++++++--------------- test/runner.html | 2 + test/unit/path-effects.html | 3 + test/unit/property-effects-elements.js | 23 ++- test/unit/property-effects.html | 167 +++++++++--------- 5 files changed, 207 insertions(+), 214 deletions(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index fc82d767db..82bb81eb14 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -415,45 +415,59 @@ function runReflectEffect(inst, property, props, oldProps, info) { function runComputedEffects(inst, changedProps, oldProps, hasPaths) { let computeEffects = inst[TYPES.COMPUTE]; if (computeEffects) { - if (orderedComputed == 2) { + if (orderedComputed) { + // Runs computed effects in efficient order by keeping a topologically- + // sorted queue of compute effects to run, and inserting subsequently + // invalidated effects as they are run dedupeId++; - const order = orderedComputedDeps(inst); + const order = getComputedOrder(inst); const queue = []; for (let p in changedProps) { - addEffectsFor(p, computeEffects, queue, order, hasPaths); + enqueueEffectsFor(p, computeEffects, queue, order, hasPaths); } let info; while ((info = queue.shift())) { if (runComputedEffect(inst, '', changedProps, oldProps, info, hasPaths)) { - addEffectsFor(info.methodInfo, computeEffects, queue, order, hasPaths); + enqueueEffectsFor(info.methodInfo, computeEffects, queue, order, hasPaths); } } Object.assign(/** @type {!Object} */ (oldProps), inst.__dataOld); Object.assign(/** @type {!Object} */ (changedProps), inst.__dataPending); inst.__dataPending = null; } else { + // Original Polymer 2.x computed effects order, which continues running + // effects until no further computed properties have been invalidated let inputProps = changedProps; while (runEffects(inst, computeEffects, inputProps, oldProps, hasPaths)) { Object.assign(/** @type {!Object} */ (oldProps), inst.__dataOld); Object.assign(/** @type {!Object} */ (changedProps), inst.__dataPending); inputProps = inst.__dataPending; inst.__dataPending = null; - if (orderedComputed) { - // Ensure all computed properties are de-duped against the same turn - dedupeId--; - } } } } } -const insertEffect = (info, effects, order) => { +/** + * Inserts a computed effect into a queue, given the specified order. Performs + * the insert using a binary search. + * + * Used by `orderedComputed: true` computed property algorithm. + * + * @param {Object} info Property effects metadata + * @param {Array} queue Ordered queue of effects + * @param {Map} order Map of computed property name->topological + * sort order + */ +const insertEffect = (info, queue, order) => { let start = 0; - let end = effects.length - 1; + let end = queue.length - 1; let idx = -1; while (start <= end) { const mid = (start + end) >> 1; - const cmp = order.get(effects[mid].methodInfo) - order.get(info.methodInfo); + // Note `methodInfo` is where the computed property name is stored in + // the effect metadata + const cmp = order.get(queue[mid].methodInfo) - order.get(info.methodInfo); if (cmp < 0) { start = mid + 1; } else if (cmp > 0) { @@ -466,10 +480,24 @@ const insertEffect = (info, effects, order) => { if (idx < 0) { idx = end + 1; } - effects.splice(idx, 0, info); + queue.splice(idx, 0, info); }; -const addEffectsFor = (prop, computeEffects, queue, order, hasPaths) => { +/** + * Inserts all downstream computed effects invalidated by the specified property + * into the topologically-sorted queue of effects to be run. + * + * Used by `orderedComputed: true` computed property algorithm. + * + * @param {string} prop Property name + * @param {Object} computeEffects Computed effects for this element + * @param {Array} queue Topologically-sorted queue of computed effects + * to be run + * @param {Map} order Map of computed property name->topological + * sort order + * @param {boolean} hasPaths True with `changedProps` contains one or more paths + */ +const enqueueEffectsFor = (prop, computeEffects, queue, order, hasPaths) => { const rootProperty = hasPaths ? root(prop) : prop; const fxs = computeEffects[rootProperty]; if (fxs) { @@ -484,21 +512,43 @@ const addEffectsFor = (prop, computeEffects, queue, order, hasPaths) => { } }; -function orderedComputedDeps(inst) { +/** + * Generates and retrieves a memoized map of computed property name to its + * topologically-sorted order. + * + * The map is generated by first assigning a "dependency count" to each property + * (defined as number properties it depends on, including its method for + * "dynamic functions"). Any properties that have no dependencies are added to + * the `ready` queue, which are properties whose order can be added to the final + * order map. Properties are popped off the `ready` one by one and a.) added as + * the next property in the order map, and b.) each property that it is a + * dependency for has its dep count decremented (and if that property's dep + * count goes to zero, it is added to the `ready` queue), until all properties + * have been visited and ordered. + * + * Used by `orderedComputed: true` computed property algorithm. + * + * @param {!Polymer_PropertyEffects} inst The instance to retrieve the computed + * effect order for. + * @return {Map} Map of computed property name->topological sort + * order + */ +function getComputedOrder(inst) { let ordered = inst.constructor.__orderedComputedDeps; if (!ordered) { ordered = new Map(); const effects = inst[TYPES.COMPUTE]; - const {counts, next} = edgeCounts(inst); + const {counts, ready} = dependencyCounts(inst); let curr; - while ((curr = next.pop())) { + while ((curr = ready.shift())) { ordered.set(curr, ordered.size); const computedByCurr = effects[curr]; if (computedByCurr) { computedByCurr.forEach(fx => { - const p = fx.info.methodInfo; - if (--counts[p] === 0) { - next.push(p); + // Note `methodInfo` is where the computed property name is stored + const computedProp = fx.info.methodInfo; + if (--counts[computedProp] === 0) { + ready.push(computedProp); } }); } @@ -508,24 +558,37 @@ function orderedComputedDeps(inst) { return ordered; } -function edgeCounts(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. + * + * Used by `orderedComputed: true` computed property algorithm. + * + * @param {!Polymer_PropertyEffects} inst The instance to generate dependency + * counts for. + * @return {Object} Object containing `counts` map (property-to-dependency + * count) and pre-populated `ready` array of properties that had zero + * dependencies. + */ +function dependencyCounts(inst) { const props = inst.constructor._properties; - const depsForComputed = inst[COMPUTE_INFO]; + const infoForComputed = inst[COMPUTE_INFO]; const counts = {}; - const next = []; + const ready = []; for (let p in props) { - const deps = depsForComputed[p]; - if (deps) { - counts[p] = deps.args.length; + 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 { - next.push(p); + ready.push(p); } } - return {counts, next}; + return {counts, ready}; } -const TRANSITIVE_DEPENDENCY = '~transitive~dependency~'; - /** * Implements the "computed property" effect by running the method with the * values of the arguments specified in the `info` object and setting the @@ -540,120 +603,19 @@ const TRANSITIVE_DEPENDENCY = '~transitive~dependency~'; * @return {boolean} True when the property being computed changed * @private */ -function runComputedEffect(inst, property, changedProps, oldProps, info, hasPaths) { - if (orderedComputed == 1) { - // Compute any computed dependencies first; this recurses through the - // dependency graph to ensure computed properties are never computed with - // dependencies that may become invalidated later in this turn - computeDependencies(inst, info, changedProps, oldProps, hasPaths); - // If the dependency was not transitive, it's definitely dirty and needs to - // be computed; if it is transitive, check its arguments against the current - // changed props and only re-compute if it is dirty - if (property === TRANSITIVE_DEPENDENCY && - !computedPropertyIsDirty(inst, info, changedProps, hasPaths)) { - return; - } - } +function runComputedEffect(inst, property, changedProps, oldProps, info) { // Dirty check dependencies and run if any invalid let result = runMethodEffect(inst, property, changedProps, oldProps, info); // Abort if method returns a no-op result if (result === NOOP) { - return; + return false; } let computedProp = info.methodInfo; if (inst.__dataHasAccessor && inst.__dataHasAccessor[computedProp]) { return inst._setPendingProperty(computedProp, result, true); } else { inst[computedProp] = result; - } -} - -/** - * Returns any dependencies of a computed property that are themselves - * also computed. - * - * @param {!Polymer_PropertyEffects} inst The instance to test - * @param {?} info Computed effect metadata - * @return {Array} Array of computed effect metadata for depenencies - * @private - */ - function computedDependenciesFor(inst, info) { - let deps = info.computedDependencies; - if (!deps) { - deps = info.computedDependencies = []; - info.args.forEach(arg => { - const dep = arg.rootProperty || arg.name; - const depInfo = inst[COMPUTE_INFO][dep]; - if (depInfo) { - deps.push(depInfo); - } - }); - } - return deps; -} - -/** - * Runs computed effects for dependencies of the computed property effect - * described by `info`. - * - * @param {!Polymer_PropertyEffects} inst The instance to test - * @param {Object} info Effect metadata - * @param {Object} changedProps Bag of current property changes - * @param {Object} oldProps Bag of previous values for changed properties - * @param {boolean} hasPaths True with `changedProps` contains one or more paths - * @return {void} - * @private - */ - function computeDependencies(inst, info, changedProps, oldProps, hasPaths) { - let deps = computedDependenciesFor(inst, info); - if (deps.length) { - deps.forEach(depInfo => { - if (depInfo.lastRun !== info.lastRun) { - depInfo.lastRun = info.lastRun; - runComputedEffect(inst, TRANSITIVE_DEPENDENCY, changedProps, oldProps, depInfo, hasPaths); - } - }); - } -} - -/** - * Returns whether the computed effect has any changed dependencies. - * - * @param {!Polymer_PropertyEffects} inst The instance to test - * @param {?} info Effect metadata - * @param {Object} changedProps Bag of current property changes - * @param {boolean} hasPaths True with `changedProps` contains one or more paths - * @return {boolean} true when the computed effect should be re-calculated - * @private - */ -function computedPropertyIsDirty(inst, info, changedProps, hasPaths) { - return info.dynamicFn && propertyIsDirty(inst, {rootProperty: info.methodName}, changedProps, hasPaths) || - info.args.some(arg => propertyIsDirty(inst, arg, changedProps, hasPaths)); -} - -/** - * Returns whether any changed props match the effect trigger - * - * @param {!Polymer_PropertyEffects} inst The instance to test - * @param {DataTrigger} trigger Descriptor - * @param {Object} changedProps Bag of current property changes - * @param {boolean} hasPaths True with `changedProps` contains one or more paths - * @return {boolean} true when the property is dirty - * @private - */ -function propertyIsDirty(inst, trigger, changedProps, hasPaths) { - if (hasPaths) { - [changedProps, inst.__dataPending].some(props => { - for (let p in props) { - if (pathMatchesTrigger(p, trigger)) { - return true; - } - } - }); return false; - } else { - return Boolean(changedProps && trigger.rootProperty in changedProps || - inst.__dataPending && trigger.rootProperty in inst.__dataPending); } } diff --git a/test/runner.html b/test/runner.html index 341b02899d..8596a652aa 100644 --- a/test/runner.html +++ b/test/runner.html @@ -33,8 +33,10 @@ 'unit/property-effects.html', 'unit/property-effects.html?legacyUndefined=true', 'unit/property-effects.html?legacyUndefined=true&legacyNoBatch=true', + 'unit/property-effects.html?orderedComputed=true', 'unit/property-effects-template.html', 'unit/path-effects.html', + 'unit/path-effects.html?orderedComputed=true', 'unit/shady.html', 'unit/shady-events.html', 'unit/shady-content.html', diff --git a/test/unit/path-effects.html b/test/unit/path-effects.html index feebb9c901..973d9a5d3c 100644 --- a/test/unit/path-effects.html +++ b/test/unit/path-effects.html @@ -21,6 +21,9 @@ From 61715f1d21d29eaa6d74f1cf0d4768133b8b4355 Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Thu, 11 Apr 2019 18:36:56 -0700 Subject: [PATCH 028/146] Fix typo in comment Co-Authored-By: kevinpschaaf --- lib/mixins/property-effects.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index 82bb81eb14..bb4b1f9165 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -520,7 +520,7 @@ const enqueueEffectsFor = (prop, computeEffects, queue, order, hasPaths) => { * (defined as number properties it depends on, including its method for * "dynamic functions"). Any properties that have no dependencies are added to * the `ready` queue, which are properties whose order can be added to the final - * order map. Properties are popped off the `ready` one by one and a.) added as + * order map. Properties are popped off the `ready` queue one by one and a.) added as * the next property in the order map, and b.) each property that it is a * dependency for has its dep count decremented (and if that property's dep * count goes to zero, it is added to the `ready` queue), until all properties From 70337ac8feb448d110702420091ded056af88a75 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Fri, 12 Apr 2019 17:19:45 -0700 Subject: [PATCH 029/146] [ci skip] Add comment --- lib/mixins/property-effects.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index bb4b1f9165..9fa0304842 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -96,6 +96,7 @@ function ensureOwnEffectMap(model, type, cloneArrays) { if (cloneArrays) { for (let p in effects) { let protoFx = effects[p]; + // Perf optimization over Array.slice let instFx = effects[p] = Array(protoFx.length); for (let i=0; i Date: Mon, 22 Apr 2019 17:44:26 -0700 Subject: [PATCH 030/146] Fix a few closure compiler issues Fix spelling on some type annotations Remove documentation of unused `hasPath` argument for runComputedEffect Remove one spot calling runComputedEffect with unused `hasPath` argument --- lib/mixins/property-effects.js | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index 9fe896cecc..1770585ac8 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -428,7 +428,7 @@ function runComputedEffects(inst, changedProps, oldProps, hasPaths) { } let info; while ((info = queue.shift())) { - if (runComputedEffect(inst, '', changedProps, oldProps, info, hasPaths)) { + if (runComputedEffect(inst, '', changedProps, oldProps, info)) { enqueueEffectsFor(info.methodInfo, computeEffects, queue, order, hasPaths); } } @@ -452,9 +452,9 @@ function runComputedEffects(inst, changedProps, oldProps, hasPaths) { /** * Inserts a computed effect into a queue, given the specified order. Performs * the insert using a binary search. - * + * * Used by `orderedComputed: true` computed property algorithm. - * + * * @param {Object} info Property effects metadata * @param {Array} queue Ordered queue of effects * @param {Map} order Map of computed property name->topological @@ -466,7 +466,7 @@ const insertEffect = (info, queue, order) => { let idx = -1; while (start <= end) { const mid = (start + end) >> 1; - // Note `methodInfo` is where the computed property name is stored in + // Note `methodInfo` is where the computed property name is stored in // the effect metadata const cmp = order.get(queue[mid].methodInfo) - order.get(info.methodInfo); if (cmp < 0) { @@ -487,14 +487,14 @@ const insertEffect = (info, queue, order) => { /** * Inserts all downstream computed effects invalidated by the specified property * into the topologically-sorted queue of effects to be run. - * + * * Used by `orderedComputed: true` computed property algorithm. - * + * * @param {string} prop Property name * @param {Object} computeEffects Computed effects for this element - * @param {Array} queue Topologically-sorted queue of computed effects + * @param {Array} queue Topologically-sorted queue of computed effects * to be run - * @param {Map} order Map of computed property name->topological + * @param {Map} order Map of computed property name->topological * sort order * @param {boolean} hasPaths True with `changedProps` contains one or more paths */ @@ -515,7 +515,7 @@ const enqueueEffectsFor = (prop, computeEffects, queue, order, hasPaths) => { /** * Generates and retrieves a memoized map of computed property name to its - * topologically-sorted order. + * topologically-sorted order. * * The map is generated by first assigning a "dependency count" to each property * (defined as number properties it depends on, including its method for @@ -531,8 +531,8 @@ const enqueueEffectsFor = (prop, computeEffects, queue, order, hasPaths) => { * * @param {!Polymer_PropertyEffects} inst The instance to retrieve the computed * effect order for. - * @return {Map} Map of computed property name->topological sort - * order + * @return {Map} Map of computed property name->topological sort + * order */ function getComputedOrder(inst) { let ordered = inst.constructor.__orderedComputedDeps; @@ -579,8 +579,8 @@ function dependencyCounts(inst) { const counts = {}; const ready = []; for (let p in props) { - const info = infoForComputed[p]; - if (info) { + 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 { @@ -600,7 +600,6 @@ function dependencyCounts(inst) { * @param {?Object} changedProps Bag of current property changes * @param {?Object} oldProps Bag of previous values for changed properties * @param {?} info Effect metadata - * @param {boolean} hasPaths True with `changedProps` contains one or more paths * @return {boolean} True when the property being computed changed * @private */ @@ -2438,7 +2437,7 @@ export const PropertyEffects = dedupingMixin(superClass => { } const info = createMethodEffect(this, sig, TYPES.COMPUTE, runComputedEffect, property, dynamicFn); // Effects are normally stored as map of dependency->effect, but for - // ordered computation, we also need tree of computedProp->dependencies + // ordered computation, we also need tree of computedProp->dependencies ensureOwnEffectMap(this, COMPUTE_INFO)[property] = info; } From 27ed93aff1ae8cc05d99d25975970c19721a4446 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Thu, 6 Jun 2019 12:59:48 -0700 Subject: [PATCH 031/146] dom-if/dom-repeat bind-to-parent --- lib/elements/dom-if.js | 2 +- lib/elements/dom-repeat.js | 2 +- lib/mixins/property-effects.js | 12 +++++++++++- lib/utils/settings.js | 2 +- lib/utils/templatize.js | 29 +++++++++++++++++++---------- 5 files changed, 33 insertions(+), 14 deletions(-) diff --git a/lib/elements/dom-if.js b/lib/elements/dom-if.js index 2b4f23c70d..70289708dd 100644 --- a/lib/elements/dom-if.js +++ b/lib/elements/dom-if.js @@ -182,7 +182,7 @@ export class DomIf extends PolymerElement { // Guard against element being detached while render was queued if (parentNode) { if (!this.__ctor) { - let template = /** @type {HTMLTemplateElement} */(wrap(this).querySelector('template')); + let template = this._templateInfo ? this : /** @type {HTMLTemplateElement} */(wrap(this).querySelector('template')); if (!template) { // Wait until childList changes and template should be there by then let observer = new MutationObserver(() => { diff --git a/lib/elements/dom-repeat.js b/lib/elements/dom-repeat.js index 7b0407d2e0..e330dce954 100644 --- a/lib/elements/dom-repeat.js +++ b/lib/elements/dom-repeat.js @@ -339,7 +339,7 @@ export class DomRepeat extends domRepeatBase { // until ready, since won't have its template content handed back to // it until then if (!this.__ctor) { - let template = this.template = /** @type {HTMLTemplateElement} */(this.querySelector('template')); + let template = this.template = this._templateInfo ? this : /** @type {HTMLTemplateElement} */(this.querySelector('template')); if (!template) { // // Wait until childList changes and template should be there by then let observer = new MutationObserver(() => { diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index 1770585ac8..802586e937 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -21,6 +21,8 @@ import { PropertyAccessors } from './property-accessors.js'; import { TemplateStamp } from './template-stamp.js'; import { sanitizeDOMValue, legacyUndefined, legacyNoBatch, legacyNotifyOrder, orderedComputed } from '../utils/settings.js'; +const btp = window.location.search.match(/btp/); + // Monotonically increasing unique ID used for de-duping effects triggered // from multiple properties in the same turn let dedupeId = 0; @@ -2919,8 +2921,16 @@ export const PropertyEffects = dedupingMixin(superClass => { // Change back to just super.methodCall() let noted = propertyEffectsBase._parseTemplateNestedTemplate.call( this, node, templateInfo, nodeInfo); + const parent = node.parentNode; + const nestedTemplateInfo = nodeInfo.templateInfo; + if (btp && parent.localName.match(/(dom-repeat|dom-if)/)) { + parent.removeChild(node); + nodeInfo.parentInfo.templateInfo = nestedTemplateInfo; + nodeInfo = nodeInfo.parentInfo; + noted = false; + } // Merge host props into outer template and add bindings - let hostProps = nodeInfo.templateInfo.hostProps; + let hostProps = nestedTemplateInfo.hostProps; let mode = '{'; for (let source in hostProps) { let parts = [{ mode, source, dependencies: [source], hostProp: true }]; diff --git a/lib/utils/settings.js b/lib/utils/settings.js index 84812d24f6..bc81c7bdf4 100644 --- a/lib/utils/settings.js +++ b/lib/utils/settings.js @@ -127,7 +127,7 @@ export const setAllowTemplateFromDomModule = function(allowDomModule) { * If no includes or relative urls are used in styles, these steps can be * skipped as an optimization. */ -export let legacyOptimizations = false; +export let legacyOptimizations = window.Polymer && window.Polymer.legacyOptimizations || false; /** * Sets `legacyOptimizations` globally for all elements to enable optimizations diff --git a/lib/utils/templatize.js b/lib/utils/templatize.js index c29d96e012..e4f5ba4211 100644 --- a/lib/utils/templatize.js +++ b/lib/utils/templatize.js @@ -368,6 +368,7 @@ function createTemplatizerClass(template, templateInfo, options) { function addPropagateEffects(template, templateInfo, options, methodHost) { let userForwardHostProp = options.forwardHostProp; if (userForwardHostProp && templateInfo.hasHostProps) { + const isTemplate = template.localName == 'template'; // Provide data API and property effects on memoized template class let klass = templateInfo.templatizeTemplateClass; if (!klass) { @@ -375,10 +376,14 @@ function addPropagateEffects(template, templateInfo, options, methodHost) { * @constructor * @extends {DataTemplate} */ - let templatizedBase = options.mutableData ? MutableDataTemplate : DataTemplate; - /** @private */ - klass = templateInfo.templatizeTemplateClass = - class TemplatizedTemplate extends templatizedBase {}; + if (isTemplate) { + let templatizedBase = options.mutableData ? MutableDataTemplate : DataTemplate; + /** @private */ + klass = templateInfo.templatizeTemplateClass = + class TemplatizedTemplate extends templatizedBase {}; + } else { + klass = class TemplatizedTemplateExtension extends template.constructor {}; + } // Add template - >instances effects // and host <- template effects let hostProps = templateInfo.hostProps; @@ -392,7 +397,16 @@ function addPropagateEffects(template, templateInfo, options, methodHost) { warnOnUndeclaredProperties(templateInfo, options, methodHost); } } - upgradeTemplate(template, klass); + if (isTemplate) { + upgradeTemplate(template, klass); + // Clear any pending data for performance + template.__dataTemp = {}; + template.__dataPending = null; + template.__dataOld = null; + template._enableProperties(); + } else { + Object.setPrototypeOf(template, klass.prototype); + } // Mix any pre-bound data into __data; no need to flush this to // instances since they pull from the template at instance-time if (template.__dataProto) { @@ -400,11 +414,6 @@ function addPropagateEffects(template, templateInfo, options, methodHost) { // to not be since this is a vanilla template we just added effects to Object.assign(template.__data, template.__dataProto); } - // Clear any pending data for performance - template.__dataTemp = {}; - template.__dataPending = null; - template.__dataOld = null; - template._enableProperties(); } } /* eslint-enable valid-jsdoc */ From e690dfe2c176ceb050aecfe62f550c9d8150d76b Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Tue, 18 Jun 2019 22:10:50 -0700 Subject: [PATCH 032/146] Runtime stamped dom-if --- lib/elements/dom-if.js | 138 ++++++++++++++++++++----------- lib/elements/dom-repeat.js | 5 +- lib/mixins/property-effects.js | 98 ++++++++++++++-------- lib/mixins/template-stamp.js | 14 ++-- lib/utils/settings.js | 40 +++++++++ lib/utils/templatize.js | 122 +++++++++++++++------------ test/runner.html | 4 + test/unit/dom-if.html | 5 ++ test/unit/dom-repeat-elements.js | 4 +- test/unit/dom-repeat.html | 5 +- 10 files changed, 295 insertions(+), 140 deletions(-) diff --git a/lib/elements/dom-if.js b/lib/elements/dom-if.js index 70289708dd..8c09d91293 100644 --- a/lib/elements/dom-if.js +++ b/lib/elements/dom-if.js @@ -16,6 +16,8 @@ import { microTask } from '../utils/async.js'; import { root } from '../utils/path.js'; import { wrap } from '../utils/wrap.js'; import { hideElementsGlobally } from '../utils/hide-template-controls.js'; +import { fastDomIf, strictTemplatePolicy } from '../utils/settings.js'; +import { showHideChildren } from '../utils/templatize.js'; /** * The `` element will stamp a light-dom `