Allow SSR renderers to produce asynchronous values #3467

justinfagnani · 2022-11-17T15:52:49Z

This is a breaking change that expands the return types of render() and the ElementRenderer methods.

Addresses #3219

changeset-bot · 2022-11-17T15:52:52Z

🦋 Changeset detected

Latest commit: 60986e2

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

This PR includes changesets to release 3 packages

Name	Type
@lit-labs/ssr	Major
@lit-labs/eleventy-plugin-lit	Patch
@lit-labs/testing	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 · 2022-11-17T15:53:09Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -8% - -0% (-1.99ms - +0.03ms)
^{this-change vs tip-of-tree}

render

lit-element-list: 87.00ms - 91.09ms
lit-html-kitchen-sink: unsure 🔍 -4% - +6% (-1.30ms - +1.98ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -5% - +5% (-0.55ms - +0.62ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -1% - +4% (-0.52ms - +2.14ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -4% - +3% (-2.44ms - +1.88ms)
^{this-change vs tip-of-tree}

update

lit-element-list: 844.65ms - 859.60ms
lit-html-kitchen-sink: faster ✔ 1% - 8% (0.62ms - 7.73ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -2% - +2% (-6.87ms - +4.75ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +1% (-2.66ms - +1.15ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -2% - +0% (-17.02ms - +1.38ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: 837.82ms - 851.29ms
reactive-element-list: unsure 🔍 -1% - +0% (-13.03ms - +1.09ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 108.0.5359.94
Sample size: 50
Built by: Benchmarks #4176
Commit: 1502e7b

render

Version	Avg time	vs
	87.00ms - 91.09ms	-

update

Version	Avg time	vs
	844.65ms - 859.60ms	-

update-reflect

Version	Avg time	vs
	837.82ms - 851.29ms	-

lit-html-kitchen-sink

Browser: chrome-headless 108.0.5359.94
Sample size: 30
Built by: Benchmarks #4176
Commit: 1502e7b

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	32.58ms - 35.03ms	-	unsure 🔍 -4% - +6% -1.30ms - +1.98ms	unsure 🔍 -13% - +3% -4.90ms - +1.30ms
tip-of-tree tip-of-tree	32.38ms - 34.56ms	unsure 🔍 -6% - +4% -1.98ms - +1.30ms	-	unsure 🔍 -14% - +2% -5.19ms - +0.92ms
previous-release previous-release	32.75ms - 38.46ms	unsure 🔍 -4% - +15% -1.30ms - +4.90ms	unsure 🔍 -3% - +16% -0.92ms - +5.19ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	87.81ms - 91.85ms	-	faster ✔ 1% - 8% 0.62ms - 7.73ms	unsure 🔍 -8% - +1% -7.25ms - +1.18ms
tip-of-tree tip-of-tree	91.09ms - 96.93ms	slower ❌ 1% - 9% 0.62ms - 7.73ms	-	unsure 🔍 -4% - +6% -3.57ms - +5.86ms
previous-release previous-release	89.17ms - 96.57ms	unsure 🔍 -1% - +8% -1.18ms - +7.25ms	unsure 🔍 -6% - +4% -5.86ms - +3.57ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	23.13ms - 24.05ms	-	unsure 🔍 -8% - -0% -1.99ms - +0.03ms	unsure 🔍 -5% - +1% -1.24ms - +0.26ms
tip-of-tree tip-of-tree	23.67ms - 25.47ms	unsure 🔍 -0% - +8% -0.03ms - +1.99ms	-	unsure 🔍 -2% - +7% -0.59ms - +1.57ms
previous-release previous-release	23.48ms - 24.67ms	unsure 🔍 -1% - +5% -0.26ms - +1.24ms	unsure 🔍 -6% - +2% -1.57ms - +0.59ms	-

lit-html-repeat

Browser: chrome-headless 108.0.5359.94
Sample size: 30
Built by: Benchmarks #4176
Commit: 1502e7b

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	11.72ms - 12.50ms	-	unsure 🔍 -5% - +5% -0.55ms - +0.62ms	unsure 🔍 -3% - +7% -0.37ms - +0.82ms
tip-of-tree tip-of-tree	11.64ms - 12.51ms	unsure 🔍 -5% - +5% -0.62ms - +0.55ms	-	unsure 🔍 -4% - +7% -0.44ms - +0.81ms
previous-release previous-release	11.44ms - 12.34ms	unsure 🔍 -7% - +3% -0.82ms - +0.37ms	unsure 🔍 -7% - +4% -0.81ms - +0.44ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	308.41ms - 315.68ms	-	unsure 🔍 -2% - +2% -6.87ms - +4.75ms	unsure 🔍 -3% - +1% -8.33ms - +4.57ms
tip-of-tree tip-of-tree	308.58ms - 317.64ms	unsure 🔍 -2% - +2% -4.75ms - +6.87ms	-	unsure 🔍 -2% - +2% -7.81ms - +6.17ms
previous-release previous-release	308.60ms - 319.25ms	unsure 🔍 -1% - +3% -4.57ms - +8.33ms	unsure 🔍 -2% - +2% -6.17ms - +7.81ms	-

lit-html-template-heavy

Browser: chrome-headless 108.0.5359.94
Sample size: 50
Built by: Benchmarks #4176
Commit: 1502e7b

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	56.69ms - 58.62ms	-	unsure 🔍 -1% - +4% -0.52ms - +2.14ms	unsure 🔍 -3% - +2% -1.55ms - +1.40ms
tip-of-tree tip-of-tree	55.93ms - 57.76ms	unsure 🔍 -4% - +1% -2.14ms - +0.52ms	-	unsure 🔍 -4% - +1% -2.32ms - +0.56ms
previous-release previous-release	56.62ms - 58.84ms	unsure 🔍 -2% - +3% -1.40ms - +1.55ms	unsure 🔍 -1% - +4% -0.56ms - +2.32ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	127.31ms - 129.30ms	-	unsure 🔍 -2% - +1% -2.66ms - +1.15ms	unsure 🔍 -2% - +1% -2.71ms - +1.10ms
tip-of-tree tip-of-tree	127.44ms - 130.68ms	unsure 🔍 -1% - +2% -1.15ms - +2.66ms	-	unsure 🔍 -2% - +2% -2.35ms - +2.25ms
previous-release previous-release	127.49ms - 130.73ms	unsure 🔍 -1% - +2% -1.10ms - +2.71ms	unsure 🔍 -2% - +2% -2.25ms - +2.35ms	-

reactive-element-list

Browser: chrome-headless 108.0.5359.94
Sample size: 50
Built by: Benchmarks #4176
Commit: 1502e7b

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	54.90ms - 57.79ms	-	unsure 🔍 -4% - +3% -2.44ms - +1.88ms	unsure 🔍 -5% - +2% -2.91ms - +1.00ms
tip-of-tree tip-of-tree	55.02ms - 58.23ms	unsure 🔍 -3% - +4% -1.88ms - +2.44ms	-	unsure 🔍 -5% - +2% -2.75ms - +1.40ms
previous-release previous-release	55.99ms - 58.61ms	unsure 🔍 -2% - +5% -1.00ms - +2.91ms	unsure 🔍 -2% - +5% -1.40ms - +2.75ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	871.83ms - 879.50ms	-	unsure 🔍 -2% - +0% -17.02ms - +1.38ms	unsure 🔍 -1% - +0% -10.76ms - +1.78ms
tip-of-tree tip-of-tree	875.12ms - 891.85ms	unsure 🔍 -0% - +2% -1.38ms - +17.02ms	-	unsure 🔍 -1% - +1% -6.40ms - +13.05ms
previous-release previous-release	875.19ms - 885.12ms	unsure 🔍 -0% - +1% -1.78ms - +10.76ms	unsure 🔍 -1% - +1% -13.05ms - +6.40ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	883.05ms - 892.47ms	-	unsure 🔍 -1% - +0% -13.03ms - +1.09ms	unsure 🔍 -1% - +0% -12.74ms - +3.58ms
tip-of-tree tip-of-tree	888.47ms - 898.99ms	unsure 🔍 -0% - +1% -1.09ms - +13.03ms	-	unsure 🔍 -1% - +1% -7.09ms - +9.88ms
previous-release previous-release	885.68ms - 899.00ms	unsure 🔍 -0% - +1% -3.58ms - +12.74ms	unsure 🔍 -1% - +1% -9.88ms - +7.09ms	-

_{tachometer-reporter-action v2 for Benchmarks}

.changeset/thick-bees-approve.md

justinfagnani · 2022-11-17T17:05:34Z

One open question for me I didn't look into yet is how this integrates into a streaming response like Koa. I suspect we'll need a utility that can pipe into a Stream correctly, but we don't seem to have an example of that in the monorepo yet. Adding a test that emulates a Koa environment could be a blocker for this PR.

packages/labs/ssr/src/test/lib/render-result_test.ts

kevinpschaaf · 2022-11-17T17:53:37Z

I suspect we'll need a utility that can pipe into a Stream correctly, but we don't seem to have an example of that in the monorepo yet.

I think we just need the utility to be an async generator that yields out strings as they resolve that can be fed into Readable.from(), ala this thing I did: https://github.com/lit/lit/pull/1690/files#diff-4e468b31bd6e90e2344bb5e19d58aa208aa697524998f5348bd640f85fa7eca1R11-R23

And you can test that by making a Readable and then flushing it out.

justinfagnani · 2022-11-17T18:02:18Z

@kevinpschaaf

I think we just need the utility to be an async generator that yields out strings as they resolve

This could be micro-optimization, but because of the overhead we saw with async generators before I would like to not incur the overhead of a promise and microtask for synchronous strings. It just seems like that would diminish or possibly eliminate the benefit we get from using sync generators.

justinfagnani · 2022-11-18T01:58:59Z

I added a RenderResultReadable that consumes a RenderResult. It should be about as optimal as we can get:

It uses a stack instead of recursion
It'll loop through sync values as much as possible, only returning from _read() when the underlying Readable pushes back with this.push() returning false
Almost no async overhead when consuming sync values: it doesn't use async functions, but then()s Promises only when encountered.

This should be usable with server frameworks like Koa like:

app.use('/', async (context) => {
  const result = render(...);
  context.body = new RenderResultReadable(result);
}

(Koa automatically pipes the body to the response when it's a Readable).

aomarks

Awesome! A bunch of small comments.

.changeset/thick-bees-approve.md

aomarks · 2022-11-18T18:40:50Z

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


  /**
   * Render an element's light DOM children.
   */
-  abstract renderLight(renderInfo: RenderInfo): IterableIterator<string>;
+  abstract renderLight(renderInfo: RenderInfo): RenderResult | undefined;


Why can this now be undefined?

I think it was a bad assumption that just because a template had <my-el>${renderLight()}<my-el> that <my-el> actually has a renderLight() implementation. So it makes sense to me that renderLight() would have nothing to render and could return undefined rather than an empty iterable.

oops, forgot to send my comments

packages/labs/ssr/src/lib/render-result.ts

packages/labs/ssr/src/lib/render-result-readable.ts

justinfagnani · 2022-11-18T20:27:54Z

I added lit/lit.dev#1005 for the corresponding lit.dev update.

aomarks · 2022-11-18T22:30:45Z

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


  /**
   * Render an element's light DOM children.
   */
-  abstract renderLight(renderInfo: RenderInfo): IterableIterator<string>;
+  abstract renderLight(renderInfo: RenderInfo): RenderResult | undefined;


augustjk

Nice! We should update the koa demo to use the RenderResultReadable. There's already some maintenance needed for the demo #3118 so could be part of that? Maybe the readme update/cleanup should be a separate task so we don't duplicate work across updating readme and the docs on lit.dev.

packages/labs/ssr/src/lib/render-result-readable.ts

justinfagnani · 2022-11-19T00:04:46Z

@augustjk

We should update the koa demo to use the RenderResultReadable

Ah, I forgot this was in the monorepo already. I'll update it. I kind of want things like this to be in the /examples folder I was talking about a few eng meetings ago.

.changeset/thick-bees-approve.md

packages/labs/ssr/src/lib/render-result.ts

justinfagnani · 2022-12-08T23:14:14Z

I simplified RenderResult as requested, and the implementations resulting from that. Worth a new review from everyone.

augustjk

Oops, changing base branch back to main produced a conflict to be resolved. New changes look good!

kevinpschaaf

LGTM

Allow SSR renderers to produce asynchronous values

4487d0a

Fix build error in localize-tools

c1ac817

justinfagnani marked this pull request as ready for review November 17, 2022 17:00

justinfagnani requested review from aomarks, kevinpschaaf and augustjk and removed request for aomarks November 17, 2022 17:00

justinfagnani commented Nov 17, 2022

View reviewed changes

.changeset/thick-bees-approve.md Outdated Show resolved Hide resolved

kevinpschaaf reviewed Nov 17, 2022

View reviewed changes

packages/labs/ssr/src/test/lib/render-result_test.ts Outdated Show resolved Hide resolved

packages/labs/ssr/src/test/lib/render-result_test.ts Show resolved Hide resolved

justinfagnani added 2 commits November 17, 2022 17:35

Add RenderResultReadable

e73140f

Add tests of Promises of iterables and fix bugs.

0506f09

justinfagnani requested a review from kevinpschaaf November 18, 2022 01:59

Expand on the change description

b7a5fd9

aomarks requested changes Nov 18, 2022

View reviewed changes

justinfagnani mentioned this pull request Nov 18, 2022

Update SSR docs for RenderResult lit/lit.dev#1005

Merged

Address feedback

d972146

justinfagnani requested a review from aomarks November 18, 2022 20:39

aomarks approved these changes Nov 18, 2022

View reviewed changes

augustjk approved these changes Nov 18, 2022

View reviewed changes

augustjk reviewed Nov 18, 2022

View reviewed changes

packages/labs/ssr/src/lib/render-result-readable.ts Outdated Show resolved Hide resolved

kevinpschaaf reviewed Nov 21, 2022

View reviewed changes

.changeset/thick-bees-approve.md Outdated Show resolved Hide resolved

packages/labs/ssr/src/lib/render-result.ts Outdated Show resolved Hide resolved

augustjk changed the base branch from main to labs-ssr-3.0 December 6, 2022 02:14

justinfagnani added 2 commits December 8, 2022 14:50

Address feedback: simplify RenderResult type

2e17b40

Simplify RenderResultReadable

c209664

justinfagnani requested review from kevinpschaaf, augustjk and aomarks December 8, 2022 23:14

aomarks approved these changes Dec 8, 2022

View reviewed changes

augustjk changed the base branch from labs-ssr-3.0 to main December 8, 2022 23:26

Merge branch 'main' into ssr-async-types

60986e2

augustjk approved these changes Dec 8, 2022

View reviewed changes

kevinpschaaf approved these changes Dec 15, 2022

View reviewed changes

usergenic mentioned this pull request Dec 15, 2022

Version Packages #3528

Closed

justinfagnani merged commit c77220e into main Jan 4, 2023

justinfagnani deleted the ssr-async-types branch January 4, 2023 20:18

This was referenced Jan 9, 2023

Version Packages #3554

Closed

Version Packages #3556

Merged

augustjk mentioned this pull request Apr 13, 2023

[labs/ssr] ElementRenderer async rendering interoperability? #3219

Closed

1 task

AndrewJakubowicz mentioned this pull request Nov 10, 2023

[labs/ssr] fix: RenderResultReadable skipping async work #4389

Merged

Allow SSR renderers to produce asynchronous values #3467

Allow SSR renderers to produce asynchronous values #3467

Conversation

justinfagnani commented Nov 17, 2022 • edited Loading

changeset-bot bot commented Nov 17, 2022 • edited Loading

🦋 Changeset detected

github-actions bot commented Nov 17, 2022 • 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

justinfagnani commented Nov 17, 2022

kevinpschaaf commented Nov 17, 2022

justinfagnani commented Nov 17, 2022

justinfagnani commented Nov 18, 2022

aomarks left a comment

Choose a reason for hiding this comment

aomarks Nov 18, 2022

Choose a reason for hiding this comment

justinfagnani Nov 18, 2022

Choose a reason for hiding this comment

aomarks Nov 18, 2022

Choose a reason for hiding this comment

justinfagnani Nov 18, 2022

Choose a reason for hiding this comment

justinfagnani commented Nov 18, 2022

aomarks Nov 18, 2022

Choose a reason for hiding this comment

augustjk left a comment

Choose a reason for hiding this comment

justinfagnani commented Nov 19, 2022

justinfagnani commented Dec 8, 2022

augustjk left a comment

Choose a reason for hiding this comment

kevinpschaaf left a comment

Choose a reason for hiding this comment

justinfagnani commented Nov 17, 2022 •

edited

Loading

changeset-bot bot commented Nov 17, 2022 •

edited

Loading

github-actions bot commented Nov 17, 2022 •

edited

Loading