Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makes createProperty easier to use to customize properties #914

Merged
merged 16 commits into from
Mar 10, 2020
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## Unreleased

### Changed
* The value returned by `render` is always rendered, even if it isn't a `TemplateResult`. ([#712](https://github.com/Polymer/lit-element/issues/712)
* Added a static `createPropertyDescriptor` method to allow easier customization of property accessors. This method should return a a `PropertyDescriptor` to install on the property. If no descriptor is returned, a property accessor is not be created. ([#911](https://github.com/Polymer/lit-element/issues/911))
* The value returned by `render` is always rendered, even if it isn't a `TemplateResult`. ([#712](https://github.com/Polymer/lit-element/issues/712))

### Added
* Added `@queryAsync(selector)` decorator which returns a Promise that resolves to the result of querying for the given selector after the element's `updateComplete` Promise resolves ([#903](https://github.com/Polymer/lit-element/issues/903)).
Expand Down
2 changes: 1 addition & 1 deletion src/demo/ts-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class TSElement extends LitElement {
@property() message = 'Hi';

@property(
{attribute: 'more-info', converter: (value: string) => `[${value}]`})
{attribute: 'more-info', converter: (value: string|null) => `[${value}]`})
extra = '';

render() {
Expand Down
79 changes: 71 additions & 8 deletions src/lib/updating-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export interface ComplexAttributeConverter<Type = unknown, TypeHint = unknown> {
}

type AttributeConverter<Type = unknown, TypeHint = unknown> =
ComplexAttributeConverter<Type>|((value: string, type?: TypeHint) => Type);
ComplexAttributeConverter<Type>|((value: string|null, type?: TypeHint) => Type);

/**
* Defines options for a property accessor.
Expand Down Expand Up @@ -112,6 +112,10 @@ export interface PropertyDeclaration<Type = unknown, TypeHint = unknown> {
* the property changes.
*/
readonly noAccessor?: boolean;

// Allows extension while preserving the ability to use the
// @property decorator.
[index: string]: unknown;
}

/**
Expand Down Expand Up @@ -281,10 +285,25 @@ export abstract class UpdatingElement extends HTMLElement {
}

/**
* Creates a property accessor on the element prototype if one does not exist.
* Creates a property accessor on the element prototype if one does not exist
* and stores a PropertyDeclaration for the property with the given options.
* The property setter calls the property's `hasChanged` property option
* or uses a strict identity check to determine whether or not to request
* an update.
*
* This method may be overridden to customize properties; however,
* when doing so, it's important to call `super.createProperty` to ensure
* the property is setup correctly. This method calls
* `createPropertyDescriptor` internally to get a descriptor to install.
* To customize what properties do when they are get or set, override
* `createPropertyDescriptor`. To customize the options for a property,
* implement `createProperty` like this:
*
* static createProperty(name, options) {
* options = Object.assign(options, {myOption: true});
* super.createProperty(name, options);
* }
*
* @nocollapse
*/
static createProperty(
Expand All @@ -304,7 +323,38 @@ export abstract class UpdatingElement extends HTMLElement {
return;
}
const key = typeof name === 'symbol' ? Symbol() : `__${name}`;
sorvell marked this conversation as resolved.
Show resolved Hide resolved
Object.defineProperty(this.prototype, name, {
const descriptor = this.createPropertyDescriptor(name, key, options);
if (descriptor !== undefined) {
Object.defineProperty(this.prototype, name, descriptor);
}
}

/**
* Creates a property descriptor to be defined on the given named property.
* If no descriptor is returned, the property will not become an accessor.
* For example,
*
* class MyElement extends LitElement {
* static createPropertyDescriptor(name, key, options) {
* const defaultDescriptor = super.createPropertyDescriptor(name, key, options);
* const setter = defaultDescriptor.set;
* return {
* get: defaultDescriptor.get,
* set(value) {
* setter.call(this, value);
* // custom action.
* },
* configurable: true,
* enumerable: true
* }
* }
* }
*
* @nocollapse
*/
protected static createPropertyDescriptor(name: PropertyKey,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we name this get...? On getX vs. createX it seems like the distinction is that createX is side effectful and getX isn't.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

key: string|symbol, _options: PropertyDeclaration) {
return {
// tslint:disable-next-line:no-any no symbol in index
get(): any {
return (this as {[key: string]: unknown})[key as string];
Expand All @@ -317,7 +367,22 @@ export abstract class UpdatingElement extends HTMLElement {
},
configurable: true,
enumerable: true
});
};
}

/**
* Returns the property options associated with the given property.
* These options are defined with a PropertyDeclaration via the `properties`
* object or the `@property` decorator and are registered in
* `createProperty(...)`.
*
* Note, this method should be considered "final" and not overridden. To
* customize the options for a given property, override `createProperty`.
*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a @final annotation. Not sure if these flow through from TypeScript to Closure via tsickle or not, but it doesn't hurt.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

*/
protected static getPropertyOptions(name: PropertyKey) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be private? It seems to only be accessed from other private methods. If so, rename to _getPropertyOptions

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's protected so options can be used in the element lifecycle. For example, in updated you might want to act on a given changed property if it has a specific option.

return this._classProperties && this._classProperties.get(name) ||
defaultPropertyDeclaration;
}

/**
Expand Down Expand Up @@ -563,8 +628,7 @@ export abstract class UpdatingElement extends HTMLElement {
const ctor = (this.constructor as typeof UpdatingElement);
const propName = ctor._attributeToPropertyMap.get(name);
if (propName !== undefined) {
const options =
ctor._classProperties!.get(propName) || defaultPropertyDeclaration;
const options = ctor.getPropertyOptions(propName);
// mark state reflecting
this._updateState = this._updateState | STATE_IS_REFLECTING_TO_PROPERTY;
this[propName as keyof this] =
Expand All @@ -585,8 +649,7 @@ export abstract class UpdatingElement extends HTMLElement {
// If we have a property key, perform property update steps.
if (name !== undefined) {
const ctor = this.constructor as typeof UpdatingElement;
const options =
ctor._classProperties!.get(name) || defaultPropertyDeclaration;
const options = ctor.getPropertyOptions(name);
if (ctor._valueHasChanged(
this[name as keyof this], oldValue, options.hasChanged)) {
if (!this._changedProperties.has(name)) {
Expand Down
132 changes: 130 additions & 2 deletions src/test/lib/updating-element_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
* http://polymer.github.io/PATENTS.txt
*/

import {property} from '../../lib/decorators.js';
import {ComplexAttributeConverter, PropertyDeclarations, PropertyValues, UpdatingElement} from '../../lib/updating-element.js';
import {property, customElement} from '../../lib/decorators.js';
import {ComplexAttributeConverter, PropertyDeclarations, PropertyValues, UpdatingElement, PropertyDeclaration, defaultConverter} from '../../lib/updating-element.js';
import {generateElementName} from '../test-helpers.js';

// tslint:disable:no-any ok in tests
Expand Down Expand Up @@ -1802,6 +1802,134 @@ suite('UpdatingElement', () => {
assert.equal(sub.getAttribute('foo'), '5');
});

test('can provide a default property declaration', async () => {
sorvell marked this conversation as resolved.
Show resolved Hide resolved

const SpecialNumber = {};

const myPropertyDeclaration = {
type: SpecialNumber,
reflect: true,
converter: {
toAttribute: function(value: unknown, type?: unknown): unknown {
switch (type) {
case String:
return value === undefined ? null : value;
default:
return defaultConverter.toAttribute!(value, type);
}
},
fromAttribute: function(value: string|null, type?: unknown) {
switch (type) {
case SpecialNumber:
return Number(value) + 10;
default:
return defaultConverter.fromAttribute!(value, type);
}
}
}
};

@customElement(generateElementName())
class E extends UpdatingElement {

static createProperty(
name: PropertyKey,
options: PropertyDeclaration) {
// Always mix into defaults to preserve custom converter.
options = Object.assign(Object.create(myPropertyDeclaration), options);
super.createProperty(name, options);
}

@property()
foo = 5;

@property({type: String})
bar?: string = 'bar';
}

const el = new E();
container.appendChild(el);
el.setAttribute('foo', '10');
el.setAttribute('bar', 'attrBar');
await el.updateComplete;
assert.equal(el.foo, 20);
assert.equal(el.bar, 'attrBar');
el.foo = 5;
el.bar = undefined;
await el.updateComplete;
assert.equal(el.getAttribute('foo'), '5');
assert.isFalse(el.hasAttribute('bar'));
});

test('can customize property options and accessor creation', async () => {

interface MyPropertyDeclaration<TypeHint = unknown> extends PropertyDeclaration {
validator?: (value: any) => TypeHint;
observer?: (oldValue: TypeHint) => void;
}

@customElement(generateElementName())
class E extends UpdatingElement {

static createPropertyDescriptor(name: PropertyKey, key: string|symbol, options: MyPropertyDeclaration) {
const defaultDescriptor = super.createPropertyDescriptor(name, key, options);
return {
get: defaultDescriptor.get,
set(this: E, value: unknown) {
const oldValue =
(this as unknown as {[key: string]: unknown})[name as string];
if (options.validator) {
value = options.validator(value);
}
(this as unknown as {[key: string]: unknown})[key as string] = value;
(this as unknown as UpdatingElement).requestUpdate(name, oldValue);
},

configurable: defaultDescriptor.configurable,
enumerable: defaultDescriptor.enumerable
};
}

updated(changedProperties: PropertyValues) {
super.updated(changedProperties);
changedProperties.forEach((value: unknown, key: PropertyKey) => {
const options = (this.constructor as typeof UpdatingElement)
.getPropertyOptions(key) as MyPropertyDeclaration;
const observer = options.observer;
if (typeof observer === 'function') {
observer.call(this, value);
}
});
}

@property({type: Number, validator: (value: number) => Math.min(10, Math.max(value, 0))})
foo = 5;

@property({})
bar = 'bar';

// tslint:disable-next-line:no-any
_observedZot?: any;

@property({observer: function(this: E, oldValue: string) { this._observedZot = {value: this.zot, oldValue}; } })
zot = '';
}

const el = new E();
container.appendChild(el);
await el.updateComplete;
el.foo = 20;
assert.equal(el.foo, 10);
assert.deepEqual(el._observedZot, {value: '', oldValue: undefined});
el.foo = -5;
assert.equal(el.foo, 0);
el.bar = 'bar2';
assert.equal(el.bar, 'bar2');
el.zot = 'zot';
await el.updateComplete;
assert.deepEqual(el._observedZot, {value: 'zot', oldValue: ''});
});

test('attribute-based property storage', async () => {
class E extends UpdatingElement {
_updateCount = 0;
Expand Down