[labs/ssr] Make FallbackRenderer not emit a shadow root by justinfagnani · Pull Request #3590 · lit/lit

justinfagnani · 2023-01-18T20:16:20Z

The bug was that FallbackRenderer didn't return undefined from renderShadow() - as a generator it returns an empty iterable. The fix is done by making a default implementation of renderShadow() in ElementRenderer that returns undefined and removing the FallbackRenderer implementation. I did that for other callbacks too and updated some comments.

No updated tests just yet, I'll add that in a bit.

changeset-bot · 2023-01-18T20:16:24Z

⚠️ No Changeset found

Latest commit: 143fac6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes changesets to release 1 package

Name	Type
lit-html	Patch

Click here to learn what changesets are, and how to add one.

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

github-actions · 2023-01-18T20:16:41Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -7% - +11% (-1.75ms - +2.59ms)
^{this-change vs tip-of-tree}

render

lit-element-list: 74.72ms - 79.57ms
lit-html-kitchen-sink: unsure 🔍 -3% - +5% (-1.04ms - +1.58ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -4% - +5% (-0.49ms - +0.58ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -1% - +3% (-0.74ms - +1.41ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -3% - +2% (-1.58ms - +1.03ms)
^{this-change vs tip-of-tree}

update

lit-element-list: 685.22ms - 698.34ms
lit-html-kitchen-sink: slower ❌ 0% - 7% (0.21ms - 5.35ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -3% - +3% (-9.26ms - +7.97ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -1% - +1% (-0.82ms - +0.95ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +0% (-4.50ms - +2.50ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: 686.96ms - 695.07ms
reactive-element-list: unsure 🔍 -0% - +0% (-3.45ms - +2.78ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 109.0.5414.74
Sample size: 50
Built by: Benchmarks #4326
Commit: 9155fc0

render

Version	Avg time	vs
	74.72ms - 79.57ms	-

update

Version	Avg time	vs
	685.22ms - 698.34ms	-

update-reflect

Version	Avg time	vs
	686.96ms - 695.07ms	-

lit-html-kitchen-sink

Browser: chrome-headless 109.0.5414.74
Sample size: 30
Built by: Benchmarks #4326
Commit: 9155fc0

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	30.02ms - 31.51ms	-	unsure 🔍 -3% - +5% -1.04ms - +1.58ms	unsure 🔍 -3% - +5% -0.87ms - +1.45ms
tip-of-tree tip-of-tree	29.41ms - 31.58ms	unsure 🔍 -5% - +3% -1.58ms - +1.04ms	-	unsure 🔍 -5% - +5% -1.38ms - +1.42ms
previous-release previous-release	29.59ms - 31.37ms	unsure 🔍 -5% - +3% -1.45ms - +0.87ms	unsure 🔍 -5% - +5% -1.42ms - +1.38ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	77.55ms - 81.13ms	-	slower ❌ 0% - 7% 0.21ms - 5.35ms	unsure 🔍 -3% - +4% -2.39ms - +2.93ms
tip-of-tree tip-of-tree	74.72ms - 78.40ms	faster ✔ 0% - 7% 0.21ms - 5.35ms	-	unsure 🔍 -7% - +0% -5.20ms - +0.18ms
previous-release previous-release	77.10ms - 81.04ms	unsure 🔍 -4% - +3% -2.93ms - +2.39ms	unsure 🔍 -0% - +7% -0.18ms - +5.20ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	23.04ms - 26.49ms	-	unsure 🔍 -7% - +11% -1.75ms - +2.59ms	unsure 🔍 -3% - +18% -0.61ms - +4.01ms
tip-of-tree tip-of-tree	23.02ms - 25.67ms	unsure 🔍 -10% - +7% -2.59ms - +1.75ms	-	unsure 🔍 -4% - +15% -0.75ms - +3.31ms
previous-release previous-release	21.52ms - 24.61ms	unsure 🔍 -16% - +2% -4.01ms - +0.61ms	unsure 🔍 -13% - +3% -3.31ms - +0.75ms	-

lit-html-repeat

Browser: chrome-headless 109.0.5414.74
Sample size: 30
Built by: Benchmarks #4326
Commit: 9155fc0

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	11.87ms - 12.65ms	-	unsure 🔍 -4% - +5% -0.49ms - +0.58ms	unsure 🔍 -6% - +5% -0.70ms - +0.64ms
tip-of-tree tip-of-tree	11.84ms - 12.58ms	unsure 🔍 -5% - +4% -0.58ms - +0.49ms	-	unsure 🔍 -6% - +5% -0.73ms - +0.58ms
previous-release previous-release	11.75ms - 12.83ms	unsure 🔍 -5% - +6% -0.64ms - +0.70ms	unsure 🔍 -5% - +6% -0.58ms - +0.73ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	267.89ms - 280.67ms	-	unsure 🔍 -3% - +3% -9.26ms - +7.97ms	unsure 🔍 -1% - +5% -1.57ms - +14.09ms
tip-of-tree tip-of-tree	269.15ms - 280.70ms	unsure 🔍 -3% - +3% -7.97ms - +9.26ms	-	unsure 🔍 -0% - +5% -0.43ms - +14.24ms
previous-release previous-release	263.50ms - 272.54ms	unsure 🔍 -5% - +1% -14.09ms - +1.57ms	unsure 🔍 -5% - +0% -14.24ms - +0.43ms	-

lit-html-template-heavy

Browser: chrome-headless 109.0.5414.74
Sample size: 50
Built by: Benchmarks #4326
Commit: 9155fc0

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	51.36ms - 52.86ms	-	unsure 🔍 -1% - +3% -0.74ms - +1.41ms	unsure 🔍 -2% - +2% -1.05ms - +1.12ms
tip-of-tree tip-of-tree	51.01ms - 52.55ms	unsure 🔍 -3% - +1% -1.41ms - +0.74ms	-	unsure 🔍 -3% - +2% -1.40ms - +0.81ms
previous-release previous-release	51.28ms - 52.86ms	unsure 🔍 -2% - +2% -1.12ms - +1.05ms	unsure 🔍 -2% - +3% -0.81ms - +1.40ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	105.04ms - 106.16ms	-	unsure 🔍 -1% - +1% -0.82ms - +0.95ms	unsure 🔍 -1% - +1% -0.83ms - +1.14ms
tip-of-tree tip-of-tree	104.85ms - 106.22ms	unsure 🔍 -1% - +1% -0.95ms - +0.82ms	-	unsure 🔍 -1% - +1% -0.97ms - +1.15ms
previous-release previous-release	104.64ms - 106.25ms	unsure 🔍 -1% - +1% -1.14ms - +0.83ms	unsure 🔍 -1% - +1% -1.15ms - +0.97ms	-

reactive-element-list

Browser: chrome-headless 109.0.5414.74
Sample size: 50
Built by: Benchmarks #4326
Commit: 9155fc0

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	49.20ms - 51.18ms	-	unsure 🔍 -3% - +2% -1.58ms - +1.03ms	unsure 🔍 -2% - +3% -0.84ms - +1.64ms
tip-of-tree tip-of-tree	49.62ms - 51.31ms	unsure 🔍 -2% - +3% -1.03ms - +1.58ms	-	unsure 🔍 -1% - +4% -0.44ms - +1.80ms
previous-release previous-release	49.04ms - 50.53ms	unsure 🔍 -3% - +2% -1.64ms - +0.84ms	unsure 🔍 -4% - +1% -1.80ms - +0.44ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	689.69ms - 695.09ms	-	unsure 🔍 -1% - +0% -4.50ms - +2.50ms	unsure 🔍 -1% - +0% -4.37ms - +3.09ms
tip-of-tree tip-of-tree	691.16ms - 695.62ms	unsure 🔍 -0% - +1% -2.50ms - +4.50ms	-	unsure 🔍 -0% - +1% -3.05ms - +3.77ms
previous-release previous-release	690.45ms - 695.61ms	unsure 🔍 -0% - +1% -3.09ms - +4.37ms	unsure 🔍 -1% - +0% -3.77ms - +3.05ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	720.66ms - 724.62ms	-	unsure 🔍 -0% - +0% -3.45ms - +2.78ms	unsure 🔍 -0% - +0% -3.32ms - +3.28ms
tip-of-tree tip-of-tree	720.57ms - 725.38ms	unsure 🔍 -0% - +0% -2.78ms - +3.45ms	-	unsure 🔍 -0% - +1% -3.26ms - +3.89ms
previous-release previous-release	720.02ms - 725.30ms	unsure 🔍 -0% - +0% -3.28ms - +3.32ms	unsure 🔍 -1% - +0% -3.89ms - +3.26ms	-

_{tachometer-reporter-action v2 for Benchmarks}

aomarks

Test

.changeset/wicked-baboons-pull.md

packages/labs/ssr/src/lib/element-renderer.ts

Co-authored-by: Augustine Kim <augustinekim@google.com>

justinfagnani · 2023-01-25T05:28:37Z

@aomarks PTAL, but I can't get the Sauce test runners passing. There's a persistent error about the /home/runner/work/lit/lit/node_modules/saucelabs/build/sc-loader/.sc-v4.8.0/bin/sc file being busy. I've seen this on other branches, but it went away when re-running failed tests. That doesn't seem to be working here.

augustjk

LGTM

justinfagnani · 2023-01-28T02:20:13Z

@aomarks tests seem to be ok now

aomarks

Nice!

justinfagnani added 6 commits January 18, 2023 09:08

Better hydration error messages

400e34d

Add changeset

bef2914

Deprecate ShadowRootOptions in favor of ShadowRootInit

244e69b

Add default implementations of ElementRenderer methods

07517d1

Update comments

1025e75

Don't check for presence of non-optional method.

d7a9d10

justinfagnani requested review from aomarks, augustjk and kevinpschaaf January 18, 2023 20:16

aomarks requested changes Jan 18, 2023

View reviewed changes

.changeset/wicked-baboons-pull.md Outdated Show resolved Hide resolved

augustjk reviewed Jan 18, 2023

View reviewed changes

packages/labs/ssr/src/lib/element-renderer.ts Outdated Show resolved Hide resolved

justinfagnani and others added 4 commits January 24, 2023 18:35

Update packages/labs/ssr/src/lib/element-renderer.ts

0342f13

Co-authored-by: Augustine Kim <augustinekim@google.com>

Merge branch 'main' into ssr-fallback-renderer

7b10a87

Fix changeset

5549326

Add test

64fda27

Update koa

143fac6

justinfagnani requested review from aomarks and augustjk January 26, 2023 00:59

augustjk approved these changes Jan 26, 2023

View reviewed changes

justinfagnani enabled auto-merge (squash) January 28, 2023 02:19

aomarks approved these changes Jan 29, 2023

View reviewed changes

justinfagnani merged commit 1d8a38e into main Jan 29, 2023

justinfagnani deleted the ssr-fallback-renderer branch January 29, 2023 16:55

augustjk mentioned this pull request Mar 10, 2023

[chore] Update queued changesets for next release #3719

Merged

Conversation

justinfagnani commented Jan 18, 2023

Uh oh!

changeset-bot bot commented Jan 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

github-actions bot commented Jan 18, 2023 • 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

Uh oh!

aomarks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

justinfagnani commented Jan 25, 2023

Uh oh!

augustjk left a comment

Choose a reason for hiding this comment

Uh oh!

justinfagnani commented Jan 28, 2023

Uh oh!

aomarks left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

changeset-bot bot commented Jan 18, 2023 •

edited

Loading

github-actions bot commented Jan 18, 2023 •

edited

Loading