From 63dadbf248afb96ccffc3acfc6d8f47214e7bde1 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 30 Jan 2019 10:06:12 -0800 Subject: [PATCH 01/10] Add warning for undeclared properties used in bindings. --- lib/mixins/property-effects.js | 5 ++++- test/unit/property-effects.html | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index 4961eec486..cbe721a3c3 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -18,7 +18,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 } from '../utils/settings.js'; +import { sanitizeDOMValue, legacyOptimizations } from '../utils/settings.js'; // Monotonically increasing unique ID used for de-duping effects triggered // from multiple properties in the same turn @@ -2390,6 +2390,9 @@ export const PropertyEffects = dedupingMixin(superClass => { * @protected */ static _addTemplatePropertyEffect(templateInfo, prop, effect) { + if (legacyOptimizations && !(prop in templateInfo.dynamicFns)) { + console.warn(`Property '${prop}' used in template but not declared in 'properties'.`); + } let hostProps = templateInfo.hostProps = templateInfo.hostProps || {}; hostProps[prop] = true; let effects = templateInfo.propertyEffects = templateInfo.propertyEffects || {}; diff --git a/test/unit/property-effects.html b/test/unit/property-effects.html index fbafbed8a9..0b8226bb8b 100644 --- a/test/unit/property-effects.html +++ b/test/unit/property-effects.html @@ -25,8 +25,8 @@ From 007f3cc251c504e7d68edf60e8318890444a56de Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 30 Jan 2019 10:06:28 -0800 Subject: [PATCH 02/10] Add warning for redeclared computed properties. --- lib/mixins/element-mixin.js | 8 ++++++-- test/unit/property-effects.html | 21 ++++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/lib/mixins/element-mixin.js b/lib/mixins/element-mixin.js index 84c590686a..e37f066609 100644 --- a/lib/mixins/element-mixin.js +++ b/lib/mixins/element-mixin.js @@ -217,8 +217,12 @@ export const ElementMixin = dedupingMixin(base => { // setup where multiple triggers for setting a property) // While we do have `hasComputedEffect` this is set on the property's // dependencies rather than itself. - if (info.computed && !proto._hasReadOnlyEffect(name)) { - proto._createComputedProperty(name, info.computed, allProps); + if (info.computed) { + if (proto._hasReadOnlyEffect(name)) { + console.warn(`Cannot redefine computed property '${name}'.`) + } else { + proto._createComputedProperty(name, info.computed, allProps); + } } if (info.readOnly && !proto._hasReadOnlyEffect(name)) { proto._createReadOnlyProperty(name, !info.computed); diff --git a/test/unit/property-effects.html b/test/unit/property-effects.html index 0b8226bb8b..e400b85b3c 100644 --- a/test/unit/property-effects.html +++ b/test/unit/property-effects.html @@ -1877,7 +1877,6 @@ suite('warn on legacy differences', () => { setup(function() { - setLegacyOptimizations(true); sinon.spy(console, 'warn'); }); @@ -1887,6 +1886,7 @@ }); test('warn if non-declared property used in binding', () => { + setLegacyOptimizations(true); Polymer({ is: 'x-warn-undeclared-binding', _template: html` @@ -1900,6 +1900,25 @@ assert.equal(console.warn.callCount, 3); }); + test('warn when re-declaring a computed property', () => { + Polymer({ + is: 'x-warn-redeclared-computed', + behaviors: [{ + properties: { + a: { computed: 'compute(x)' } + } + }, { + properties: { + a: { computed: 'compute(y)' } + } + }] + }); + const el = document.createElement('x-warn-redeclared-computed'); + document.body.appendChild(el); + document.body.removeChild(el); + assert.equal(console.warn.callCount, 1); + }); + }); From 563bc8586cec105e690172556e27a5eb9fb05a5e Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 30 Jan 2019 10:33:34 -0800 Subject: [PATCH 03/10] Fix lint warning --- lib/mixins/element-mixin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mixins/element-mixin.js b/lib/mixins/element-mixin.js index e37f066609..f5e695c813 100644 --- a/lib/mixins/element-mixin.js +++ b/lib/mixins/element-mixin.js @@ -219,7 +219,7 @@ export const ElementMixin = dedupingMixin(base => { // dependencies rather than itself. if (info.computed) { if (proto._hasReadOnlyEffect(name)) { - console.warn(`Cannot redefine computed property '${name}'.`) + console.warn(`Cannot redefine computed property '${name}'.`); } else { proto._createComputedProperty(name, info.computed, allProps); } From 6bd15ccb8ee9d9040b316e129aa768fe47d28999 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Thu, 31 Jan 2019 18:42:14 -0800 Subject: [PATCH 04/10] Upgrade webcomponentsjs --- package-lock.json | 92 ++++++++++++++++++++--------------------------- package.json | 2 +- 2 files changed, 40 insertions(+), 54 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9da912d1cb..b468dee87f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1310,9 +1310,9 @@ "integrity": "sha512-bx0TzeZ11VqYDGLuXfznen8+4u0hADk2dD5RNMFxWL9MM4c5NXCDCgNXgssb4i2zQOos/GGe4tl5AuE0LzJkLA==" }, "@webcomponents/webcomponentsjs": { - "version": "2.2.2", - "resolved": "https://registry.npmjs.org/@webcomponents/webcomponentsjs/-/webcomponentsjs-2.2.2.tgz", - "integrity": "sha512-t45WI6m/dD+gmMaQ/u0k1gCrSg5yESnfGaxfFPV1GST2RL/aEScoaotFsfjfP8DPcq2OH6leyHoWBT+pSuGsdQ==", + "version": "2.2.5", + "resolved": "https://registry.npmjs.org/@webcomponents/webcomponentsjs/-/webcomponentsjs-2.2.5.tgz", + "integrity": "sha512-aLSQa2+NjGv+6hwjQ/mHhrav7qhRf1ZMsUNWNqP3ca9BK9YoDmjsrcFFr8TaZ+ucwm+JiNVEtoQinNpbn863pw==", "dev": true }, "abbrev": { @@ -9756,7 +9756,7 @@ }, "array-flatten": { "version": "1.1.1", - "resolved": "http://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz", + "resolved": "https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz", "integrity": "sha1-ml9pkFGx5wczKPKgCJaLZOopVdI=", "dev": true }, @@ -10635,7 +10635,7 @@ }, "string_decoder": { "version": "0.10.31", - "resolved": "http://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", "integrity": "sha1-YuIDvEF2bGwoyfyEMB2rHFMQ+pQ=", "dev": true } @@ -11119,7 +11119,7 @@ }, "concat-stream": { "version": "1.6.2", - "resolved": "http://registry.npmjs.org/concat-stream/-/concat-stream-1.6.2.tgz", + "resolved": "https://registry.npmjs.org/concat-stream/-/concat-stream-1.6.2.tgz", "integrity": "sha512-27HBghJxjiZtIk3Ycvn/4kbJk/1uZuJFfuPEns6LaEvpvG1f0hTea8lilrouyo9mVc2GWdcEZ8OLoGmSADlrCw==", "dev": true, "requires": { @@ -11145,7 +11145,7 @@ }, "content-disposition": { "version": "0.5.2", - "resolved": "http://registry.npmjs.org/content-disposition/-/content-disposition-0.5.2.tgz", + "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-0.5.2.tgz", "integrity": "sha1-DPaLud318r55YcOoUXjLhdunjLQ=", "dev": true }, @@ -11529,7 +11529,7 @@ }, "string_decoder": { "version": "0.10.31", - "resolved": "http://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", "integrity": "sha1-YuIDvEF2bGwoyfyEMB2rHFMQ+pQ=", "dev": true } @@ -11895,7 +11895,7 @@ }, "expand-range": { "version": "1.8.2", - "resolved": "http://registry.npmjs.org/expand-range/-/expand-range-1.8.2.tgz", + "resolved": "https://registry.npmjs.org/expand-range/-/expand-range-1.8.2.tgz", "integrity": "sha1-opnv/TNf4nIeuujiV+x5ZE/IUzc=", "dev": true, "requires": { @@ -12144,7 +12144,7 @@ }, "is-accessor-descriptor": { "version": "0.1.6", - "resolved": "http://registry.npmjs.org/is-accessor-descriptor/-/is-accessor-descriptor-0.1.6.tgz", + "resolved": "https://registry.npmjs.org/is-accessor-descriptor/-/is-accessor-descriptor-0.1.6.tgz", "integrity": "sha1-qeEss66Nh2cn7u84Q/igiXtcmNY=", "dev": true, "requires": { @@ -12164,7 +12164,7 @@ }, "is-data-descriptor": { "version": "0.1.4", - "resolved": "http://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz", + "resolved": "https://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz", "integrity": "sha1-C17mSDiOLIYCgueT8YVv7D8wG1Y=", "dev": true, "requires": { @@ -12683,15 +12683,13 @@ "version": "1.0.0", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.0.tgz", "integrity": "sha1-ibTRmasr7kneFk6gK4nORi1xt2c=", - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -12708,22 +12706,19 @@ "version": "1.1.0", "resolved": "https://registry.npmjs.org/code-point-at/-/code-point-at-1.1.0.tgz", "integrity": "sha1-DQcLTQQ6W+ozovGkDi7bPZpMz3c=", - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "resolved": "https://registry.npmjs.org/concat-map/-/concat-map-0.0.1.tgz", "integrity": "sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=", - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/console-control-strings/-/console-control-strings-1.1.0.tgz", "integrity": "sha1-PXz0Rk22RG6mRL9LOVB/mFEAjo4=", - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -12854,8 +12849,7 @@ "version": "2.0.3", "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.3.tgz", "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=", - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -12869,7 +12863,6 @@ "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-1.0.0.tgz", "integrity": "sha1-754xOG8DGn8NZDr4L95QxFfvAMs=", "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -12886,7 +12879,6 @@ "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.4.tgz", "integrity": "sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==", "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -12895,15 +12887,13 @@ "version": "0.0.8", "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=", - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.2.4", "resolved": "https://registry.npmjs.org/minipass/-/minipass-2.2.4.tgz", "integrity": "sha512-hzXIWWet/BzWhYs2b+u7dRHlruXhwdgvlTMDKC6Cb1U7ps6Ac6yQlR39xsbjWJE377YTCtKwIXIpJ5oP+j5y8g==", "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -12924,7 +12914,6 @@ "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -13013,8 +13002,7 @@ "version": "1.0.1", "resolved": "https://registry.npmjs.org/number-is-nan/-/number-is-nan-1.0.1.tgz", "integrity": "sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=", - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -13028,7 +13016,6 @@ "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -13166,7 +13153,6 @@ "resolved": "https://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", "integrity": "sha1-EYvfW4zcUaKn5w0hHgfisLmxB9M=", "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -13423,7 +13409,7 @@ }, "string_decoder": { "version": "0.10.31", - "resolved": "http://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", "integrity": "sha1-YuIDvEF2bGwoyfyEMB2rHFMQ+pQ=", "dev": true }, @@ -13456,7 +13442,7 @@ }, "global-modules": { "version": "0.2.3", - "resolved": "http://registry.npmjs.org/global-modules/-/global-modules-0.2.3.tgz", + "resolved": "https://registry.npmjs.org/global-modules/-/global-modules-0.2.3.tgz", "integrity": "sha1-6lo77ULG1s6ZWk+KEmm12uIjgo0=", "dev": true, "requires": { @@ -13474,7 +13460,7 @@ }, "global-prefix": { "version": "0.1.5", - "resolved": "http://registry.npmjs.org/global-prefix/-/global-prefix-0.1.5.tgz", + "resolved": "https://registry.npmjs.org/global-prefix/-/global-prefix-0.1.5.tgz", "integrity": "sha1-jTvGuNo8qBEqFg2NSW/wRiv+948=", "dev": true, "requires": { @@ -14009,7 +13995,7 @@ }, "is-accessor-descriptor": { "version": "0.1.6", - "resolved": "http://registry.npmjs.org/is-accessor-descriptor/-/is-accessor-descriptor-0.1.6.tgz", + "resolved": "https://registry.npmjs.org/is-accessor-descriptor/-/is-accessor-descriptor-0.1.6.tgz", "integrity": "sha1-qeEss66Nh2cn7u84Q/igiXtcmNY=", "dev": true, "requires": { @@ -14057,7 +14043,7 @@ }, "is-data-descriptor": { "version": "0.1.4", - "resolved": "http://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz", + "resolved": "https://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz", "integrity": "sha1-C17mSDiOLIYCgueT8YVv7D8wG1Y=", "dev": true, "requires": { @@ -15046,7 +15032,7 @@ }, "multimatch": { "version": "2.1.0", - "resolved": "http://registry.npmjs.org/multimatch/-/multimatch-2.1.0.tgz", + "resolved": "https://registry.npmjs.org/multimatch/-/multimatch-2.1.0.tgz", "integrity": "sha1-nHkGoi+0wCkZ4vX3UWG0zb1LKis=", "dev": true, "requires": { @@ -15058,7 +15044,7 @@ }, "multipipe": { "version": "1.0.2", - "resolved": "http://registry.npmjs.org/multipipe/-/multipipe-1.0.2.tgz", + "resolved": "https://registry.npmjs.org/multipipe/-/multipipe-1.0.2.tgz", "integrity": "sha1-zBPv2DPJzamfIk+GhGG44aP9k50=", "dev": true, "requires": { @@ -15495,7 +15481,7 @@ }, "pako": { "version": "0.2.9", - "resolved": "http://registry.npmjs.org/pako/-/pako-0.2.9.tgz", + "resolved": "https://registry.npmjs.org/pako/-/pako-0.2.9.tgz", "integrity": "sha1-8/dSL073gjSNqBYbrZ7P1Rv4OnU=", "dev": true }, @@ -16079,7 +16065,7 @@ }, "pretty-bytes": { "version": "4.0.2", - "resolved": "http://registry.npmjs.org/pretty-bytes/-/pretty-bytes-4.0.2.tgz", + "resolved": "https://registry.npmjs.org/pretty-bytes/-/pretty-bytes-4.0.2.tgz", "integrity": "sha1-sr+C5zUNZcbDOqlaqlpPYyf2HNk=", "dev": true }, @@ -16378,7 +16364,7 @@ }, "is-accessor-descriptor": { "version": "0.1.6", - "resolved": "http://registry.npmjs.org/is-accessor-descriptor/-/is-accessor-descriptor-0.1.6.tgz", + "resolved": "https://registry.npmjs.org/is-accessor-descriptor/-/is-accessor-descriptor-0.1.6.tgz", "integrity": "sha1-qeEss66Nh2cn7u84Q/igiXtcmNY=", "dev": true, "requires": { @@ -16398,7 +16384,7 @@ }, "is-data-descriptor": { "version": "0.1.4", - "resolved": "http://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz", + "resolved": "https://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz", "integrity": "sha1-C17mSDiOLIYCgueT8YVv7D8wG1Y=", "dev": true, "requires": { @@ -16792,7 +16778,7 @@ }, "resolve-dir": { "version": "0.1.1", - "resolved": "http://registry.npmjs.org/resolve-dir/-/resolve-dir-0.1.1.tgz", + "resolved": "https://registry.npmjs.org/resolve-dir/-/resolve-dir-0.1.1.tgz", "integrity": "sha1-shklmlYC+sXFxJatiUpujMQwJh4=", "dev": true, "requires": { @@ -17555,7 +17541,7 @@ }, "string-width": { "version": "1.0.2", - "resolved": "http://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", + "resolved": "https://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", "integrity": "sha1-EYvfW4zcUaKn5w0hHgfisLmxB9M=", "dev": true, "requires": { @@ -17566,7 +17552,7 @@ }, "string_decoder": { "version": "1.1.1", - "resolved": "http://registry.npmjs.org/string_decoder/-/string_decoder-1.1.1.tgz", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.1.1.tgz", "integrity": "sha512-n/ShnvDi6FHbbVfviro+WojiFzv+s8MPMHBczVePfUpDJLwoLT0ht1l4YwBCbi8pJAveEEdnkHyPyTP/mzRfwg==", "dev": true, "requires": { @@ -17624,7 +17610,7 @@ }, "supports-color": { "version": "2.0.0", - "resolved": "http://registry.npmjs.org/supports-color/-/supports-color-2.0.0.tgz", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-2.0.0.tgz", "integrity": "sha1-U10EXOa2Nj+kARcIRimZXp3zJMc=", "dev": true }, @@ -17767,7 +17753,7 @@ }, "temp": { "version": "0.8.3", - "resolved": "http://registry.npmjs.org/temp/-/temp-0.8.3.tgz", + "resolved": "https://registry.npmjs.org/temp/-/temp-0.8.3.tgz", "integrity": "sha1-4Ma8TSa5AxJEEOT+2BEDAU38H1k=", "dev": true, "requires": { @@ -18116,7 +18102,7 @@ }, "underscore": { "version": "1.6.0", - "resolved": "http://registry.npmjs.org/underscore/-/underscore-1.6.0.tgz", + "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.6.0.tgz", "integrity": "sha1-izixDKze9jM3uLJOT/htRa6lKag=", "dev": true }, @@ -18302,7 +18288,7 @@ "dependencies": { "ansi-align": { "version": "1.1.0", - "resolved": "http://registry.npmjs.org/ansi-align/-/ansi-align-1.1.0.tgz", + "resolved": "https://registry.npmjs.org/ansi-align/-/ansi-align-1.1.0.tgz", "integrity": "sha1-LwwWWIKXOa3V67FeawxuNCPwFro=", "dev": true, "requires": { @@ -18930,7 +18916,7 @@ }, "is-accessor-descriptor": { "version": "0.1.6", - "resolved": "http://registry.npmjs.org/is-accessor-descriptor/-/is-accessor-descriptor-0.1.6.tgz", + "resolved": "https://registry.npmjs.org/is-accessor-descriptor/-/is-accessor-descriptor-0.1.6.tgz", "integrity": "sha1-qeEss66Nh2cn7u84Q/igiXtcmNY=", "dev": true, "requires": { @@ -18950,7 +18936,7 @@ }, "is-data-descriptor": { "version": "0.1.4", - "resolved": "http://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz", + "resolved": "https://registry.npmjs.org/is-data-descriptor/-/is-data-descriptor-0.1.4.tgz", "integrity": "sha1-C17mSDiOLIYCgueT8YVv7D8wG1Y=", "dev": true, "requires": { @@ -21021,7 +21007,7 @@ }, "stream-combiner": { "version": "0.2.2", - "resolved": "https://registry.npmjs.org/stream-combiner/-/stream-combiner-0.2.2.tgz", + "resolved": "http://registry.npmjs.org/stream-combiner/-/stream-combiner-0.2.2.tgz", "integrity": "sha1-rsjLrBd7Vrb0+kec7YwZEs7lKFg=", "dev": true, "requires": { diff --git a/package.json b/package.json index 26c688e629..860067b899 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "@polymer/gen-typescript-declarations": "^1.5.1", "@polymer/iron-component-page": "^3.0.0-pre.12", "@polymer/test-fixture": "^3.0.0-pre.12", - "@webcomponents/webcomponentsjs": "^2.2.2", + "@webcomponents/webcomponentsjs": "^2.2.5", "babel-eslint": "^7.2.3", "babel-preset-minify": "^0.2.0", "del": "^3.0.0", From 35c48d895219912f36031342d981bc9faa21074b Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Fri, 1 Feb 2019 10:12:35 -0800 Subject: [PATCH 05/10] Add warnings for disabling boolean settings. --- lib/mixins/element-mixin.js | 6 ++++ test/unit/property-effects.html | 57 +++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/lib/mixins/element-mixin.js b/lib/mixins/element-mixin.js index f5e695c813..702167d66d 100644 --- a/lib/mixins/element-mixin.js +++ b/lib/mixins/element-mixin.js @@ -226,12 +226,18 @@ export const ElementMixin = dedupingMixin(base => { } if (info.readOnly && !proto._hasReadOnlyEffect(name)) { proto._createReadOnlyProperty(name, !info.computed); + } else if (info.readOnly === false && proto._hasReadOnlyEffect(name)) { + console.warn(`Cannot make readOnly property '${name}' non-readOnly.`); } if (info.reflectToAttribute && !proto._hasReflectEffect(name)) { proto._createReflectedProperty(name); + } else if (info.reflectToAttribute === false && proto._hasReflectEffect(name)) { + console.warn(`Cannot make reflected property '${name}' non-reflected.`); } if (info.notify && !proto._hasNotifyEffect(name)) { proto._createNotifyingProperty(name); + } else if (info.notify === false && proto._hasNotifyEffect(name)) { + console.warn(`Cannot make notify property '${name}' non-notify.`); } // always add observer if (info.observer) { diff --git a/test/unit/property-effects.html b/test/unit/property-effects.html index e400b85b3c..087023914f 100644 --- a/test/unit/property-effects.html +++ b/test/unit/property-effects.html @@ -1919,6 +1919,63 @@ assert.equal(console.warn.callCount, 1); }); + test('warn when disabling readOnly on a readOnly property', () => { + Polymer({ + is: 'x-warn-disable-readonly', + behaviors: [{ + properties: { + a: { readOnly: true } + } + }, { + properties: { + a: { readOnly: false } + } + }] + }); + const el = document.createElement('x-warn-disable-readonly'); + document.body.appendChild(el); + document.body.removeChild(el); + assert.equal(console.warn.callCount, 1); + }); + + test('warn when disabling reflect on a reflect property', () => { + Polymer({ + is: 'x-warn-disable-reflect', + behaviors: [{ + properties: { + a: { reflectToAttribute: true } + } + }, { + properties: { + a: { reflectToAttribute: false } + } + }] + }); + const el = document.createElement('x-warn-disable-reflect'); + document.body.appendChild(el); + document.body.removeChild(el); + assert.equal(console.warn.callCount, 1); + }); + + test('warn when disabling notify on a notify property', () => { + Polymer({ + is: 'x-warn-disable-notify', + behaviors: [{ + properties: { + a: { notify: true } + } + }, { + properties: { + a: { notify: false } + } + }] + }); + const el = document.createElement('x-warn-disable-notify'); + document.body.appendChild(el); + document.body.removeChild(el); + assert.equal(console.warn.callCount, 1); + }); + }); From 9dea1f780f139d0a49e1c98ef2249322bbc5c701 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Fri, 1 Feb 2019 10:12:47 -0800 Subject: [PATCH 06/10] Clarify warning. Add comment. --- lib/mixins/property-effects.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index cbe721a3c3..c6ccb2916a 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -2390,8 +2390,14 @@ export const PropertyEffects = dedupingMixin(superClass => { * @protected */ static _addTemplatePropertyEffect(templateInfo, prop, effect) { + // `dynamicFns` is the flattened property list, so we can use that to + // detect non-declared properties. Properties must be listed in + // `properties` to be included in `observedAttributes` since CE V1 + // reads that at registration time, and we want to keep template parsing + // lazy if (legacyOptimizations && !(prop in templateInfo.dynamicFns)) { - console.warn(`Property '${prop}' used in template but not declared in 'properties'.`); + console.warn(`Property '${prop}' used in template but not declared in 'properties'; ` + + `attribute will not be observed.`); } let hostProps = templateInfo.hostProps = templateInfo.hostProps || {}; hostProps[prop] = true; From 28f2281b7a62409a2719ac33b14f933b4e317810 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Fri, 1 Feb 2019 10:13:26 -0800 Subject: [PATCH 07/10] Remove addressed TODO comment. --- 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 702167d66d..2e60ffb76e 100644 --- a/lib/mixins/element-mixin.js +++ b/lib/mixins/element-mixin.js @@ -195,7 +195,6 @@ export const ElementMixin = dedupingMixin(base => { * disables the effect, the setter would fail unexpectedly. * Based on feedback, we may want to try to make effects more malleable * and/or provide an advanced api for manipulating them. - * Also consider adding warnings when an effect cannot be changed. * * @param {!PolymerElement} proto Element class prototype to add accessors * and effects to From c309fef60bec918666bf92a807e7496c8db3ea84 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Mon, 4 Feb 2019 14:14:05 -0800 Subject: [PATCH 08/10] Upgrade wcjs --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index b468dee87f..d3fd24d2f2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1310,9 +1310,9 @@ "integrity": "sha512-bx0TzeZ11VqYDGLuXfznen8+4u0hADk2dD5RNMFxWL9MM4c5NXCDCgNXgssb4i2zQOos/GGe4tl5AuE0LzJkLA==" }, "@webcomponents/webcomponentsjs": { - "version": "2.2.5", - "resolved": "https://registry.npmjs.org/@webcomponents/webcomponentsjs/-/webcomponentsjs-2.2.5.tgz", - "integrity": "sha512-aLSQa2+NjGv+6hwjQ/mHhrav7qhRf1ZMsUNWNqP3ca9BK9YoDmjsrcFFr8TaZ+ucwm+JiNVEtoQinNpbn863pw==", + "version": "2.2.6", + "resolved": "https://registry.npmjs.org/@webcomponents/webcomponentsjs/-/webcomponentsjs-2.2.6.tgz", + "integrity": "sha512-TbuyV7hHIq4jMlvQ8pF3C6QNEqFeBX4be9AND5DO8UCRRYcZBpjl1yH9IChbebeWr1QNcK5WlZC3voAlQCdisQ==", "dev": true }, "abbrev": { diff --git a/package.json b/package.json index 860067b899..3fcebe360a 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "@polymer/gen-typescript-declarations": "^1.5.1", "@polymer/iron-component-page": "^3.0.0-pre.12", "@polymer/test-fixture": "^3.0.0-pre.12", - "@webcomponents/webcomponentsjs": "^2.2.5", + "@webcomponents/webcomponentsjs": "^2.2.6", "babel-eslint": "^7.2.3", "babel-preset-minify": "^0.2.0", "del": "^3.0.0", From 11cd9cb24133835daee443127f8b84e42e16aa1b Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Mon, 4 Feb 2019 15:11:58 -0800 Subject: [PATCH 09/10] Move undeclared property warning to element-mixin. --- lib/mixins/element-mixin.js | 28 +++++++++++++++++++++++++++- lib/mixins/property-effects.js | 9 --------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/lib/mixins/element-mixin.js b/lib/mixins/element-mixin.js index 2e60ffb76e..6c0f0692da 100644 --- a/lib/mixins/element-mixin.js +++ b/lib/mixins/element-mixin.js @@ -727,7 +727,7 @@ export const ElementMixin = dedupingMixin(base => { } /** - * Overrides `PropertyAccessors` to add map of dynamic functions on + * Overrides `PropertyEffects` to add map of dynamic functions on * template info, for consumption by `PropertyEffects` template binding * code. This map determines which method templates should have accessors * created for them. @@ -743,6 +743,32 @@ export const ElementMixin = dedupingMixin(base => { return super._parseTemplateContent(template, templateInfo, nodeInfo); } + /** + * Overrides `PropertyEffects` to warn on use of undeclared properties in + * template. + * + * @param {Object} templateInfo Template metadata to add effect to + * @param {string} prop Property that should trigger the effect + * @param {Object=} effect Effect metadata object + * @return {void} + * @protected + */ + static _addTemplatePropertyEffect(templateInfo, prop, effect) { + // Warn if properties are used in template without being declared. + // Properties must be listed in `properties` to be included in + // `observedAttributes` since CE V1 reads that at registration time, and + // since we want to keep template parsing lazy, we can't automatically + // add undeclared properties used in templates to `observedAttributes`. + // 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)) { + console.warn(`Property '${prop}' used in template but not declared in 'properties'; ` + + `attribute will not be observed.`); + } + return super._addTemplatePropertyEffect(templateInfo, prop, effect); + } + } return PolymerElement; diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js index c6ccb2916a..9669571996 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -2390,15 +2390,6 @@ export const PropertyEffects = dedupingMixin(superClass => { * @protected */ static _addTemplatePropertyEffect(templateInfo, prop, effect) { - // `dynamicFns` is the flattened property list, so we can use that to - // detect non-declared properties. Properties must be listed in - // `properties` to be included in `observedAttributes` since CE V1 - // reads that at registration time, and we want to keep template parsing - // lazy - if (legacyOptimizations && !(prop in templateInfo.dynamicFns)) { - console.warn(`Property '${prop}' used in template but not declared in 'properties'; ` + - `attribute will not be observed.`); - } let hostProps = templateInfo.hostProps = templateInfo.hostProps || {}; hostProps[prop] = true; let effects = templateInfo.propertyEffects = templateInfo.propertyEffects || {}; From 21e83e9e948ae8d4e8c9ae5bb9727ef7513b9d83 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Mon, 4 Feb 2019 15:23:27 -0800 Subject: [PATCH 10/10] Remove unused import --- 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 9669571996..4961eec486 100644 --- a/lib/mixins/property-effects.js +++ b/lib/mixins/property-effects.js @@ -18,7 +18,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, legacyOptimizations } from '../utils/settings.js'; +import { sanitizeDOMValue } from '../utils/settings.js'; // Monotonically increasing unique ID used for de-duping effects triggered // from multiple properties in the same turn