Skip to content

Commit

Permalink
Get Polymer compiling clean under closure recommended flags
Browse files Browse the repository at this point in the history
With these changes we have zero errors and zero warnings with `RECOMMENDED_FLAGS`!

Most of the changes were adding `@override` for methods and properties in mixins. Apparently if you implement an interface you need to say `@override` for each method or property on the interface. This combines with our mixin strategy to the tune of needing to add `@override` on every non-private method and property.

I'm not sure this is intended behavior of the compiler. Filed google/closure-compiler#3137 to see if it is.
  • Loading branch information
rictic committed Nov 4, 2018
1 parent 5341dbd commit 566dcfa
Show file tree
Hide file tree
Showing 12 changed files with 337 additions and 170 deletions.
23 changes: 22 additions & 1 deletion externs/polymer-externs.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ PolymerInit.prototype.hostAttributes;
/** @type {(!Object<string, string> | undefined)} */
PolymerInit.prototype.listeners;

/** @record */
let PolymerElementConstructor = function () {};
/** @type {(string | undefined)} */
PolymerElementConstructor.is;
Expand Down Expand Up @@ -78,6 +79,26 @@ function Polymer(init){}
*/
Polymer.sanitizeDOMValue;

/**
* @type {boolean}
*/
Polymer.passiveTouchGestures;

/**
* @type {boolean}
*/
Polymer.strictTemplatePolicy;

/**
* @type {boolean}
*/
Polymer.allowTemplateFromDomModule;

/**
* @type {string}
*/
Polymer.rootPath;

/**
* @param {string} string
* @param {Object} obj
Expand Down Expand Up @@ -174,7 +195,7 @@ var PolymerDeepPropertyChange;
* @constructor
* @template T
*/
let DomRepeatEvent = function() {};
var DomRepeatEvent = function() {};

