Skip to content

Commit

Permalink
Expose less protected data.
Browse files Browse the repository at this point in the history
* Hide _own* getters since they are unnecessary
* declared properties always have accessors unless they would stomp existing accessors (or have property  effects)
  • Loading branch information
Steven Orvell committed Nov 15, 2017
1 parent d331c67 commit 74fb515
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 58 deletions.
42 changes: 21 additions & 21 deletions lib/mixins/element-mixin.html
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@
if (!klass.hasOwnProperty(
JSCompiler_renameProperty('__classPropertyDefaults', klass))) {
klass.__classPropertyDefaults = null;
//let props = propertiesForClass(klass);
let props = klass._properties;
for (let p in props) {
let info = props[p];
Expand All @@ -130,6 +129,22 @@
return klass.__classPropertyDefaults;
}

/**
* Returns a memoized version of the the `observers` array.
* @param {PolymerElementConstructor} klass Element class
* @return {Array} Array containing own observers for the given class
* @protected
*/
function ownObserversForClass(klass) {
if (!klass.hasOwnProperty(
JSCompiler_renameProperty('__ownObservers', klass))) {
klass.__ownObservers =
klass.hasOwnProperty(JSCompiler_renameProperty('observers', klass)) ?
/** @type {PolymerElementConstructor} */ (klass).observers : null;
}
return klass.__ownObservers;
}

/**
* Creates effects for a property.
*
Expand Down Expand Up @@ -218,8 +233,9 @@
if (info.observer) {
proto._createPropertyObserver(name, info.observer, allProps[info.observer]);
}
// always ensure an accessor is made for properties
if (!info.readOnly) {
// always ensure an accessor is made for properties but don't stomp
// on existing values.
if (!info.readOnly && !(name in proto)) {
proto._createPropertyAccessor(name);
}
}
Expand All @@ -232,31 +248,15 @@
*/
class PolymerElement extends polymerElementBase {

/**
* Returns a memoized version of the the `observers` array. Use for
*
* @return {Array} Array containing own observers for this class
* @protected
*/
static get _ownObservers() {
if (!this.hasOwnProperty(
JSCompiler_renameProperty('__ownObservers', this))) {
this.__ownObservers =
this.hasOwnProperty(JSCompiler_renameProperty('observers', this)) ?
/** @type {PolymerElementConstructor} */ (this).observers : null;
}
return this.__ownObservers;
}

static _finalizeClass() {
super._finalizeClass();
if (this.hasOwnProperty(
JSCompiler_renameProperty('is', this)) && this.is) {
Polymer.telemetry.register(this.prototype);
}
const observers = this._ownObservers;
const observers = ownObserversForClass(this);
if (observers) {
this.createObservers(observers, this._ownProperties);
this.createObservers(observers, this._properties);
}
// note: create "working" template that is finalized at instance time
let template = /** @type {PolymerElementConstructor} */ (this).template;
Expand Down
6 changes: 5 additions & 1 deletion lib/mixins/properties-changed.html
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,12 @@
* @param {Object} props Object whose keys are names of accessors.
*/
static createProperties(props) {
const proto = this.prototype;
for (let prop in props) {
this.prototype._createPropertyAccessor(prop);
// don't stomp an existing accessor
if (!(prop in proto)) {
proto._createPropertyAccessor(prop);
}
}
}

Expand Down
73 changes: 42 additions & 31 deletions lib/mixins/properties-mixin.html
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,43 @@
}
}

/**
* Returns a memoized version of the `properties` object for the
* given class. Properties not in object format are converted to at
* least {type}.
*
* @param {PropertiesClassConstructor} ctor PropertiesClass constructor
* @return {object} Memoized properties object
*/
function ownPropertiesForClass(ctor) {
if (!ctor.hasOwnProperty(
JSCompiler_renameProperty('__ownProperties', ctor))) {
const props = ctor.properties;
ctor.__ownProperties = props ? mixProperties({}, props) : null;
}
return ctor.__ownProperties;
}

/**
* Returns a memoized version attribute info for the
* given class.
*
* @param {PropertiesClassConstructor} ctor PropertiesClass constructor
* @return {object} Memoized attribute info object
*/
function attributeInfoForClass(ctor) {
if (!ctor.hasOwnProperty(
JSCompiler_renameProperty('__attributeInfo', ctor))) {
const proto = ctor.prototype;
const attrInfo = ctor.__attributeInfo = {};
const props = ctor._properties;
for (let prop in props) {
attrInfo[proto._attributeForProperty(prop)] = prop;
}
}
return ctor.__attributeInfo;
}

/**
* @polymer
* @mixinClass
Expand Down Expand Up @@ -131,27 +168,12 @@
* @protected
*/
static _finalizeClass() {
const props = this._ownProperties;
const props = ownPropertiesForClass(this);
if (props) {
this.createProperties(props);
}
}

/**
* Returns a memoized version of the `properties` object. Properties
* not in object format are converted to at lesat {type}.
*
* @return {Object} Object containing own properties for this class
* @protected
*/
static get _ownProperties() {
if (!this.hasOwnProperty(JSCompiler_renameProperty('__ownProperties', this))) {
const props = this.properties;
this.__ownProperties = props ? mixProperties({}, props) : null;
}
return this.__ownProperties;
}

/**
* Returns a memoized version of all properties, including those inherited
* from super classes. Properties not in object format are converted to
Expand All @@ -161,28 +183,17 @@
* @protected
*/
static get _properties() {
if (!this.hasOwnProperty(JSCompiler_renameProperty('__properties', this))) {
if (!this.hasOwnProperty(
JSCompiler_renameProperty('__properties', this))) {
const superCtor = superForClass(this);
this.__properties = Object.assign({},
superCtor && superCtor._properties, this._ownProperties);
superCtor && superCtor._properties, ownPropertiesForClass(this));
}
return this.__properties;
}

static get _attributeInfo() {
if (!this.hasOwnProperty(JSCompiler_renameProperty('__attributeInfo', this))) {
const proto = this.prototype;
const attrInfo = this.__attributeInfo = {};
const props = this._properties;
for (let prop in props) {
attrInfo[proto._attributeForProperty(prop)] = prop;
}
}
return this.__attributeInfo;
}

_propertyForAttribute(name) {
return this.constructor._attributeInfo[name] ||
return attributeInfoForClass(this.constructor)[name] ||
super._propertyForAttribute(name);
}

Expand Down
21 changes: 17 additions & 4 deletions test/unit/polymer.element.html
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@
computedProp: {
computed: '_compute(computedPropDep)'
},
accessor: {
accessor: true
}
accessor: String,
noStomp: String
};
}

