Skip to content

Commit 297b8b2

Browse files
author
Steven Orvell
committed
Address review feedback.
1 parent 734adc5 commit 297b8b2

File tree

2 files changed

+38
-21
lines changed

2 files changed

+38
-21
lines changed

src/lib/updating-element.ts

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -285,10 +285,24 @@ export abstract class UpdatingElement extends HTMLElement {
285285
}
286286

287287
/**
288-
* Creates a property accessor on the element prototype if one does not exist.
288+
* Creates a property accessor on the element prototype if one does not exist
289+
* and stores a PropertyDeclaration for the property with the given options.
289290
* The property setter calls the property's `hasChanged` property option
290291
* or uses a strict identity check to determine whether or not to request
291292
* an update.
293+
*
294+
* This method may be overridden to customize properties; however,
295+
* when doing so, it's important to call `super.createProperty` to ensure
296+
* the property is setup correctly. This method calls
297+
* `createPropertyDescriptor` internally to get a descriptor to install.
298+
* To customize what properties do when they are get or set, override
299+
* `createPropertyDescriptor`. To customize the options for a property,
300+
*
301+
* static createProperty(name, options) {
302+
* options = Object.assign(options, {myOption: true});
303+
* super.createProperty(name, options);
304+
* }
305+
*
292306
* @nocollapse
293307
*/
294308
static createProperty(
@@ -338,7 +352,7 @@ export abstract class UpdatingElement extends HTMLElement {
338352
* @nocollapse
339353
*/
340354
protected static createPropertyDescriptor(name: PropertyKey,
341-
key: string|symbol, _options: PropertyDeclaration,) {
355+
key: string|symbol, _options: PropertyDeclaration) {
342356
return {
343357
// tslint:disable-next-line:no-any no symbol in index
344358
get(): any {
@@ -355,7 +369,17 @@ export abstract class UpdatingElement extends HTMLElement {
355369
};
356370
}
357371

358-
protected static getDeclarationForProperty(name: PropertyKey) {
372+
/**
373+
* Returns the property options associated with the given property.
374+
* These options are defined with a PropertyDeclaration via the `properties`
375+
* object or the `@property` decorator and are registered in
376+
* `createProperty(...)`.
377+
*
378+
* Note, this method should be considered "final" and not overridden. To
379+
* customize the options for a given property, override `createProperty`.
380+
*
381+
*/
382+
protected static getPropertyOptions(name: PropertyKey) {
359383
return this._classProperties && this._classProperties.get(name) ||
360384
defaultPropertyDeclaration;
361385
}
@@ -432,10 +456,9 @@ export abstract class UpdatingElement extends HTMLElement {
432456
private static _propertyValueFromAttribute(
433457
value: string|null, options: PropertyDeclaration) {
434458
const type = options.type;
435-
const converter = options.converter ||
436-
defaultPropertyDeclaration.converter;
459+
const converter = options.converter || defaultConverter;
437460
const fromAttribute =
438-
(typeof converter === 'function' ? converter : converter!.fromAttribute);
461+
(typeof converter === 'function' ? converter : converter.fromAttribute);
439462
return fromAttribute ? fromAttribute(value, type) : value;
440463
}
441464

@@ -453,14 +476,11 @@ export abstract class UpdatingElement extends HTMLElement {
453476
return;
454477
}
455478
const type = options.type;
456-
let converter = options.converter;
457-
let toAttribute =
458-
converter && (converter as ComplexAttributeConverter).toAttribute;
459-
if (!toAttribute) {
460-
converter = defaultPropertyDeclaration.converter;
461-
toAttribute = converter && (converter as ComplexAttributeConverter).toAttribute;
462-
}
463-
return toAttribute ? toAttribute(value, type) : value;
479+
const converter = options.converter;
480+
const toAttribute =
481+
converter && (converter as ComplexAttributeConverter).toAttribute ||
482+
defaultConverter.toAttribute;
483+
return toAttribute!(value, type);
464484
}
465485

466486
private _updateState: UpdateState = 0;
@@ -607,7 +627,7 @@ export abstract class UpdatingElement extends HTMLElement {
607627
const ctor = (this.constructor as typeof UpdatingElement);
608628
const propName = ctor._attributeToPropertyMap.get(name);
609629
if (propName !== undefined) {
610-
const options = ctor.getDeclarationForProperty(propName);
630+
const options = ctor.getPropertyOptions(propName);
611631
// mark state reflecting
612632
this._updateState = this._updateState | STATE_IS_REFLECTING_TO_PROPERTY;
613633
this[propName as keyof this] =
@@ -628,7 +648,7 @@ export abstract class UpdatingElement extends HTMLElement {
628648
// If we have a property key, perform property update steps.
629649
if (name !== undefined) {
630650
const ctor = this.constructor as typeof UpdatingElement;
631-
const options = ctor.getDeclarationForProperty(name);
651+
const options = ctor.getPropertyOptions(name);
632652
if (ctor._valueHasChanged(
633653
this[name as keyof this], oldValue, options.hasChanged)) {
634654
if (!this._changedProperties.has(name)) {
@@ -721,9 +741,6 @@ export abstract class UpdatingElement extends HTMLElement {
721741
* ```
722742
*/
723743
protected performUpdate(): void|Promise<unknown> {
724-
if (!this._hasRequestedUpdate) {
725-
return;
726-
}
727744
// Mixin instance properties once, if they exist.
728745
if (this._instanceProperties) {
729746
this._applyInstanceProperties();

src/test/lib/updating-element_test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1874,7 +1874,7 @@ suite('UpdatingElement', () => {
18741874
static createPropertyDescriptor(name: PropertyKey, key: string|symbol, options: MyPropertyDeclaration) {
18751875
const defaultDescriptor = super.createPropertyDescriptor(name, key, options);
18761876
return {
1877-
get: defaultDescriptor!.get,
1877+
get: defaultDescriptor.get,
18781878
set(this: E, value: unknown) {
18791879
const oldValue =
18801880
(this as unknown as {[key: string]: unknown})[name as string];
@@ -1894,7 +1894,7 @@ suite('UpdatingElement', () => {
18941894
super.updated(changedProperties);
18951895
changedProperties.forEach((value: unknown, key: PropertyKey) => {
18961896
const options = (this.constructor as typeof UpdatingElement)
1897-
.getDeclarationForProperty(key) as MyPropertyDeclaration;
1897+
.getPropertyOptions(key) as MyPropertyDeclaration;
18981898
const observer = options.observer;
18991899
if (typeof observer === 'function') {
19001900
observer.call(this, value);

0 commit comments

Comments
 (0)