Skip to content

[reactive-element] Make decorators accept ReactiveElement from elsewhere#3917

Merged
augustjk merged 3 commits intomainfrom
reactive-element-interface
May 24, 2023
Merged

[reactive-element] Make decorators accept ReactiveElement from elsewhere#3917
augustjk merged 3 commits intomainfrom
reactive-element-interface

Conversation

@augustjk
Copy link
Copy Markdown
Member

@augustjk augustjk commented May 24, 2023

Fixes #3241

This specifically affected decorators made with decorateProperty which also includes the labs/context decorators. This is a workaround until something like microsoft/TypeScript#52119 is implemented.

Manually tested by creating a project with multiple versions of reactive-element installed, and replacing the nested version with one built with these changes applied.

@augustjk augustjk requested review from justinfagnani and rictic May 24, 2023 00:48
@augustjk augustjk requested a review from kevinpschaaf as a code owner May 24, 2023 00:48
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented May 24, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 22392d2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@lit/reactive-element Patch
lit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 24, 2023 •
edited
Loading

📊 Tachometer Benchmark Results

Summary

nop-update

  • lit-html-kitchen-sink: unsure 🔍 -1% - +15% (-0.15ms - +2.67ms)
    this-change vs tip-of-tree

render

  • lit-element-list: 86.21ms - 90.67ms
  • lit-html-kitchen-sink: unsure 🔍 -5% - +8% (-1.87ms - +2.98ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -10% - +5% (-1.18ms - +0.59ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -2% - +5% (-1.40ms - +2.82ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -3% - +4% (-1.50ms - +2.32ms)
    this-change vs tip-of-tree

update

  • lit-element-list: 938.18ms - 957.10ms
  • lit-html-kitchen-sink: unsure 🔍 -6% - +2% (-5.61ms - +1.90ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -9% - +9% (-28.93ms - +28.60ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -1% - +3% (-1.66ms - +3.99ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -1% - +2% (-9.34ms - +19.97ms)
    this-change vs tip-of-tree

update-reflect

  • lit-element-list: 906.30ms - 920.61ms
  • reactive-element-list: unsure 🔍 -2% - +1% (-14.17ms - +12.53ms)
    this-change vs tip-of-tree

Results

lit-element-list

render

VersionAvg timevs
86.21ms - 90.67ms-

update

VersionAvg timevs
938.18ms - 957.10ms-

update-reflect

VersionAvg timevs
906.30ms - 920.61ms-
lit-html-kitchen-sink

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
35.91ms - 39.74ms-unsure 🔍
-5% - +8%
-1.87ms - +2.98ms		unsure 🔍
-8% - +6%
-2.88ms - +2.33ms
tip-of-tree
tip-of-tree		35.78ms - 38.76msunsure 🔍
-8% - +5%
-2.98ms - +1.87ms		-unsure 🔍
-8% - +4%
-3.13ms - +1.48ms
previous-release
previous-release		36.33ms - 39.86msunsure 🔍
-6% - +8%
-2.33ms - +2.88ms		unsure 🔍
-4% - +8%
-1.48ms - +3.13ms		-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
85.99ms - 91.49ms-unsure 🔍
-6% - +2%
-5.61ms - +1.90ms		unsure 🔍
-10% - +4%
-9.56ms - +4.12ms
tip-of-tree
tip-of-tree		88.04ms - 93.15msunsure 🔍
-2% - +6%
-1.90ms - +5.61ms		-unsure 🔍
-8% - +6%
-7.63ms - +5.90ms
previous-release
previous-release		85.20ms - 97.73msunsure 🔍
-5% - +11%
-4.12ms - +9.56ms		unsure 🔍
-7% - +8%
-5.90ms - +7.63ms		-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
17.44ms - 19.93ms-unsure 🔍
-1% - +15%
-0.15ms - +2.67ms		unsure 🔍
-10% - +9%
-1.80ms - +1.63ms
tip-of-tree
tip-of-tree		16.77ms - 18.09msunsure 🔍
-14% - +0%
-2.67ms - +0.15ms		-unsure 🔍
-14% - -0%
-2.70ms - +0.00ms
previous-release
previous-release		17.60ms - 19.95msunsure 🔍
-9% - +10%
-1.63ms - +1.80ms		unsure 🔍
-0% - +16%
-0.00ms - +2.70ms		-
lit-html-repeat

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
10.92ms - 12.30ms-unsure 🔍
-10% - +5%
-1.18ms - +0.59ms		unsure 🔍
-13% - +7%
-1.57ms - +0.88ms
tip-of-tree
tip-of-tree		11.35ms - 12.47msunsure 🔍
-5% - +10%
-0.59ms - +1.18ms		-unsure 🔍
-10% - +9%
-1.21ms - +1.11ms
previous-release
previous-release		10.95ms - 12.97msunsure 🔍
-8% - +14%
-0.88ms - +1.57ms		unsure 🔍
-9% - +10%
-1.11ms - +1.21ms		-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
289.98ms - 330.32ms-unsure 🔍
-9% - +9%
-28.93ms - +28.60ms		unsure 🔍
-10% - +9%
-31.94ms - +28.44ms
tip-of-tree
tip-of-tree		289.81ms - 330.82msunsure 🔍
-9% - +9%
-28.60ms - +28.93ms		-unsure 🔍
-10% - +9%
-32.00ms - +28.83ms
previous-release
previous-release		289.43ms - 334.36msunsure 🔍
-9% - +10%
-28.44ms - +31.94ms		unsure 🔍
-9% - +10%
-28.83ms - +32.00ms		-
lit-html-template-heavy

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
59.38ms - 62.72ms-unsure 🔍
-2% - +5%
-1.40ms - +2.82ms		unsure 🔍
-4% - +2%
-2.77ms - +1.54ms
tip-of-tree
tip-of-tree		59.05ms - 61.62msunsure 🔍
-5% - +2%
-2.82ms - +1.40ms		-unsure 🔍
-5% - +1%
-3.20ms - +0.55ms
previous-release
previous-release		60.30ms - 63.03msunsure 🔍
-3% - +5%
-1.54ms - +2.77ms		unsure 🔍
-1% - +5%
-0.55ms - +3.20ms		-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
115.05ms - 118.76ms-unsure 🔍
-1% - +3%
-1.66ms - +3.99ms		unsure 🔍
-3% - +2%
-3.33ms - +2.36ms
tip-of-tree
tip-of-tree		113.61ms - 117.87msunsure 🔍
-3% - +1%
-3.99ms - +1.66ms		-unsure 🔍
-4% - +1%
-4.69ms - +1.38ms
previous-release
previous-release		115.24ms - 119.55msunsure 🔍
-2% - +3%
-2.36ms - +3.33ms		unsure 🔍
-1% - +4%
-1.38ms - +4.69ms		-
reactive-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
58.23ms - 60.84ms-unsure 🔍
-3% - +4%
-1.50ms - +2.32ms		unsure 🔍
-6% - +2%
-3.53ms - +1.02ms
tip-of-tree
tip-of-tree		57.73ms - 60.53msunsure 🔍
-4% - +3%
-2.32ms - +1.50ms		-unsure 🔍
-7% - +1%
-3.99ms - +0.67ms
previous-release
previous-release		58.93ms - 62.65msunsure 🔍
-2% - +6%
-1.02ms - +3.53ms		unsure 🔍
-1% - +7%
-0.67ms - +3.99ms		-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
931.91ms - 953.60ms-unsure 🔍
-1% - +2%
-9.34ms - +19.97ms		unsure 🔍
-1% - +2%
-11.24ms - +18.21ms
tip-of-tree
tip-of-tree		927.58ms - 947.29msunsure 🔍
-2% - +1%
-19.97ms - +9.34ms		-unsure 🔍
-2% - +1%
-15.84ms - +12.18ms
previous-release
previous-release		929.31ms - 949.22msunsure 🔍
-2% - +1%
-18.21ms - +11.24ms		unsure 🔍
-1% - +2%
-12.18ms - +15.84ms		-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
924.57ms - 939.81ms-unsure 🔍
-2% - +1%
-14.17ms - +12.53ms		unsure 🔍
-1% - +1%
-8.79ms - +13.48ms
tip-of-tree
tip-of-tree		922.05ms - 943.97msunsure 🔍
-1% - +2%
-12.53ms - +14.17ms		-unsure 🔍
-1% - +2%
-10.48ms - +16.81ms
previous-release
previous-release		921.72ms - 937.96msunsure 🔍
-1% - +1%
-13.48ms - +8.79ms		unsure 🔍
-2% - +1%
-16.81ms - +10.48ms		-

tachometer-reporter-action v2 for Benchmarks

rictic
rictic approved these changes May 24, 2023
View reviewed changes
rictic
rictic approved these changes May 24, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@rictic rictic left a comment

Choose a reason for hiding this comment

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

Great!

justinfagnani
justinfagnani approved these changes May 24, 2023
View reviewed changes
packages/reactive-element/src/decorators/base.ts
}) =>
(
protoOrDescriptor: ReactiveElement | ClassElement,
protoOrDescriptor: Interface<ReactiveElement> | ClassElement,
Copy link
Copy Markdown
Collaborator

@justinfagnani justinfagnani May 24, 2023

Choose a reason for hiding this comment

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

Just a note: we should have use function overload signatures here.

@augustjk augustjk merged commit f6387e3 into main May 24, 2023
@augustjk augustjk deleted the reactive-element-interface branch May 24, 2023 17:24
This was referenced May 24, 2023
Closed
Closed
@lit-robot lit-robot mentioned this pull request Jun 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rictic rictic rictic approved these changes

@justinfagnani justinfagnani justinfagnani approved these changes

@kevinpschaaf kevinpschaaf Awaiting requested review from kevinpschaaf kevinpschaaf is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

@augustjk @rictic @justinfagnani