Expand All @@ -59,6 +58,10 @@
this._calledConstructor++;
}

get noStomp() {
return 'noStomp';
}

connectedCallback() {
super.connectedCallback();
this._calledConnectedCallback++;
Expand Down Expand Up @@ -386,13 +389,23 @@ <h1>Sub template</h1>
assert.equal(el.computedProp, true);
});

test('properties accessor: true creates accessor', function() {
test('properties have accessors', function() {
assert.isTrue(el.__dataHasAccessor.accessor);
el._propertiesChanged = sinon.spy();
el.accessor = 'hi';
assert.equal(el._propertiesChanged.args[0][0].accessor, 'hi');
});

test('properties with no effects do not stomp existing accessors', function() {
assert.notOk(el.__dataHasAccessor.noStomp);
el._propertiesChanged = sinon.spy();
el.accessor = 'hi';
el.noStomp = 'hi';
const props = el._propertiesChanged.args[0][0];
assert.equal(props.accessor, 'hi');
assert.notOk(props.noStomp);
});

test('attributes', function() {
const fixtureEl = fixture('my-element-attr');
assert.equal(fixtureEl.prop, 'attr');
Expand Down
11 changes: 10 additions & 1 deletion test/unit/polymer.properties-element.html
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@

static get properties() {
return {
prop: String
prop: String,
noStomp: String
};
}

Expand All @@ -34,8 +35,15 @@
this._tapListener = sinon.spy();
this._calledConstructor++;
this.prop = 'prop';
this.noStomp = 'stomped';
}

get noStomp() {
return 'noStomp';
}

set noStomp(value) {}

connectedCallback() {
super.connectedCallback();
this._calledConnectedCallback++;
Expand Down Expand Up @@ -228,6 +236,7 @@

test('properties', function() {
assert.equal(el.prop, 'prop');
assert.equal(el.noStomp, 'noStomp');
assert.isTrue(el._propertiesChanged.calledOnce);
assert.deepEqual(el._propertiesChanged.args[0][0], {prop: 'prop'});
});
Expand Down

0 comments on commit 74fb515

Please sign in to comment.