/**
* @type {{
Expand Down
26 changes: 11 additions & 15 deletions lib/elements/array-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ let ArraySelectorMixin = dedupingMixin(superClass => {

/**
* @constructor
* @extends {superClass}
* @implements {Polymer_ElementMixin}
* @private
*/
Expand All @@ -52,7 +51,6 @@ let ArraySelectorMixin = dedupingMixin(superClass => {
class ArraySelectorMixin extends elementBase {

static get properties() {

return {

/**
Expand All @@ -76,31 +74,22 @@ let ArraySelectorMixin = dedupingMixin(superClass => {
* When `multi` is true, this is an array that contains any selected.
* When `multi` is false, this is the currently selected item, or `null`
* if no item is selected.
* @type {?(Object|Array<!Object>)}
* @type {?Object|?Array<!Object>}
*/
selected: {
type: Object,
notify: true
},
selected: {type: Object, notify: true},

/**
* When `multi` is false, this is the currently selected item, or `null`
* if no item is selected.
* @type {?Object}
*/
selectedItem: {
type: Object,
notify: true
},
selectedItem: {type: Object, notify: true},

/**
* When `true`, calling `select` on an item that is already selected
* will deselect the item.
*/
toggle: {
type: Boolean,
value: false
}
toggle: {type: Boolean, value: false}

};
}
Expand Down Expand Up @@ -208,6 +197,7 @@ let ArraySelectorMixin = dedupingMixin(superClass => {

/**
* Clears the selection state.
* @override
* @return {void}
*/
clearSelection() {
Expand All @@ -226,6 +216,7 @@ let ArraySelectorMixin = dedupingMixin(superClass => {
/**
* Returns whether the item is currently selected.
*
* @override
* @param {*} item Item from `items` array to test
* @return {boolean} Whether the item is selected
*/
Expand All @@ -236,6 +227,7 @@ let ArraySelectorMixin = dedupingMixin(superClass => {
/**
* Returns whether the item is currently selected.
*
* @override
* @param {number} idx Index from `items` array to test
* @return {boolean} Whether the item is selected
*/
Expand Down Expand Up @@ -265,6 +257,7 @@ let ArraySelectorMixin = dedupingMixin(superClass => {
/**
* Deselects the given item if it is already selected.
*
* @override
* @param {*} item Item from `items` array to deselect
* @return {void}
*/
Expand All @@ -288,6 +281,7 @@ let ArraySelectorMixin = dedupingMixin(superClass => {
/**
* Deselects the given index if it is already selected.
*
* @override
* @param {number} idx Index from `items` array to deselect
* @return {void}
*/
Expand All @@ -299,6 +293,7 @@ let ArraySelectorMixin = dedupingMixin(superClass => {
* Selects the given item. When `toggle` is true, this will automatically
* deselect the item if already selected.
*
* @override
* @param {*} item Item from `items` array to select
* @return {void}
*/
Expand All @@ -310,6 +305,7 @@ let ArraySelectorMixin = dedupingMixin(superClass => {
* Selects the given index. When `toggle` is true, this will automatically
* deselect the item if already selected.
*
* @override
* @param {number} idx Index from `items` array to select
* @return {void}
*/
Expand Down
25 changes: 12 additions & 13 deletions lib/legacy/legacy-data-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ const UndefinedArgumentError = class extends Error {

/**
* Wraps effect functions to catch `UndefinedArgumentError`s and warn.
*
*
* @param {Object=} effect Effect metadata object
* @param {Object=} fnName Name of user function, if known
* @return {?Object} Effect metadata object
* @return {?Object|undefined} Effect metadata object
*/
function wrapEffect(effect, fnName) {
if (effect && effect.fn) {
Expand All @@ -54,11 +54,11 @@ function wrapEffect(effect, fnName) {
* Mixin to selectively add back Polymer 1.x's `undefined` rules
* governing when observers & computing functions run based
* on all arguments being defined (reference https://www.polymer-project.org/1.0/docs/devguide/observers#multi-property-observers).
*
*
* When loaded, all legacy elements (defined with `Polymer({...})`)
* will have the mixin applied. The mixin only restores legacy data handling
* if `_legacyUndefinedCheck: true` is set on the element's prototype.
*
*
* This mixin is intended for use to help migration from Polymer 1.x to
* 2.x+ by allowing legacy code to work while identifying observers and
* computing functions that need undefined checks to work without
Expand All @@ -72,15 +72,14 @@ function wrapEffect(effect, fnName) {
export const LegacyDataMixin = dedupingMixin(superClass => {

/**
* @constructor
* @extends {superClass}
* @unrestricted
* @private */
* @private
*/
class LegacyDataMixin extends superClass {
/**
* Overrides `Polymer.PropertyEffects` to add `undefined` argument
* checking to match Polymer 1.x style rules
*
*
* @param {!Array<!MethodArg>} args Array of argument metadata
* @param {string} path Property/path name that triggered the method effect
* @param {Object} props Bag of current property changes
Expand All @@ -98,7 +97,7 @@ export const LegacyDataMixin = dedupingMixin(superClass => {
// Break out of effect's control flow; will be caught in
// wrapped property effect function below
const name = args[i].name;
throw new UndefinedArgumentError(`Argument '${name}' is undefined. Ensure it has an undefined check.`, name);
throw new UndefinedArgumentError(`Argument '${name}' is undefined.`, name);
}
}
}
Expand All @@ -108,7 +107,7 @@ export const LegacyDataMixin = dedupingMixin(superClass => {
/**
* Overrides `Polyer.PropertyEffects` to wrap effect functions to
* catch `UndefinedArgumentError`s and warn.
*
*
* @param {string} property Property that should trigger the effect
* @param {string} type Effect type, from this.PROPERTY_EFFECT_TYPES
* @param {Object=} effect Effect metadata object
Expand Down Expand Up @@ -143,9 +142,9 @@ export const LegacyDataMixin = dedupingMixin(superClass => {
// LegacyDataMixin is applied to base class _before_ metaprogramming, to
// ensure override of _addPropertyEffect et.al. are used by metaprogramming
// performed in _finalizeClass
Polymer.Class = (info, mixin) => Class(info,
superClass => mixin ?
mixin(LegacyDataMixin(superClass)) :
Polymer.Class = (info, mixin) => Class(info,
superClass => mixin ?
mixin(LegacyDataMixin(superClass)) :
LegacyDataMixin(superClass)
);

Expand Down
Loading

0 comments on commit 566dcfa

Please sign in to comment.