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

[reactive-element] Adds decorators for @compute, @observe, and @listen #1870

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sorvell
Copy link
Member

@sorvell sorvell commented May 11, 2021

Fixes #1873.

  • Adds @computed decorator for creating computed properties. Decorator specifies an array of deps which are the names of dependent properties and a compute function which is called with dependency values and should return the computed property value.

  • Adds @observe decorator for observing property changes. Decorator specifies an observe function which receives the current and previous value of the property as well as the name of the property. Note, the observed property must be a reactive property.

  • Adds @listen decorator for declaratively adding event listeners. The decorator should be placed on an element property which is an event listener function. The decorator must provide an object which specifies the event type, options, and an optional target. The target defaults to the element itself or may be root to add the listener to the element's
    renderRoot, or an instance of EventTarget. If target is an EventTarget,
    the listener is dynamically added and removed when the element is connected and disconnected.

* Adds `@computed` decorator for creating computed properties. Decorator specifies an array of deps which are the names of dependent properties and a compute function which is called with dependency values and should return the computed property value.

* Adds `@observe` decorator for observing property changes. Decorator specifies an observe function which receives the current and previous value of the property as well as the name of the property. Note, the observed property must be a reactive property.

* Adds `@listen` decorator for declaratively adding event listeners. The decorator should be placed on an element property which is an event listener function. The decorator must provide an object which specifies the event `type`, `options`, and an optional `target`. The `target` defaults to the element itself or may be `root` to add the listener to the element's
 `renderRoot`, or an instance of `EventTarget`. If `target` is an `EventTarget`,
the listener is dynamically added and removed when the element is connected and disconnected.
@github-actions
Copy link
Contributor

github-actions bot commented May 11, 2021

📊 Tachometer Benchmark Results

Summary

nop-update

  • lit-html-kitchen-sink: unsure 🔍 -2% - +8% (-0.76ms - +2.97ms)
    this-change vs tip-of-tree

