Skip to content

async-replace correctly re-renders when value is unchanged#4409

Merged
AndrewJakubowicz merged 3 commits intolit:mainfrom
sorvell:async-replace-re-render
Nov 21, 2023
Merged

async-replace correctly re-renders when value is unchanged#4409
AndrewJakubowicz merged 3 commits intolit:mainfrom
sorvell:async-replace-re-render

Conversation

@sorvell
Copy link
Copy Markdown
Member

@sorvell sorvell commented Nov 20, 2023

Fixes #4408.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Nov 20, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 667a341

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

This PR includes changesets to release 2 packages
Name Type
lit-html 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 Nov 20, 2023 •
edited
Loading

📊 Tachometer Benchmark Results

Summary

nop-update

  • this-change, tip-of-tree, previous-release: unsure 🔍 -5% - +3% (-0.64ms - +0.32ms)
    this-change vs tip-of-tree

render

  • this-change: 47.24ms - 49.38ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -5% - +2% (-0.95ms - +0.30ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +4% (-0.21ms - +1.27ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +3% (-0.32ms - +1.14ms)
    this-change vs tip-of-tree

update

  • this-change: 501.12ms - 505.33ms
  • this-change, tip-of-tree, previous-release: faster ✔ 2% - 12% (0.61ms - 5.20ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +2% (-1.39ms - +1.16ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +2% (-2.89ms - +9.15ms)
    this-change vs tip-of-tree

update-reflect

  • this-change: 503.77ms - 508.74ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +1% (-8.33ms - +5.56ms)
    this-change vs tip-of-tree

Results

this-change

render

VersionAvg timevs
47.24ms - 49.38ms-

update

VersionAvg timevs
501.12ms - 505.33ms-

update-reflect

VersionAvg timevs
503.77ms - 508.74ms-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
17.87ms - 18.72ms-unsure 🔍
-5% - +2%
-0.95ms - +0.30ms		unsure 🔍
-5% - +2%
-0.97ms - +0.33ms
tip-of-tree
tip-of-tree		18.16ms - 19.08msunsure 🔍
-2% - +5%
-0.30ms - +0.95ms		-unsure 🔍
-4% - +4%
-0.67ms - +0.68ms
previous-release
previous-release		18.12ms - 19.10msunsure 🔍
-2% - +5%
-0.33ms - +0.97ms		unsure 🔍
-4% - +4%
-0.68ms - +0.67ms		-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
38.25ms - 41.27ms-faster ✔
2% - 12%
0.61ms - 5.20ms		unsure 🔍
-9% - +3%
-3.57ms - +1.39ms
tip-of-tree
tip-of-tree		40.93ms - 44.39msslower ❌
1% - 13%
0.61ms - 5.20ms		-unsure 🔍
-2% - +11%
-0.81ms - +4.43ms
previous-release
previous-release		38.88ms - 42.82msunsure 🔍
-4% - +9%
-1.39ms - +3.57ms		unsure 🔍
-10% - +2%
-4.43ms - +0.81ms		-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
11.14ms - 11.79ms-unsure 🔍
-5% - +3%
-0.64ms - +0.32ms		unsure 🔍
-6% - +3%
-0.71ms - +0.35ms
tip-of-tree
tip-of-tree		11.28ms - 11.98msunsure 🔍
-3% - +6%
-0.32ms - +0.64ms		-unsure 🔍
-5% - +5%
-0.56ms - +0.53ms
previous-release
previous-release		11.22ms - 12.06msunsure 🔍
-3% - +6%
-0.35ms - +0.71ms		unsure 🔍
-5% - +5%
-0.53ms - +0.56ms		-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
34.18ms - 35.19ms-unsure 🔍
-1% - +4%
-0.21ms - +1.27ms		unsure 🔍
-1% - +3%
-0.29ms - +1.09ms
tip-of-tree
tip-of-tree		33.61ms - 34.70msunsure 🔍
-4% - +1%
-1.27ms - +0.21ms		-unsure 🔍
-2% - +2%
-0.85ms - +0.58ms
previous-release
previous-release		33.82ms - 34.76msunsure 🔍
-3% - +1%
-1.09ms - +0.29ms		unsure 🔍
-2% - +2%
-0.58ms - +0.85ms		-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
68.05ms - 69.37ms-unsure 🔍
-2% - +2%
-1.39ms - +1.16ms		unsure 🔍
-1% - +2%
-0.73ms - +1.25ms
tip-of-tree
tip-of-tree		67.73ms - 69.91msunsure 🔍
-2% - +2%
-1.16ms - +1.39ms		-unsure 🔍
-1% - +2%
-0.94ms - +1.70ms
previous-release
previous-release		67.70ms - 69.18msunsure 🔍
-2% - +1%
-1.25ms - +0.73ms		unsure 🔍
-2% - +1%
-1.70ms - +0.94ms		-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
32.87ms - 34.10ms-unsure 🔍
-1% - +3%
-0.32ms - +1.14ms		unsure 🔍
-3% - +3%
-0.96ms - +0.97ms
tip-of-tree
tip-of-tree		32.67ms - 33.48msunsure 🔍
-3% - +1%
-1.14ms - +0.32ms		-unsure 🔍
-4% - +1%
-1.25ms - +0.45ms
previous-release
previous-release		32.73ms - 34.22msunsure 🔍
-3% - +3%
-0.97ms - +0.96ms		unsure 🔍
-1% - +4%
-0.45ms - +1.25ms		-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
515.04ms - 524.86ms-unsure 🔍
-1% - +2%
-2.89ms - +9.15ms		unsure 🔍
-1% - +2%
-3.25ms - +9.45ms
tip-of-tree
tip-of-tree		513.34ms - 520.30msunsure 🔍
-2% - +1%
-9.15ms - +2.89ms		-unsure 🔍
-1% - +1%
-5.35ms - +5.29ms
previous-release
previous-release		512.83ms - 520.88msunsure 🔍
-2% - +1%
-9.45ms - +3.25ms		unsure 🔍
-1% - +1%
-5.29ms - +5.35ms		-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
518.70ms - 528.53ms-unsure 🔍
-2% - +1%
-8.33ms - +5.56ms		unsure 🔍
-1% - +1%
-4.97ms - +7.31ms
tip-of-tree
tip-of-tree		520.09ms - 529.91msunsure 🔍
-1% - +2%
-5.56ms - +8.33ms		-unsure 🔍
-1% - +2%
-3.58ms - +8.69ms
previous-release
previous-release		518.76ms - 526.13msunsure 🔍
-1% - +1%
-7.31ms - +4.97ms		unsure 🔍
-2% - +1%
-8.69ms - +3.58ms		-

tachometer-reporter-action v2 for Benchmarks

AndrewJakubowicz
AndrewJakubowicz approved these changes Nov 20, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@AndrewJakubowicz AndrewJakubowicz left a comment

Choose a reason for hiding this comment

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

Looks great! I suggested some changes to shrink the test to only reproduce the bug.

Thank you!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 20, 2023

The size of lit-html.js and lit-core.min.js are as expected.

@AndrewJakubowicz
Update packages/lit-html/src/test/directives/async-replace_test.ts
2fda7d2 
Co-authored-by: Andrew Jakubowicz <spyr1014@gmail.com>
augustjk
augustjk approved these changes Nov 20, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

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

Nice!

AndrewJakubowicz
AndrewJakubowicz reviewed Nov 21, 2023
View reviewed changes
@AndrewJakubowicz AndrewJakubowicz merged commit 1af7991 into lit:main Nov 21, 2023
@lit-robot lit-robot mentioned this pull request Jan 9, 2024
Merged
@eleses eleses mentioned this pull request Aug 15, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AndrewJakubowicz AndrewJakubowicz AndrewJakubowicz approved these changes

@augustjk augustjk augustjk approved these changes

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

@rictic rictic Awaiting requested review from rictic

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[lit-html] Re-rendering an asyncReplace clears its value.

3 participants

@sorvell @AndrewJakubowicz @augustjk