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

[labs/context] Add options objects to Context controller constructors #3632

Merged
merged 3 commits into from Feb 4, 2023

Conversation

justinfagnani
Copy link
Collaborator

This makes the controllers consistent with the decorators and options like subscribe, callback, and initialValue easier to read in source.

@changeset-bot
Copy link

changeset-bot bot commented Feb 2, 2023

🦋 Changeset detected

Latest commit: 73434f8

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

This PR includes changesets to release 1 package
Name Type
@lit-labs/context Minor

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
Contributor

github-actions bot commented Feb 2, 2023

📊 Tachometer Benchmark Results

Summary

nop-update

  • lit-html-kitchen-sink: unsure 🔍 -4% - +12% (-1.04ms - +2.94ms)
    this-change vs tip-of-tree

render

  • lit-element-list: 77.06ms - 80.02ms
  • lit-html-kitchen-sink: unsure 🔍 -5% - +3% (-1.52ms - +0.95ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -4% - +6% (-0.47ms - +0.73ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -3% - +2% (-1.53ms - +1.05ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -3% - +1% (-1.41ms - +0.39ms)
    this-change vs tip-of-tree

update

  • lit-element-list: 811.37ms - 819.59ms
  • lit-html-kitchen-sink: unsure 🔍 -6% - +5% (-5.11ms - +4.27ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -8% - +15% (-27.20ms - +48.66ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -0% - +2% (-0.61ms - +3.04ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -1% - +1% (-4.77ms - +7.57ms)
    this-change vs tip-of-tree

update-reflect

  • lit-element-list: 811.83ms - 819.95ms
  • reactive-element-list: unsure 🔍 -0% - +1% (-4.06ms - +9.26ms)
    this-change vs tip-of-tree

Results

lit-element-list

render

VersionAvg timevs
77.06ms - 80.02ms-

update

VersionAvg timevs
811.37ms - 819.59ms-

update-reflect

VersionAvg timevs
811.83ms - 819.95ms-
lit-html-kitchen-sink

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
31.72ms - 33.03ms-unsure 🔍
-5% - +3%
-1.52ms - +0.95ms
unsure 🔍
-4% - +2%
-1.33ms - +0.69ms
tip-of-tree
tip-of-tree
31.61ms - 33.71msunsure 🔍
-3% - +5%
-0.95ms - +1.52ms
-unsure 🔍
-4% - +4%
-1.34ms - +1.26ms
previous-release
previous-release
31.94ms - 33.46msunsure 🔍
-2% - +4%
-0.69ms - +1.33ms
unsure 🔍
-4% - +4%
-1.26ms - +1.34ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
83.98ms - 88.91ms-unsure 🔍
-6% - +5%
-5.11ms - +4.27ms
slower ❌
1% - 11%
1.07ms - 8.64ms
tip-of-tree
tip-of-tree
82.87ms - 90.85msunsure 🔍
-5% - +6%
-4.27ms - +5.11ms
-slower ❌
0% - 13%
0.36ms - 10.19ms
previous-release
previous-release
78.72ms - 84.46msfaster ✔
1% - 10%
1.07ms - 8.64ms
faster ✔
1% - 12%
0.36ms - 10.19ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
23.98ms - 26.47ms-unsure 🔍
-4% - +12%
-1.04ms - +2.94ms
unsure 🔍
-6% - +11%
-1.47ms - +2.57ms
tip-of-tree
tip-of-tree
22.72ms - 25.82msunsure 🔍
-12% - +4%
-2.94ms - +1.04ms
-unsure 🔍
-11% - +7%
-2.62ms - +1.82ms
previous-release
previous-release
23.09ms - 26.26msunsure 🔍
-10% - +6%
-2.57ms - +1.47ms
unsure 🔍
-8% - +11%
-1.82ms - +2.62ms
-
lit-html-repeat

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
11.59ms - 12.47ms-unsure 🔍
-4% - +6%
-0.47ms - +0.73ms
unsure 🔍
-8% - +1%
-1.04ms - +0.10ms
tip-of-tree
tip-of-tree
11.50ms - 12.30msunsure 🔍
-6% - +4%
-0.73ms - +0.47ms
-faster ✔
1% - 9%
0.07ms - 1.13ms
previous-release
previous-release
12.14ms - 12.86msunsure 🔍
-1% - +9%
-0.10ms - +1.04ms
slower ❌
0% - 10%
0.07ms - 1.13ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
317.19ms - 374.13ms-unsure 🔍
-8% - +15%
-27.20ms - +48.66ms
unsure 🔍
-7% - +14%
-21.67ms - +47.52ms
tip-of-tree
tip-of-tree
309.87ms - 360.00msunsure 🔍
-14% - +8%
-48.66ms - +27.20ms
-unsure 🔍
-9% - +10%
-29.65ms - +34.04ms
previous-release
previous-release
313.09ms - 352.39msunsure 🔍
-13% - +6%
-47.52ms - +21.67ms
unsure 🔍
-10% - +9%
-34.04ms - +29.65ms
-
lit-html-template-heavy

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
54.55ms - 56.52ms-unsure 🔍
-3% - +2%
-1.53ms - +1.05ms
unsure 🔍
-3% - +2%
-1.60ms - +1.20ms
tip-of-tree
tip-of-tree
54.94ms - 56.61msunsure 🔍
-2% - +3%
-1.05ms - +1.53ms
-unsure 🔍
-2% - +2%
-1.26ms - +1.34ms
previous-release
previous-release
54.74ms - 56.73msunsure 🔍
-2% - +3%
-1.20ms - +1.60ms
unsure 🔍
-2% - +2%
-1.34ms - +1.26ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
125.56ms - 128.11ms-unsure 🔍
-0% - +2%
-0.61ms - +3.04ms
unsure 🔍
-1% - +1%
-1.80ms - +1.49ms
tip-of-tree
tip-of-tree
124.31ms - 126.93msunsure 🔍
-2% - +0%
-3.04ms - +0.61ms
-unsure 🔍
-2% - +0%
-3.04ms - +0.31ms
previous-release
previous-release
125.94ms - 128.03msunsure 🔍
-1% - +1%
-1.49ms - +1.80ms
unsure 🔍
-0% - +2%
-0.31ms - +3.04ms
-
reactive-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
52.23ms - 53.47ms-unsure 🔍
-3% - +1%
-1.41ms - +0.39ms
unsure 🔍
-4% - -0%
-2.10ms - +0.01ms
tip-of-tree
tip-of-tree
52.71ms - 54.02msunsure 🔍
-1% - +3%
-0.39ms - +1.41ms
-unsure 🔍
-3% - +1%
-1.61ms - +0.55ms
previous-release
previous-release
53.04ms - 54.76msunsure 🔍
-0% - +4%
-0.01ms - +2.10ms
unsure 🔍
-1% - +3%
-0.55ms - +1.61ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
854.67ms - 863.02ms-unsure 🔍
-1% - +1%
-4.77ms - +7.57ms
unsure 🔍
-1% - +1%
-6.57ms - +6.27ms
tip-of-tree
tip-of-tree
852.90ms - 861.99msunsure 🔍
-1% - +1%
-7.57ms - +4.77ms
-unsure 🔍
-1% - +1%
-8.22ms - +5.12ms
previous-release
previous-release
854.12ms - 863.87msunsure 🔍
-1% - +1%
-6.27ms - +6.57ms
unsure 🔍
-1% - +1%
-5.12ms - +8.22ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
866.72ms - 876.08ms-unsure 🔍
-0% - +1%
-4.06ms - +9.26ms
unsure 🔍
-1% - +1%
-7.12ms - +6.65ms
tip-of-tree
tip-of-tree
864.06ms - 873.54msunsure 🔍
-1% - +0%
-9.26ms - +4.06ms
-unsure 🔍
-1% - +0%
-9.77ms - +4.09ms
previous-release
previous-release
866.58ms - 876.69msunsure 🔍
-1% - +1%
-6.65ms - +7.12ms
unsure 🔍
-0% - +1%
-4.09ms - +9.77ms
-

tachometer-reporter-action v2 for Benchmarks

Copy link
Member

@sorvell sorvell left a comment

Choose a reason for hiding this comment

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

LGTM, just one comment to consider option of just making this breaking rather than deprecating.

) {
this.host = host;
// This is a potentially fragile duck-type. It means a context object can't
Copy link
Member

Choose a reason for hiding this comment

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

Consider optional: make this a breaking change rather than exposing the fragility?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I already have some deprecations checked in, and there's one more rename coming down the pike, so I thought I'd deprecate first, then remove them all at once.

@justinfagnani justinfagnani merged commit 2fa009f into main Feb 4, 2023
@justinfagnani justinfagnani deleted the context-consumer-options branch February 4, 2023 16:16
@lit-robot lit-robot mentioned this pull request Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants