Makes `createProperty` easier to use to customize properties #914

sorvell · 2020-03-05T22:42:47Z

Fixes #911. It is now easier to override property accessor creation. A new static getPropertyDescriptor(name: PropertyKey, key: string|symbol, options: PropertyDeclaraction) is added here. This method should return a property descriptor to use when defining the property. If no descriptor is returned, the property will not become an accessor.

Example:

class MyElement extends LitElement {
  static getPropertyDescriptor(
      name: PropertyKey,
      key: string|symbol,
      options: PropertyDeclaration) {

    const defaultDescriptor = super.getPropertyDescriptor(name, key, options); 
    const setter = defaultDescriptor.set;
    return Object.assign(descriptor, {
      set(this: MyElement, value: unknown) {
        setter.call(this, value);
        console.log('Property', name, 'was set to', value);
     }
  }
  //...
}

Fixes #905. A couple changes to make customizing property accessors easier. * Adds static methods`setOptionsForProperty(name, options)`, `getOptionsForProperty` and renames `_requestUpdate` to `requestUpdateInternal` and makes it protected. With these changes, users can override `createProperty` and (1) pass in and later retrieve custom property otions, (2) customize how accessors are defined and call `requestUpdateInternal` in the setter to make the property reactive.

This function receives the property declaration, key of the private storage for the property, and default property descriptor. It should return a PropertyDescriptor. Users can customer this descriptor to control how property accessors behave.

src/lib/updating-element.ts

This method is called internallly by `createProperty` to get a descriptor to install on the property. Users can implement it to customize what property accessors do.

Users can implement setters with `requestUpdate`.

src/lib/updating-element.ts

src/test/lib/updating-element_test.ts

src/lib/updating-element.ts

kevinpschaaf

LGTM

Ping @justinfagnani to weigh in on getPropertyOptions vs. getDeclarationForProperty naming before it's too late... "Declaration" is a sort of obtuse name for the options, and we call the argument options throughout the code, but we've already grandfathered "Declaration" in the type. I like options better, but I can see a consistency argument either way.

justinfagnani

Looks good as-is, but I had a couple of small naming questions.

justinfagnani · 2020-03-10T01:24:23Z

src/lib/updating-element.ts

+   *
+   * Note, this method should be considered "final" and not overridden. To
+   * customize the options for a given property, override `createProperty`.
+   *


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

justinfagnani · 2020-03-10T01:25:29Z

src/lib/updating-element.ts

+   * customize the options for a given property, override `createProperty`.
+   *
+   */
+  protected static getPropertyOptions(name: PropertyKey) {


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

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.

justinfagnani · 2020-03-10T01:26:43Z

src/lib/updating-element.ts

+   *
+   * @nocollapse
+   */
+  protected static createPropertyDescriptor(name: PropertyKey,


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

Steven Orvell added 5 commits February 28, 2020 17:21

Adds tests for property customization

97789b5

Chage argument order.

45ede17

Update CHANGELOG.md

5e317f6

sorvell requested review from justinfagnani and kevinpschaaf March 5, 2020 22:42

googlebot added the cla: yes label Mar 5, 2020

Steven Orvell added 2 commits March 5, 2020 15:00

Lint fix.

e2ef7d9

Expands property customization test

d0ecc90

justinfagnani requested changes Mar 9, 2020

View reviewed changes

src/lib/updating-element.ts Outdated Show resolved Hide resolved

src/lib/updating-element.ts Show resolved Hide resolved

src/lib/updating-element.ts Outdated Show resolved Hide resolved

src/lib/updating-element.ts Outdated Show resolved Hide resolved

Steven Orvell added 4 commits March 9, 2020 10:35

Improved type.

b96a366

Adds createPropertyDescriptor(name, key, options)

a33f63f

This method is called internallly by `createProperty` to get a descriptor to install on the property. Users can implement it to customize what property accessors do.

Switch back to private _requestUpdate

5479afa

Users can implement setters with `requestUpdate`.

Adds API doc for createPropertyDescriptor

734adc5

kevinpschaaf reviewed Mar 9, 2020

View reviewed changes

src/lib/updating-element.ts Outdated Show resolved Hide resolved

src/lib/updating-element.ts Outdated Show resolved Hide resolved

src/test/lib/updating-element_test.ts Show resolved Hide resolved

src/lib/updating-element.ts Outdated Show resolved Hide resolved

Steven Orvell added 4 commits March 9, 2020 17:14

Address review feedback.

297b8b2

Fix spacing.

def6df1

Fix spacing.

ec9048b

Update comment.

8f5815f

kevinpschaaf approved these changes Mar 10, 2020

View reviewed changes

justinfagnani approved these changes Mar 10, 2020

View reviewed changes

Address review feedback.

90c85a5

sorvell merged commit 8055b93 into master Mar 10, 2020

sorvell deleted the customize-properties branch March 10, 2020 19:58

justinfagnani mentioned this pull request Mar 14, 2020

Can we help you possibly use LitElement? Netflix/x-element#53

Open

sorvell mentioned this pull request Mar 17, 2020

PropertyDeclaration added index signature incompatible with previous type #928

Closed

kevinpschaaf mentioned this pull request Apr 14, 2020

Makes performUpdate possible to call synchronously #959

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Makes `createProperty` easier to use to customize properties #914

Makes `createProperty` easier to use to customize properties #914

sorvell commented Mar 5, 2020 •

edited by kevinpschaaf

kevinpschaaf left a comment •

edited

justinfagnani left a comment

justinfagnani Mar 10, 2020

sorvell Mar 10, 2020

justinfagnani Mar 10, 2020

sorvell Mar 10, 2020

justinfagnani Mar 10, 2020

sorvell Mar 10, 2020

Makes createProperty easier to use to customize properties #914

Makes createProperty easier to use to customize properties #914

Conversation

sorvell commented Mar 5, 2020 • edited by kevinpschaaf

kevinpschaaf left a comment • edited

Choose a reason for hiding this comment

justinfagnani left a comment

Choose a reason for hiding this comment

justinfagnani Mar 10, 2020

Choose a reason for hiding this comment

sorvell Mar 10, 2020

Choose a reason for hiding this comment

justinfagnani Mar 10, 2020

Choose a reason for hiding this comment

sorvell Mar 10, 2020

Choose a reason for hiding this comment

justinfagnani Mar 10, 2020

Choose a reason for hiding this comment

sorvell Mar 10, 2020

Choose a reason for hiding this comment

Makes `createProperty` easier to use to customize properties #914

Makes `createProperty` easier to use to customize properties #914

sorvell commented Mar 5, 2020 •

edited by kevinpschaaf

kevinpschaaf left a comment •

edited