[lit-html] Don't remove attributes with noChange on first render #1922

justinfagnani · 2021-05-21T17:32:33Z

Fixes #1920

In attribute expressions noChange is now treated like undefined and null on first render, instead of like nothing. Previously the attribute was removed since we initialized attribute part values to nothing.

github-actions · 2021-05-21T17:32:59Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -1% - +1% (-0.23ms - +0.20ms)
^{this-change vs tip-of-tree}

render

lit-element-list: unsure 🔍 -3% - +4% (-2.62ms - +3.19ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -2% - +1% (-0.70ms - +0.35ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -1% - +2% (-0.16ms - +0.21ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -1% - +1% (-0.52ms - +0.81ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -0% - +2% (-0.22ms - +1.15ms)
^{this-change vs tip-of-tree}

update

lit-element-list: unsure 🔍 -1% - +2% (-8.28ms - +13.53ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -3% - +1% (-2.88ms - +0.95ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -1% - +0% (-3.93ms - +1.56ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -0% - +2% (-0.62ms - +2.52ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +0% (-8.75ms - +2.57ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: unsure 🔍 -1% - +2% (-9.68ms - +13.40ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +1% (-7.03ms - +6.80ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 92.0.4515.131
Sample size: 50
Built by: Benchmarks #1857
Commit: 4532729

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	88.38ms - 92.65ms	-	unsure 🔍 -3% - +4% -2.62ms - +3.19ms	faster ✔ 15% - 20% 16.85ms - 22.55ms
tip-of-tree tip-of-tree	88.26ms - 92.20ms	unsure 🔍 -4% - +3% -3.19ms - +2.62ms	-	faster ✔ 16% - 20% 17.26ms - 22.72ms
previous-release previous-release	108.33ms - 112.11ms	slower ❌ 18% - 25% 16.85ms - 22.55ms	slower ❌ 19% - 26% 17.26ms - 22.72ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	745.65ms - 762.22ms	-	unsure 🔍 -1% - +2% -8.28ms - +13.53ms	faster ✔ 8% - 11% 68.59ms - 92.76ms
tip-of-tree tip-of-tree	744.22ms - 758.40ms	unsure 🔍 -2% - +1% -13.53ms - +8.28ms	-	faster ✔ 9% - 11% 72.00ms - 94.60ms
previous-release previous-release	825.81ms - 843.41ms	slower ❌ 9% - 12% 68.59ms - 92.76ms	slower ❌ 10% - 13% 72.00ms - 94.60ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	813.14ms - 830.19ms	-	unsure 🔍 -1% - +2% -9.68ms - +13.40ms	faster ✔ 3% - 6% 25.48ms - 47.87ms
tip-of-tree tip-of-tree	812.02ms - 827.58ms	unsure 🔍 -2% - +1% -13.40ms - +9.68ms	-	faster ✔ 3% - 6% 27.89ms - 49.17ms
previous-release previous-release	851.08ms - 865.60ms	slower ❌ 3% - 6% 25.48ms - 47.87ms	slower ❌ 3% - 6% 27.89ms - 49.17ms	-

lit-html-kitchen-sink

Browser: chrome-headless 92.0.4515.131
Sample size: 30
Built by: Benchmarks #1857
Commit: 4532729

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	35.37ms - 35.61ms	-	unsure 🔍 -2% - +1% -0.70ms - +0.35ms	faster ✔ 16% - 17% 6.89ms - 7.35ms
tip-of-tree tip-of-tree	35.15ms - 36.17ms	unsure 🔍 -1% - +2% -0.35ms - +0.70ms	-	faster ✔ 15% - 18% 6.41ms - 7.49ms
previous-release previous-release	42.42ms - 42.81ms	slower ❌ 19% - 21% 6.89ms - 7.35ms	slower ❌ 18% - 21% 6.41ms - 7.49ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	87.70ms - 90.18ms	-	unsure 🔍 -3% - +1% -2.88ms - +0.95ms	faster ✔ 2% - 6% 1.59ms - 5.64ms
tip-of-tree tip-of-tree	88.44ms - 91.37ms	unsure 🔍 -1% - +3% -0.95ms - +2.88ms	-	faster ✔ 1% - 5% 0.48ms - 4.81ms
previous-release previous-release	90.95ms - 94.15ms	slower ❌ 2% - 6% 1.59ms - 5.64ms	slower ❌ 1% - 5% 0.48ms - 4.81ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	29.81ms - 30.08ms	-	unsure 🔍 -1% - +1% -0.23ms - +0.20ms	faster ✔ 1% - 4% 0.26ms - 1.39ms
tip-of-tree tip-of-tree	29.79ms - 30.12ms	unsure 🔍 -1% - +1% -0.20ms - +0.23ms	-	faster ✔ 1% - 4% 0.24ms - 1.39ms
previous-release previous-release	30.22ms - 31.32ms	slower ❌ 1% - 5% 0.26ms - 1.39ms	slower ❌ 1% - 5% 0.24ms - 1.39ms	-

lit-html-repeat

Browser: chrome-headless 92.0.4515.131
Sample size: 30
Built by: Benchmarks #1857
Commit: 4532729

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	12.20ms - 12.45ms	-	unsure 🔍 -1% - +2% -0.16ms - +0.21ms	faster ✔ 8% - 10% 1.14ms - 1.42ms
tip-of-tree tip-of-tree	12.16ms - 12.43ms	unsure 🔍 -2% - +1% -0.21ms - +0.16ms	-	faster ✔ 9% - 11% 1.16ms - 1.45ms
previous-release previous-release	13.55ms - 13.66ms	slower ❌ 9% - 12% 1.14ms - 1.42ms	slower ❌ 9% - 12% 1.16ms - 1.45ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	366.83ms - 370.61ms	-	unsure 🔍 -1% - +0% -3.93ms - +1.56ms	faster ✔ 28% - 29% 142.75ms - 151.69ms
tip-of-tree tip-of-tree	367.92ms - 371.90ms	unsure 🔍 -0% - +1% -1.56ms - +3.93ms	-	faster ✔ 28% - 29% 141.52ms - 150.55ms
previous-release previous-release	511.89ms - 519.99ms	slower ❌ 39% - 41% 142.75ms - 151.69ms	slower ❌ 38% - 41% 141.52ms - 150.55ms	-

lit-html-template-heavy

Browser: chrome-headless 92.0.4515.131
Sample size: 50
Built by: Benchmarks #1857
Commit: 4532729

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	59.91ms - 60.76ms	-	unsure 🔍 -1% - +1% -0.52ms - +0.81ms	faster ✔ 16% - 19% 11.25ms - 13.68ms
tip-of-tree tip-of-tree	59.68ms - 60.70ms	unsure 🔍 -1% - +1% -0.81ms - +0.52ms	-	faster ✔ 16% - 19% 11.36ms - 13.86ms
previous-release previous-release	71.66ms - 73.94ms	slower ❌ 19% - 23% 11.25ms - 13.68ms	slower ❌ 19% - 23% 11.36ms - 13.86ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	129.18ms - 131.45ms	-	unsure 🔍 -0% - +2% -0.62ms - +2.52ms	faster ✔ 10% - 12% 14.87ms - 17.86ms
tip-of-tree tip-of-tree	128.28ms - 130.46ms	unsure 🔍 -2% - +0% -2.52ms - +0.62ms	-	faster ✔ 11% - 13% 15.85ms - 18.77ms
previous-release previous-release	145.70ms - 147.66ms	slower ❌ 11% - 14% 14.87ms - 17.86ms	slower ❌ 12% - 15% 15.85ms - 18.77ms	-

reactive-element-list

Browser: chrome-headless 92.0.4515.131
Sample size: 50
Built by: Benchmarks #1857
Commit: 4532729

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	60.60ms - 61.35ms	-	unsure 🔍 -0% - +2% -0.22ms - +1.15ms	unsure 🔍 -1% - +2% -0.34ms - +0.93ms
tip-of-tree tip-of-tree	59.94ms - 61.08ms	unsure 🔍 -2% - +0% -1.15ms - +0.22ms	-	unsure 🔍 -2% - +1% -0.94ms - +0.60ms
previous-release previous-release	60.17ms - 61.20ms	unsure 🔍 -2% - +1% -0.93ms - +0.34ms	unsure 🔍 -1% - +2% -0.60ms - +0.94ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	746.44ms - 754.47ms	-	unsure 🔍 -1% - +0% -8.75ms - +2.57ms	unsure 🔍 -1% - +1% -3.88ms - +6.51ms
tip-of-tree tip-of-tree	749.55ms - 757.54ms	unsure 🔍 -0% - +1% -2.57ms - +8.75ms	-	unsure 🔍 -0% - +1% -0.78ms - +9.58ms
previous-release previous-release	745.84ms - 752.44ms	unsure 🔍 -1% - +1% -6.51ms - +3.88ms	unsure 🔍 -1% - +0% -9.58ms - +0.78ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	854.88ms - 863.71ms	-	unsure 🔍 -1% - +1% -7.03ms - +6.80ms	unsure 🔍 -0% - +1% -4.11ms - +7.91ms
tip-of-tree tip-of-tree	854.09ms - 864.73ms	unsure 🔍 -1% - +1% -6.80ms - +7.03ms	-	unsure 🔍 -1% - +1% -4.69ms - +8.71ms
previous-release previous-release	853.33ms - 861.47ms	unsure 🔍 -1% - +0% -7.91ms - +4.11ms	unsure 🔍 -1% - +1% -8.71ms - +4.69ms	-

_{tachometer-reporter-action v2 for Benchmarks}

changeset-bot · 2021-08-27T19:27:13Z

🦋 Changeset detected

Latest commit: 945e349

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

sorvell

LGTM, just a couple of minor suggestions to improve the tests.

packages/lit-html/src/test/lit-html_test.ts

kevinpschaaf

LGTM

Don't remove attributes with noChange on first render

02c94ac

justinfagnani requested a review from kevinpschaaf May 21, 2021 17:32

google-cla bot added the cla: yes label May 21, 2021

WIP

ef6918d

justinfagnani mentioned this pull request May 25, 2021

Attribute content is cleared entirely if an expression in an AttributePart evaluates to noChange on first render #1920

Closed

justinfagnani changed the title ~~Don't remove attributes with noChange on first render~~ [lit-html] Don't remove attributes with noChange on first render May 25, 2021

justinfagnani added 2 commits May 25, 2021 11:32

TEST_CONFIG

6918d50

TEST_CONFIG

4f49b61

kevinpschaaf added this to the Lit RC.next milestone Aug 20, 2021

Merge branch 'main' into no-change

43c883f

justinfagnani added 2 commits August 31, 2021 17:06

Simpler fix for interpolated bindings only

6b30a25

Add changeset

e4d3357

justinfagnani requested a review from sorvell September 1, 2021 00:11

justinfagnani added 3 commits August 31, 2021 18:04

Merge branch 'main' into no-change

482d1c9

Undo some things

fc32c31

Revert test config changes

aceb66a

sorvell approved these changes Sep 1, 2021

View reviewed changes

packages/lit-html/src/test/lit-html_test.ts Outdated Show resolved Hide resolved

packages/lit-html/src/test/lit-html_test.ts Show resolved Hide resolved

justinfagnani added 2 commits September 1, 2021 16:29

Merge branch 'main' into no-change

d731b99

Clean up tests

fe1f9c4

kevinpschaaf approved these changes Sep 2, 2021

View reviewed changes

justinfagnani added 2 commits September 1, 2021 17:24

Fix for interpolated empty strings

cdcdcf9

Update until() tests

945e349

justinfagnani merged commit 8189f09 into main Sep 2, 2021

justinfagnani deleted the no-change branch September 2, 2021 01:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lit-html] Don't remove attributes with noChange on first render #1922

[lit-html] Don't remove attributes with noChange on first render #1922

Uh oh!

justinfagnani commented May 21, 2021

github-actions bot commented May 21, 2021 •

edited

Loading

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

render

update

update-reflect

changeset-bot bot commented Aug 27, 2021 •

edited

Loading

sorvell left a comment

Uh oh!

Uh oh!

kevinpschaaf left a comment

Labels

4 participants

[lit-html] Don't remove attributes with noChange on first render #1922

[lit-html] Don't remove attributes with noChange on first render #1922

Uh oh!

Conversation

justinfagnani commented May 21, 2021

github-actions bot commented May 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

render

update

update-reflect

changeset-bot bot commented Aug 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

sorvell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kevinpschaaf left a comment

Choose a reason for hiding this comment

Labels

4 participants

github-actions bot commented May 21, 2021 •

edited

Loading

changeset-bot bot commented Aug 27, 2021 •

edited

Loading