render

  • lit-element-list: unsure 🔍 -3% - +3% (-3.37ms - +3.46ms)
    this-change vs tip-of-tree
  • lit-html-kitchen-sink: unsure 🔍 -3% - +5% (-1.46ms - +2.27ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -9% - +5% (-1.22ms - +0.65ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -1% - +5% (-0.33ms - +3.46ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -4% - +4% (-2.67ms - +2.69ms)
    this-change vs tip-of-tree

update

  • lit-element-list: unsure 🔍 -3% - +1% (-22.62ms - +9.84ms)
    this-change vs tip-of-tree
  • lit-html-kitchen-sink: unsure 🔍 -5% - +3% (-5.27ms - +3.46ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -1% - +3% (-2.30ms - +11.59ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -3% - +2% (-3.81ms - +3.09ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -2% - +2% (-13.74ms - +15.25ms)
    this-change vs tip-of-tree

update-reflect

  • lit-element-list: unsure 🔍 -3% - +1% (-25.04ms - +7.12ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -2% - +1% (-20.91ms - +12.53ms)
    this-change vs tip-of-tree

Results

lit-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
106.22ms - 111.24ms-unsure 🔍
-3% - +3%
-3.37ms - +3.46ms
faster ✔
23% - 27%
32.36ms - 39.66ms
tip-of-tree
tip-of-tree
106.37ms - 111.00msunsure 🔍
-3% - +3%
-3.46ms - +3.37ms
-faster ✔
23% - 27%
32.54ms - 39.58ms
previous-release
previous-release
142.09ms - 147.39msslower ❌
29% - 37%
32.36ms - 39.66ms
slower ❌
29% - 37%
32.54ms - 39.58ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
839.92ms - 860.78ms-unsure 🔍
-3% - +1%
-22.62ms - +9.84ms
faster ✔
8% - 11%
70.42ms - 99.16ms
tip-of-tree
tip-of-tree
844.30ms - 869.17msunsure 🔍
-1% - +3%
-9.84ms - +22.62ms
-faster ✔
7% - 10%
62.52ms - 94.29ms
previous-release
previous-release
925.26ms - 945.02msslower ❌
8% - 12%
70.42ms - 99.16ms
slower ❌
7% - 11%
62.52ms - 94.29ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
888.98ms - 908.42ms-unsure 🔍
-3% - +1%
-25.04ms - +7.12ms
faster ✔
5% - 8%
42.58ms - 72.95ms
tip-of-tree
tip-of-tree
894.85ms - 920.48msunsure 🔍
-1% - +3%
-7.12ms - +25.04ms
-faster ✔
3% - 7%
31.47ms - 66.13ms
previous-release
previous-release
944.79ms - 968.13msslower ❌
5% - 8%
42.58ms - 72.95ms
slower ❌
3% - 7%
31.47ms - 66.13ms
-
lit-html-kitchen-sink

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
44.55ms - 47.65ms-unsure 🔍
-3% - +5%
-1.46ms - +2.27ms
faster ✔
10% - 17%
5.17ms - 9.41ms
tip-of-tree
tip-of-tree
44.66ms - 46.73msunsure 🔍
-5% - +3%
-2.27ms - +1.46ms
-faster ✔
11% - 17%
5.92ms - 9.48ms
previous-release
previous-release
51.94ms - 54.84msslower ❌
11% - 21%
5.17ms - 9.41ms
slower ❌
13% - 21%
5.92ms - 9.48ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
112.33ms - 119.07ms-unsure 🔍
-5% - +3%
-5.27ms - +3.46ms
faster ✔
1% - 9%
1.25ms - 11.75ms
tip-of-tree
tip-of-tree
113.83ms - 119.38msunsure 🔍
-3% - +5%
-3.46ms - +5.27ms
-faster ✔
1% - 8%
0.70ms - 10.48ms
previous-release
previous-release
118.18ms - 126.22msslower ❌
1% - 10%
1.25ms - 11.75ms
slower ❌
1% - 9%
0.70ms - 10.48ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
37.96ms - 41.13ms-unsure 🔍
-2% - +8%
-0.76ms - +2.97ms
slower ❌
1% - 12%
0.50ms - 4.48ms
tip-of-tree
tip-of-tree
37.45ms - 39.42msunsure 🔍
-7% - +2%
-2.97ms - +0.76ms
-unsure 🔍
-1% - +8%
-0.17ms - +2.94ms
previous-release
previous-release
35.84ms - 38.26msfaster ✔
1% - 11%
0.50ms - 4.48ms
unsure 🔍
-8% - +0%
-2.94ms - +0.17ms
-
lit-html-repeat

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
13.26ms - 14.31ms-unsure 🔍
-9% - +5%
-1.22ms - +0.65ms
faster ✔
9% - 22%
1.23ms - 3.73ms
tip-of-tree
tip-of-tree
13.29ms - 14.83msunsure 🔍
-5% - +9%
-0.65ms - +1.22ms
-faster ✔
6% - 21%
0.83ms - 3.57ms
previous-release
previous-release
15.13ms - 17.40msslower ❌
9% - 27%
1.23ms - 3.73ms
slower ❌
5% - 26%
0.83ms - 3.57ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
395.41ms - 406.05ms-unsure 🔍
-1% - +3%
-2.30ms - +11.59ms
faster ✔
27% - 30%
151.17ms - 169.67ms
tip-of-tree
tip-of-tree
391.63ms - 400.56msunsure 🔍
-3% - +1%
-11.59ms - +2.30ms
-faster ✔
28% - 31%
156.28ms - 173.84ms
previous-release
previous-release
553.59ms - 568.72msslower ❌
37% - 43%
151.17ms - 169.67ms
slower ❌
39% - 44%
156.28ms - 173.84ms
-
lit-html-template-heavy

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
68.52ms - 71.53ms-unsure 🔍
-1% - +5%
-0.33ms - +3.46ms
faster ✔
14% - 19%
11.89ms - 15.80ms
tip-of-tree
tip-of-tree
67.31ms - 69.61msunsure 🔍
-5% - +0%
-3.46ms - +0.33ms
-faster ✔
17% - 20%
13.71ms - 17.10ms
previous-release
previous-release
82.62ms - 85.12msslower ❌
17% - 23%
11.89ms - 15.80ms
slower ❌
20% - 25%
13.71ms - 17.10ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
132.39ms - 137.37ms-unsure 🔍
-3% - +2%
-3.81ms - +3.09ms
faster ✔
12% - 17%
19.39ms - 26.48ms
tip-of-tree
tip-of-tree
132.85ms - 137.63msunsure 🔍
-2% - +3%
-3.09ms - +3.81ms
-faster ✔
12% - 16%
19.10ms - 26.05ms
previous-release
previous-release
155.30ms - 160.33msslower ❌
14% - 20%
19.39ms - 26.48ms
slower ❌
14% - 19%
19.10ms - 26.05ms
-
reactive-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
74.03ms - 77.84ms-unsure 🔍
-4% - +4%
-2.67ms - +2.69ms
unsure 🔍
-1% - +6%
-0.39ms - +4.67ms
tip-of-tree
tip-of-tree
74.03ms - 77.81msunsure 🔍
-4% - +4%
-2.69ms - +2.67ms
-unsure 🔍
-1% - +6%
-0.40ms - +4.65ms
previous-release
previous-release
72.12ms - 75.47msunsure 🔍
-6% - +0%
-4.67ms - +0.39ms
unsure 🔍
-6% - +0%
-4.65ms - +0.40ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
837.40ms - 859.01ms-unsure 🔍
-2% - +2%
-13.74ms - +15.25ms
unsure 🔍
-1% - +2%
-12.28ms - +17.95ms
tip-of-tree
tip-of-tree
837.79ms - 857.11msunsure 🔍
-2% - +2%
-15.25ms - +13.74ms
-unsure 🔍
-1% - +2%
-12.24ms - +16.40ms
previous-release
previous-release
834.80ms - 855.94msunsure 🔍
-2% - +1%
-17.95ms - +12.28ms
unsure 🔍
-2% - +1%
-16.40ms - +12.24ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
925.62ms - 951.55ms-unsure 🔍
-2% - +1%
-20.91ms - +12.53ms
unsure 🔍
-2% - +2%
-16.60ms - +18.41ms
tip-of-tree
tip-of-tree
932.22ms - 953.33msunsure 🔍
-1% - +2%
-12.53ms - +20.91ms
-unsure 🔍
-1% - +2%
-10.70ms - +20.89ms
previous-release
previous-release
925.92ms - 949.44msunsure 🔍
-2% - +2%
-18.41ms - +16.60ms
unsure 🔍
-2% - +1%
-20.89ms - +10.70ms
-

tachometer-reporter-action v2 for Benchmarks

@justinfagnani
Copy link
Collaborator

@sorvell could we get this broken up into separate PRs per decorator?

@justinfagnani justinfagnani changed the title Adds decorators for @compute, @observe, and @listen [reactive-element] Adds decorators for @compute, @observe, and @listen May 12, 2021
* private _clickHandler(e: Event) => {
* this.clickedOn = e.target.localName;
* }
* @listen('keydown', window, {capture: true}))
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that the docs do not match the function definition.

listen('keydown', window, { capture: true })
vs
listen({ type: 'keydown', options: { capture: true }, target: window })

@robrez
Copy link

robrez commented Oct 7, 2021

Pretty excited about these APIs. Betting there was some internal discussions on your end.. was kind of wondering about multi-property observers?

@abdonrd
Copy link
Contributor

abdonrd commented Nov 26, 2021

@sorvell what is the status of this? Thanks!

@WickyNilliams
Copy link
Contributor

WickyNilliams commented Apr 27, 2022

Curious if there is still a plan to add these decorators? They would be very handy, and i expect many component libraries end up implementing these themselves.

I implemented an observe decorator in my design system today. I tried to adapt the one in this PR, but hit issues where this would be undefined inside the callback e.g.

@property()
@observe(() => {
  this.doSomething() // error `this` is undefined
})
foo: number = 0

I guess maybe the code is no longer compatible with Lit 2?

Since I couldn't workout a good approach to allow access to this with a lambda passed to the decorator, I settled on this pattern instead:

@property()
foo: number = 0

@observe("foo")
doSomething() {
}

This also works with @state, and I added a runtime check (dev only) to ensure the property name exists

@abdonrd
Copy link
Contributor

abdonrd commented May 26, 2022

Also interested on this!

@tehbiga
Copy link

tehbiga commented Jul 6, 2022

Sharing interest!
These are so far the major missing elements in Lit for me!

@a11delavar
Copy link
Contributor

If anyone is interested in these features (excluding @computed), check out the lit wrapper I created. It contains @eventListener and @updated decorators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[reactive-element] Consider adding additional decorators to support common user needs
8 participants