Give a clearer error message when rendering into null/undefined #2661

rictic · 2022-03-23T13:21:33Z

No description provided.

changeset-bot · 2022-03-23T13:21:36Z

🦋 Changeset detected

Latest commit: 6be911e

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 · 2022-03-23T13:22:07Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -9% - +2% (-3.00ms - +0.86ms)
^{this-change vs tip-of-tree}

render

lit-element-list: unsure 🔍 -3% - +1% (-3.02ms - +1.05ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -1% - +1% (-0.32ms - +0.21ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -4% - +1% (-0.51ms - +0.13ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +1% (-1.43ms - +0.97ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -3% - +1% (-2.05ms - +0.79ms)
^{this-change vs tip-of-tree}

update

lit-element-list: unsure 🔍 -2% - +2% (-13.88ms - +13.37ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -2% - +4% (-1.50ms - +3.48ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -2% - +1% (-8.53ms - +2.84ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -1% - +3% (-1.35ms - +3.68ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +2% (-12.93ms - +20.88ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: unsure 🔍 -1% - +1% (-13.24ms - +12.23ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -3% - +1% (-25.93ms - +8.87ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 98.0.4758.102
Sample size: 50
Built by: Benchmarks #2549
Commit: 2e92d80

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	86.72ms - 88.61ms	-	unsure 🔍 -3% - +1% -3.02ms - +1.05ms	faster ✔ 20% - 22% 21.87ms - 24.94ms
tip-of-tree tip-of-tree	86.85ms - 90.45ms	unsure 🔍 -1% - +3% -1.05ms - +3.02ms	-	faster ✔ 18% - 22% 20.25ms - 24.59ms
previous-release previous-release	109.86ms - 112.28ms	slower ❌ 25% - 29% 21.87ms - 24.94ms	slower ❌ 22% - 28% 20.25ms - 24.59ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	864.87ms - 881.40ms	-	unsure 🔍 -2% - +2% -13.88ms - +13.37ms	faster ✔ 7% - 9% 61.55ms - 86.24ms
tip-of-tree tip-of-tree	862.56ms - 884.21ms	unsure 🔍 -2% - +2% -13.37ms - +13.88ms	-	faster ✔ 6% - 9% 59.45ms - 87.82ms
previous-release previous-release	937.86ms - 956.19ms	slower ❌ 7% - 10% 61.55ms - 86.24ms	slower ❌ 7% - 10% 59.45ms - 87.82ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	957.84ms - 976.02ms	-	unsure 🔍 -1% - +1% -13.24ms - +12.23ms	faster ✔ 3% - 6% 31.20ms - 57.73ms
tip-of-tree tip-of-tree	958.51ms - 976.35ms	unsure 🔍 -1% - +1% -12.23ms - +13.24ms	-	faster ✔ 3% - 6% 30.81ms - 57.11ms
previous-release previous-release	1001.73ms - 1021.05ms	slower ❌ 3% - 6% 31.20ms - 57.73ms	slower ❌ 3% - 6% 30.81ms - 57.11ms	-

lit-html-kitchen-sink

Browser: chrome-headless 98.0.4758.102
Sample size: 30
Built by: Benchmarks #2549
Commit: 2e92d80

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	35.22ms - 35.59ms	-	unsure 🔍 -1% - +1% -0.32ms - +0.21ms	faster ✔ 14% - 21% 5.62ms - 9.24ms
tip-of-tree tip-of-tree	35.27ms - 35.65ms	unsure 🔍 -1% - +1% -0.21ms - +0.32ms	-	faster ✔ 14% - 21% 5.56ms - 9.19ms
previous-release previous-release	41.03ms - 44.64ms	slower ❌ 16% - 26% 5.62ms - 9.24ms	slower ❌ 16% - 26% 5.56ms - 9.19ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	95.02ms - 99.09ms	-	unsure 🔍 -2% - +4% -1.50ms - +3.48ms	faster ✔ 0% - 8% 0.33ms - 8.08ms
tip-of-tree tip-of-tree	94.62ms - 97.50ms	unsure 🔍 -4% - +2% -3.48ms - +1.50ms	-	faster ✔ 2% - 9% 1.60ms - 8.79ms
previous-release previous-release	97.96ms - 104.55ms	slower ❌ 0% - 8% 0.33ms - 8.08ms	slower ❌ 2% - 9% 1.60ms - 8.79ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	30.70ms - 31.19ms	-	unsure 🔍 -9% - +2% -3.00ms - +0.86ms	faster ✔ 12% - 15% 4.29ms - 5.49ms
tip-of-tree tip-of-tree	30.10ms - 33.93ms	unsure 🔍 -3% - +10% -0.86ms - +3.00ms	-	faster ✔ 5% - 16% 1.83ms - 5.82ms
previous-release previous-release	35.29ms - 36.39ms	slower ❌ 14% - 18% 4.29ms - 5.49ms	slower ❌ 5% - 19% 1.83ms - 5.82ms	-

lit-html-repeat

Browser: chrome-headless 98.0.4758.102
Sample size: 30
Built by: Benchmarks #2549
Commit: 2e92d80

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	12.86ms - 13.26ms	-	unsure 🔍 -4% - +1% -0.51ms - +0.13ms	faster ✔ 11% - 14% 1.61ms - 2.08ms
tip-of-tree tip-of-tree	12.99ms - 13.51ms	unsure 🔍 -1% - +4% -0.13ms - +0.51ms	-	faster ✔ 9% - 13% 1.37ms - 1.94ms
previous-release previous-release	14.78ms - 15.03ms	slower ❌ 12% - 16% 1.61ms - 2.08ms	slower ❌ 10% - 15% 1.37ms - 1.94ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	359.53ms - 366.86ms	-	unsure 🔍 -2% - +1% -8.53ms - +2.84ms	faster ✔ 30% - 32% 157.22ms - 169.04ms
tip-of-tree tip-of-tree	361.69ms - 370.39ms	unsure 🔍 -1% - +2% -2.84ms - +8.53ms	-	faster ✔ 29% - 31% 153.93ms - 166.65ms
previous-release previous-release	521.68ms - 530.96ms	slower ❌ 43% - 47% 157.22ms - 169.04ms	slower ❌ 42% - 46% 153.93ms - 166.65ms	-

lit-html-template-heavy

Browser: chrome-headless 98.0.4758.102
Sample size: 50
Built by: Benchmarks #2549
Commit: 2e92d80

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	64.15ms - 65.64ms	-	unsure 🔍 -2% - +1% -1.43ms - +0.97ms	faster ✔ 15% - 18% 11.27ms - 13.85ms
tip-of-tree tip-of-tree	64.18ms - 66.06ms	unsure 🔍 -1% - +2% -0.97ms - +1.43ms	-	faster ✔ 14% - 18% 10.92ms - 13.75ms
previous-release previous-release	76.40ms - 78.51ms	slower ❌ 17% - 21% 11.27ms - 13.85ms	slower ❌ 17% - 21% 10.92ms - 13.75ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	140.31ms - 144.35ms	-	unsure 🔍 -1% - +3% -1.35ms - +3.68ms	faster ✔ 13% - 16% 21.06ms - 26.99ms
tip-of-tree tip-of-tree	139.67ms - 142.67ms	unsure 🔍 -3% - +1% -3.68ms - +1.35ms	-	faster ✔ 14% - 17% 22.55ms - 27.83ms
previous-release previous-release	164.19ms - 168.53ms	slower ❌ 15% - 19% 21.06ms - 26.99ms	slower ❌ 16% - 20% 22.55ms - 27.83ms	-

reactive-element-list

Browser: chrome-headless 98.0.4758.102
Sample size: 50
Built by: Benchmarks #2549
Commit: 2e92d80

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	61.77ms - 63.96ms	-	unsure 🔍 -3% - +1% -2.05ms - +0.79ms	unsure 🔍 -3% - +1% -1.94ms - +0.73ms
tip-of-tree tip-of-tree	62.59ms - 64.40ms	unsure 🔍 -1% - +3% -0.79ms - +2.05ms	-	unsure 🔍 -2% - +2% -1.16ms - +1.21ms
previous-release previous-release	62.71ms - 64.23ms	unsure 🔍 -1% - +3% -0.73ms - +1.94ms	unsure 🔍 -2% - +2% -1.21ms - +1.16ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	861.65ms - 886.09ms	-	unsure 🔍 -1% - +2% -12.93ms - +20.88ms	unsure 🔍 -1% - +2% -10.61ms - +20.74ms
tip-of-tree tip-of-tree	858.21ms - 881.57ms	unsure 🔍 -2% - +1% -20.88ms - +12.93ms	-	unsure 🔍 -2% - +2% -14.17ms - +16.35ms
previous-release previous-release	858.98ms - 878.62ms	unsure 🔍 -2% - +1% -20.74ms - +10.61ms	unsure 🔍 -2% - +2% -16.35ms - +14.17ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	965.19ms - 989.60ms	-	unsure 🔍 -3% - +1% -25.93ms - +8.87ms	unsure 🔍 -2% - +1% -24.48ms - +10.02ms
tip-of-tree tip-of-tree	973.52ms - 998.32ms	unsure 🔍 -1% - +3% -8.87ms - +25.93ms	-	unsure 🔍 -2% - +2% -16.09ms - +18.69ms
previous-release previous-release	972.43ms - 996.81ms	unsure 🔍 -1% - +3% -10.02ms - +24.48ms	unsure 🔍 -2% - +2% -18.69ms - +16.09ms	-

_{tachometer-reporter-action v2 for Benchmarks}

justinfagnani · 2022-03-23T16:40:13Z

Is there an issue for this one? Was is coming up internally?

justinfagnani · 2022-03-23T16:42:48Z

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

@@ -637,6 +637,13 @@ export const render = (
  container: HTMLElement | DocumentFragment,
  options?: RenderOptions
 ): RootPart => {
+  if (DEV_MODE && container == null) {


This feels like runtime type-checking here, which we don't do in most other places. What's an example of a problematic call? Would a check that the container is a HTMLElement or DocumentFragment be better for that case?

The case that came up was:

render(html`...`, document.body);

In particular, the error use case was

<html> <head> <script src="./bundle.js"></script> </head> <body> <some-lit-element></some-lit-element> </body> </html>

Without this proposed patch, would generate the cryptic
Uncaught TypeError: Cannot read properties of null (reading '_$litPart$')\
error (because ./bundle.js launches before document.body is set).

This patch aims at creating a more humane error message in these kind of situations.

So because bundle.js is a sync script and it contains some top-level render(html..., document.body) call and document.body doesn't exist yet? The <some-lit-element> has nothing to do with it right?

Just wondering here because there are probably a lot of places in our API you could incorrectly pass null. This seems ok though.

Yes <some-lit-element> was there to justify the inclusion of lit in the bundle.js. The correct code should be

<html> <head></head> <body> <some-lit-element></some-lit-element> <script src="./bundle.js"></script> </body> </html>

Give a clearer error message when rendering into null/undefined

6be911e

rictic requested a review from justinfagnani as a code owner March 23, 2022 13:21

AndrewJakubowicz approved these changes Mar 23, 2022

View reviewed changes

justinfagnani reviewed Mar 23, 2022

View reviewed changes

justinfagnani approved these changes Mar 25, 2022

View reviewed changes

rictic merged commit 9a3a38c into main Mar 25, 2022

rictic deleted the clearer-null-error branch March 25, 2022 19:25

This was referenced Mar 31, 2022

Version Packages #2687

Closed

Version Packages #2700

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give a clearer error message when rendering into null/undefined #2661

Give a clearer error message when rendering into null/undefined #2661

rictic commented Mar 23, 2022

changeset-bot bot commented Mar 23, 2022

github-actions bot commented Mar 23, 2022 •

edited

📊 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 Mar 23, 2022

justinfagnani Mar 23, 2022

rictic Mar 23, 2022

rodrigob Mar 25, 2022

justinfagnani Mar 25, 2022

rodrigob Mar 25, 2022

Give a clearer error message when rendering into null/undefined #2661

Give a clearer error message when rendering into null/undefined #2661

Conversation

rictic commented Mar 23, 2022

changeset-bot bot commented Mar 23, 2022

🦋 Changeset detected

github-actions bot commented Mar 23, 2022 • edited

📊 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 Mar 23, 2022

justinfagnani Mar 23, 2022

Choose a reason for hiding this comment

rictic Mar 23, 2022

Choose a reason for hiding this comment

rodrigob Mar 25, 2022

Choose a reason for hiding this comment

justinfagnani Mar 25, 2022

Choose a reason for hiding this comment

rodrigob Mar 25, 2022

Choose a reason for hiding this comment

github-actions bot commented Mar 23, 2022 •

edited