[localize] Fix handling of nested HTML by aomarks · Pull Request #3674 · lit/lit

aomarks · 2023-02-16T00:21:52Z

Fixes handling of nested html-tagged template expressions in lit localize analysis.

For example, the following example was analyzed incorrectly:

msg(html`Hello <b>${html`<i>World</i>`}</b>!`);

We combine runs of HTML markup and expressions so that we have fewer placeholders during translation, so we pass the template through an HTML parser, nested expressions and all. The problem was that if the nested expressions contained HTML, those would get parsed as part of the outer message, which would corrupt the analyzed template. The fix was to replace nested expressions with placeholders (the index) during HTML parsing (because we don't need the HTML parser to see the contents of expressions at all) and then substitute the original expression back after HTML parsing.

Fixes #2426

changeset-bot · 2023-02-16T00:21:55Z

🦋 Changeset detected

Latest commit: 7c836d5

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

This PR includes changesets to release 1 package

Name	Type
@lit/localize-tools	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 · 2023-02-16T00:22:15Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -14% - +3% (-3.76ms - +0.79ms)
^{this-change vs tip-of-tree}

render

lit-element-list: 77.45ms - 81.13ms
lit-html-kitchen-sink: unsure 🔍 -4% - +4% (-1.33ms - +1.43ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -8% - +2% (-1.01ms - +0.23ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +5% (-1.01ms - +2.62ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -2% - +2% (-1.02ms - +1.22ms)
^{this-change vs tip-of-tree}

update

lit-element-list: 826.58ms - 835.34ms
lit-html-kitchen-sink: unsure 🔍 -6% - +3% (-4.90ms - +2.44ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -9% - +7% (-30.64ms - +25.10ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +1% (-3.12ms - +1.53ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -0% - +1% (-2.27ms - +9.61ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: 824.64ms - 832.84ms
reactive-element-list: unsure 🔍 -0% - +1% (-2.16ms - +10.07ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4428
Commit: 11f0a71

render

Version	Avg time	vs
	77.45ms - 81.13ms	-

update

Version	Avg time	vs
	826.58ms - 835.34ms	-

update-reflect

Version	Avg time	vs
	824.64ms - 832.84ms	-

lit-html-kitchen-sink

Browser: chrome-headless 109.0.5414.119
Sample size: 30
Built by: Benchmarks #4428
Commit: 11f0a71

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	31.77ms - 33.91ms	-	unsure 🔍 -4% - +4% -1.33ms - +1.43ms	unsure 🔍 -5% - +4% -1.49ms - +1.32ms
tip-of-tree tip-of-tree	31.92ms - 33.66ms	unsure 🔍 -4% - +4% -1.43ms - +1.33ms	-	unsure 🔍 -4% - +3% -1.38ms - +1.12ms
previous-release previous-release	32.02ms - 33.83ms	unsure 🔍 -4% - +5% -1.32ms - +1.49ms	unsure 🔍 -3% - +4% -1.12ms - +1.38ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	80.74ms - 86.40ms	-	unsure 🔍 -6% - +3% -4.90ms - +2.44ms	unsure 🔍 -9% - +1% -8.35ms - +0.56ms
tip-of-tree tip-of-tree	82.45ms - 87.14ms	unsure 🔍 -3% - +6% -2.44ms - +4.90ms	-	unsure 🔍 -8% - +2% -6.83ms - +1.49ms
previous-release previous-release	84.02ms - 90.90ms	unsure 🔍 -1% - +10% -0.56ms - +8.35ms	unsure 🔍 -2% - +8% -1.49ms - +6.83ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	22.80ms - 26.06ms	-	unsure 🔍 -14% - +3% -3.76ms - +0.79ms	unsure 🔍 -15% - +1% -3.94ms - +0.45ms
tip-of-tree tip-of-tree	24.32ms - 27.50ms	unsure 🔍 -4% - +16% -0.79ms - +3.76ms	-	unsure 🔍 -9% - +7% -2.43ms - +1.91ms
previous-release previous-release	24.70ms - 27.65ms	unsure 🔍 -2% - +16% -0.45ms - +3.94ms	unsure 🔍 -7% - +9% -1.91ms - +2.43ms	-

lit-html-repeat

Browser: chrome-headless 109.0.5414.119
Sample size: 30
Built by: Benchmarks #4428
Commit: 11f0a71

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	11.51ms - 12.32ms	-	unsure 🔍 -8% - +2% -1.01ms - +0.23ms	unsure 🔍 -7% - +3% -0.92ms - +0.41ms
tip-of-tree tip-of-tree	11.84ms - 12.77ms	unsure 🔍 -2% - +9% -0.23ms - +1.01ms	-	unsure 🔍 -5% - +7% -0.57ms - +0.84ms
previous-release previous-release	11.64ms - 12.70ms	unsure 🔍 -3% - +8% -0.41ms - +0.92ms	unsure 🔍 -7% - +5% -0.84ms - +0.57ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	321.02ms - 358.38ms	-	unsure 🔍 -9% - +7% -30.64ms - +25.10ms	unsure 🔍 -8% - +8% -28.71ms - +26.02ms
tip-of-tree tip-of-tree	321.79ms - 363.16ms	unsure 🔍 -7% - +9% -25.10ms - +30.64ms	-	unsure 🔍 -8% - +9% -27.35ms - +30.21ms
previous-release previous-release	321.04ms - 361.05ms	unsure 🔍 -8% - +8% -26.02ms - +28.71ms	unsure 🔍 -9% - +8% -30.21ms - +27.35ms	-

lit-html-template-heavy

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4428
Commit: 11f0a71

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	54.94ms - 57.80ms	-	unsure 🔍 -2% - +5% -1.01ms - +2.62ms	unsure 🔍 -1% - +5% -0.81ms - +2.81ms
tip-of-tree tip-of-tree	54.44ms - 56.69ms	unsure 🔍 -5% - +2% -2.62ms - +1.01ms	-	unsure 🔍 -3% - +3% -1.39ms - +1.78ms
previous-release previous-release	54.26ms - 56.48ms	unsure 🔍 -5% - +1% -2.81ms - +0.81ms	unsure 🔍 -3% - +2% -1.78ms - +1.39ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	123.16ms - 125.58ms	-	unsure 🔍 -2% - +1% -3.12ms - +1.53ms	unsure 🔍 -1% - +2% -0.69ms - +2.54ms
tip-of-tree tip-of-tree	123.18ms - 127.15ms	unsure 🔍 -1% - +3% -1.53ms - +3.12ms	-	unsure 🔍 -0% - +3% -0.54ms - +3.98ms
previous-release previous-release	122.38ms - 124.51ms	unsure 🔍 -2% - +1% -2.54ms - +0.69ms	unsure 🔍 -3% - +0% -3.98ms - +0.54ms	-

reactive-element-list

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4428
Commit: 11f0a71

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	52.86ms - 54.34ms	-	unsure 🔍 -2% - +2% -1.02ms - +1.22ms	unsure 🔍 -1% - +3% -0.38ms - +1.39ms
tip-of-tree tip-of-tree	52.65ms - 54.34ms	unsure 🔍 -2% - +2% -1.22ms - +1.02ms	-	unsure 🔍 -1% - +3% -0.57ms - +1.38ms
previous-release previous-release	52.60ms - 53.58ms	unsure 🔍 -3% - +1% -1.39ms - +0.38ms	unsure 🔍 -3% - +1% -1.38ms - +0.57ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	871.84ms - 880.12ms	-	unsure 🔍 -0% - +1% -2.27ms - +9.61ms	unsure 🔍 -0% - +1% -0.35ms - +10.91ms
tip-of-tree tip-of-tree	868.05ms - 876.57ms	unsure 🔍 -1% - +0% -9.61ms - +2.27ms	-	unsure 🔍 -0% - +1% -4.11ms - +7.33ms
previous-release previous-release	866.89ms - 874.51ms	unsure 🔍 -1% - +0% -10.91ms - +0.35ms	unsure 🔍 -1% - +0% -7.33ms - +4.11ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	883.87ms - 893.43ms	-	unsure 🔍 -0% - +1% -2.16ms - +10.07ms	unsure 🔍 -0% - +1% -4.31ms - +8.77ms
tip-of-tree tip-of-tree	880.88ms - 888.51ms	unsure 🔍 -1% - +0% -10.07ms - +2.16ms	-	unsure 🔍 -1% - +0% -7.60ms - +4.15ms
previous-release previous-release	881.96ms - 890.88ms	unsure 🔍 -1% - +0% -8.77ms - +4.31ms	unsure 🔍 -0% - +1% -4.15ms - +7.60ms	-

_{tachometer-reporter-action v2 for Benchmarks}

augustjk

Nice! Much simpler this way.

aomarks added 3 commits February 15, 2023 16:13

Cover nested html in test cases

e139baf

Safely handle embedded HTML while passing through HTML parser

8cd54c5

Changeset

7c836d5

aomarks requested a review from augustjk February 16, 2023 00:21

aomarks requested a review from justinfagnani as a code owner February 16, 2023 00:21

augustjk approved these changes Feb 16, 2023

View reviewed changes

aomarks merged commit 52ab087 into main Feb 16, 2023

aomarks deleted the localize-nested-html branch February 16, 2023 00:53

lit-robot mentioned this pull request Mar 10, 2023

Version Packages #3717

Merged

Conversation

aomarks commented Feb 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Feb 16, 2023

🦋 Changeset detected

Uh oh!

github-actions bot commented Feb 16, 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!

augustjk left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aomarks commented Feb 16, 2023 •

edited

Loading

github-actions bot commented Feb 16, 2023 •

edited

Loading