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

Better types for PropertyValues<T> #2482

Merged
merged 3 commits into from
Feb 5, 2022
Merged

Conversation

justinfagnani
Copy link
Collaborator

Fixes #2481

This is technically a breaking change in that:

class E extends LitElement {
  declare foo: number;
  update(changedProperties: PropertyValues<this>) {
    changedProperties.get('bar'); // Error
  }
}

This should be an ok breaking change as it catches a legitimate problem. The thing to look out for here is whether this causes unexpected breaks. I don't quite understand what the conditional type was doing here, but since one branch was never, removing that should't cause any new errors.

@changeset-bot
Copy link

changeset-bot bot commented Feb 2, 2022

🦋 Changeset detected

Latest commit: 9ee474a

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

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, 2022

📊 Tachometer Benchmark Results

Summary

nop-update

  • lit-html-kitchen-sink: unsure 🔍 -2% - +4% (-0.70ms - +1.22ms)
    this-change vs tip-of-tree

render

  • lit-element-list: unsure 🔍 -2% - +1% (-2.30ms - +1.34ms)
    this-change vs tip-of-tree
  • lit-html-kitchen-sink: unsure 🔍 -2% - +3% (-1.04ms - +1.13ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -3% - +5% (-0.37ms - +0.62ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -0% - +4% (-0.01ms - +2.60ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -1% - +2% (-0.77ms - +1.71ms)
    this-change vs tip-of-tree

update

  • lit-element-list: unsure 🔍 -1% - +0% (-14.69ms - +3.41ms)
    this-change vs tip-of-tree
  • lit-html-kitchen-sink: unsure 🔍 -3% - +3% (-3.25ms - +3.25ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -6% - +8% (-23.59ms - +28.75ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -3% - +2% (-4.52ms - +3.20ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -2% - +0% (-19.84ms - +4.87ms)
    this-change vs tip-of-tree

update-reflect

  • lit-element-list: unsure 🔍 -0% - +2% (-3.41ms - +19.95ms)
    this-change vs tip-of-tree
  • reactive-element-list: unsure 🔍 -0% - +2% (-3.93ms - +20.79ms)
    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
104.33ms - 106.80ms-unsure 🔍
-2% - +1%
-2.30ms - +1.34ms
faster ✔
22% - 24%
29.67ms - 33.39ms
tip-of-tree
tip-of-tree
104.70ms - 107.38msunsure 🔍
-1% - +2%
-1.34ms - +2.30ms
-faster ✔
21% - 24%
29.12ms - 32.99ms
previous-release
previous-release
135.70ms - 138.49msslower ❌
28% - 32%
29.67ms - 33.39ms
slower ❌
27% - 31%
29.12ms - 32.99ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
1007.00ms - 1020.94ms-unsure 🔍
-1% - +0%
-14.69ms - +3.41ms
faster ✔
6% - 8%
61.63ms - 82.25ms
tip-of-tree
tip-of-tree
1013.84ms - 1025.39msunsure 🔍
-0% - +1%
-3.41ms - +14.69ms
-faster ✔
5% - 7%
56.76ms - 75.84ms
previous-release
previous-release
1078.31ms - 1093.51msslower ❌
6% - 8%
61.63ms - 82.25ms
slower ❌
6% - 7%
56.76ms - 75.84ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
1105.27ms - 1123.42ms-unsure 🔍
-0% - +2%
-3.41ms - +19.95ms
faster ✔
3% - 5%
32.86ms - 56.12ms
tip-of-tree
tip-of-tree
1098.72ms - 1113.42msunsure 🔍
-2% - +0%
-19.95ms - +3.41ms
-faster ✔
4% - 5%
42.42ms - 63.10ms
previous-release
previous-release
1151.56ms - 1166.11msslower ❌
3% - 5%
32.86ms - 56.12ms
slower ❌
4% - 6%
42.42ms - 63.10ms
-
lit-html-kitchen-sink

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
44.65ms - 45.60ms-unsure 🔍
-2% - +3%
-1.04ms - +1.13ms
faster ✔
10% - 14%
5.20ms - 7.15ms
tip-of-tree
tip-of-tree
44.10ms - 46.05msunsure 🔍
-3% - +2%
-1.13ms - +1.04ms
-faster ✔
10% - 15%
4.93ms - 7.52ms
previous-release
previous-release
50.44ms - 52.16msslower ❌
11% - 16%
5.20ms - 7.15ms
slower ❌
11% - 17%
4.93ms - 7.52ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
116.40ms - 120.61ms-unsure 🔍
-3% - +3%
-3.25ms - +3.25ms
unsure 🔍
-7% - +0%
-8.68ms - +0.19ms
tip-of-tree
tip-of-tree
116.03ms - 120.98msunsure 🔍
-3% - +3%
-3.25ms - +3.25ms
-unsure 🔍
-7% - +0%
-8.87ms - +0.38ms
previous-release
previous-release
118.85ms - 126.66msunsure 🔍
-0% - +7%
-0.19ms - +8.68ms
unsure 🔍
-0% - +8%
-0.38ms - +8.87ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
33.05ms - 34.51ms-unsure 🔍
-2% - +4%
-0.70ms - +1.22ms
faster ✔
8% - 16%
3.05ms - 6.20ms
tip-of-tree
tip-of-tree
32.90ms - 34.14msunsure 🔍
-4% - +2%
-1.22ms - +0.70ms
-faster ✔
9% - 16%
3.36ms - 6.42ms
previous-release
previous-release
37.01ms - 39.80msslower ❌
9% - 19%
3.05ms - 6.20ms
slower ❌
10% - 19%
3.36ms - 6.42ms
-
lit-html-repeat

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
13.48ms - 14.24ms-unsure 🔍
-3% - +5%
-0.37ms - +0.62ms
faster ✔
9% - 15%
1.41ms - 2.44ms
tip-of-tree
tip-of-tree
13.42ms - 14.05msunsure 🔍
-4% - +3%
-0.62ms - +0.37ms
-faster ✔
10% - 16%
1.58ms - 2.52ms
previous-release
previous-release
15.43ms - 16.13msslower ❌
10% - 18%
1.41ms - 2.44ms
slower ❌
11% - 19%
1.58ms - 2.52ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
355.56ms - 392.85ms-unsure 🔍
-6% - +8%
-23.59ms - +28.75ms
faster ✔
26% - 35%
136.72ms - 193.36ms
tip-of-tree
tip-of-tree
353.25ms - 389.99msunsure 🔍
-8% - +6%
-28.75ms - +23.59ms
-faster ✔
27% - 35%
139.49ms - 195.76ms
previous-release
previous-release
517.93ms - 560.56msslower ❌
35% - 53%
136.72ms - 193.36ms
slower ❌
36% - 54%
139.49ms - 195.76ms
-
lit-html-template-heavy

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
69.90ms - 71.83ms-unsure 🔍
-0% - +4%
-0.01ms - +2.60ms
faster ✔
12% - 15%
9.48ms - 12.37ms
tip-of-tree
tip-of-tree
68.69ms - 70.45msunsure 🔍
-4% - -0%
-2.60ms - +0.01ms
-faster ✔
13% - 16%
10.83ms - 13.62ms
previous-release
previous-release
80.71ms - 82.87msslower ❌
13% - 18%
9.48ms - 12.37ms
slower ❌
15% - 20%
10.83ms - 13.62ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
155.77ms - 161.56ms-unsure 🔍
-3% - +2%
-4.52ms - +3.20ms
faster ✔
10% - 14%
17.67ms - 24.94ms
tip-of-tree
tip-of-tree
156.77ms - 161.87msunsure 🔍
-2% - +3%
-3.20ms - +4.52ms
-faster ✔
10% - 13%
17.28ms - 24.02ms
previous-release
previous-release
177.78ms - 182.17msslower ❌
11% - 16%
17.67ms - 24.94ms
slower ❌
11% - 15%
17.28ms - 24.02ms
-
reactive-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
74.86ms - 76.44ms-unsure 🔍
-1% - +2%
-0.77ms - +1.71ms
unsure 🔍
-2% - +2%
-1.35ms - +1.35ms
tip-of-tree
tip-of-tree
74.22ms - 76.13msunsure 🔍
-2% - +1%
-1.71ms - +0.77ms
-unsure 🔍
-3% - +1%
-1.92ms - +0.98ms
previous-release
previous-release
74.56ms - 76.74msunsure 🔍
-2% - +2%
-1.35ms - +1.35ms
unsure 🔍
-1% - +3%
-0.98ms - +1.92ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
1039.78ms - 1055.53ms-unsure 🔍
-2% - +0%
-19.84ms - +4.87ms
unsure 🔍
-0% - +2%
-4.87ms - +20.21ms
tip-of-tree
tip-of-tree
1045.62ms - 1064.65msunsure 🔍
-0% - +2%
-4.87ms - +19.84ms
-slower ❌
0% - 3%
1.52ms - 28.78ms
previous-release
previous-release
1030.22ms - 1049.74msunsure 🔍
-2% - +0%
-20.21ms - +4.87ms
faster ✔
0% - 3%
1.52ms - 28.78ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
1151.92ms - 1170.29ms-unsure 🔍
-0% - +2%
-3.93ms - +20.79ms
unsure 🔍
-0% - +2%
-4.56ms - +21.48ms
tip-of-tree
tip-of-tree
1144.41ms - 1160.95msunsure 🔍
-2% - +0%
-20.79ms - +3.93ms
-unsure 🔍
-1% - +1%
-12.36ms - +12.43ms
previous-release
previous-release
1143.41ms - 1161.88msunsure 🔍
-2% - +0%
-21.48ms - +4.56ms
unsure 🔍
-1% - +1%
-12.43ms - +12.36ms
-

tachometer-reporter-action v2 for Benchmarks

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.

PropertyValues type does not work with keyof this
2 participants