Improve queryAssignedElements decorator browser compatibility #2392

AndrewJakubowicz · 2022-01-07T21:20:18Z

Context

Internally queryAssignedElements was using the newer HTMLSlotElement.assignedElements method. This newer method doesn't have ShadyDOM polyfill support, but would still leave some browsers unsupported if it did have polyfill support.

Using Modern browser breakdown as a guide, assignedElements is not supported until Firefox 66. This leaves Firefox versions 64 and 65 without support.

tl;dr queryAssignedElements doesn't work on older browsers.

How

Gracefully fallback on assignedNodes if required. The behavior of the decorator remains unchanged.

Testing

This is a private implementation change, so the existing unit tests should cover this. No functional changes.
Also testing via cl/420366110

Size of decorators before changes (on main):

Name                                   Size     Minified  Gzipped  Brotli   
----------------------------------------------------------------------------
decorators/query-assigned-elements.js  74 B     73 B      87 B     62 B     
decorators/query-assigned-nodes.js     71 B     70 B      86 B     63 B 

decorators/query-assigned-elements.js (DEV)  519 B     518 B     380 B    323 B
decorators/query-assigned-nodes.js (DEV)     593 B     592 B     442 B    362 B

Size of decorators after change:

Name                                   Size      Minified  Gzipped  Brotli   
-----------------------------------------------------------------------------
decorators/query-assigned-elements.js  74 B    73 B      87 B     62 B     
decorators/query-assigned-nodes.js     71 B    70 B      86 B     63 B

decorators/query-assigned-elements.js (DEV)  685 B     684 B     464 B    391 B    
decorators/query-assigned-nodes.js (DEV)     606 B     605 B     421 B    361 B

The release size is the same after this change, only dev has slightly increased in size.

…ort. Due to the Lit library browser recommendations and available polyfill support, the easiest way to ensure queryAssignedElements is fully supported is by continuing to use assignedNodes. Why? ShadyDOM doesn't polyfill assignedElements: webcomponents/polyfills#46 But even if we did polyfill assignedElements, there is still a complexity introduced by assignedElements being added to browsers slightly after we recommend polyfills on our browser support matrix. For example, Firefox 65 would not have polyfills applied, and doesn't yet support assignedElements. assignedElements are added in Firefox 66.

changeset-bot · 2022-01-07T21:20:22Z

🦋 Changeset detected

Latest commit: c4592a5

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-01-07T21:21:10Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -4% - +4% (-0.95ms - +1.12ms)
^{this-change vs tip-of-tree}

render

lit-element-list: unsure 🔍 -4% - +2% (-3.13ms - +1.77ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -3% - +10% (-0.88ms - +3.14ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -6% - +1% (-0.76ms - +0.10ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +4% (-0.91ms - +2.23ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -3% - +3% (-1.99ms - +1.44ms)
^{this-change vs tip-of-tree}

update

lit-element-list: unsure 🔍 -3% - +1% (-22.57ms - +8.88ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -3% - +5% (-2.36ms - +4.23ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -3% - +1% (-8.92ms - +2.22ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -3% - +2% (-3.94ms - +2.70ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -2% - +2% (-19.36ms - +13.86ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: unsure 🔍 -2% - +1% (-18.54ms - +13.89ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -4% - +1% (-38.69ms - +7.05ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 96.0.4664.110
Sample size: 50
Built by: Benchmarks #2199
Commit: d434be7

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	77.62ms - 80.25ms	-	unsure 🔍 -4% - +2% -3.13ms - +1.77ms	faster ✔ 18% - 22% 17.53ms - 21.40ms
tip-of-tree tip-of-tree	77.55ms - 81.68ms	unsure 🔍 -2% - +4% -1.77ms - +3.13ms	-	faster ✔ 17% - 21% 16.27ms - 21.29ms
previous-release previous-release	96.98ms - 99.82ms	slower ❌ 22% - 27% 17.53ms - 21.40ms	slower ❌ 20% - 27% 16.27ms - 21.29ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	829.62ms - 850.43ms	-	unsure 🔍 -3% - +1% -22.57ms - +8.88ms	faster ✔ 6% - 10% 56.00ms - 88.81ms
tip-of-tree tip-of-tree	835.08ms - 858.66ms	unsure 🔍 -1% - +3% -8.88ms - +22.57ms	-	faster ✔ 5% - 9% 48.25ms - 82.88ms
previous-release previous-release	899.76ms - 925.11ms	slower ❌ 7% - 11% 56.00ms - 88.81ms	slower ❌ 6% - 10% 48.25ms - 82.88ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	924.81ms - 946.80ms	-	unsure 🔍 -2% - +1% -18.54ms - +13.89ms	faster ✔ 2% - 8% 23.09ms - 81.12ms
tip-of-tree tip-of-tree	926.21ms - 950.05ms	unsure 🔍 -1% - +2% -13.89ms - +18.54ms	-	faster ✔ 2% - 8% 20.40ms - 79.16ms
previous-release previous-release	961.05ms - 1014.76ms	slower ❌ 2% - 9% 23.09ms - 81.12ms	slower ❌ 2% - 8% 20.40ms - 79.16ms	-

lit-html-kitchen-sink

Browser: chrome-headless 96.0.4664.110
Sample size: 30
Built by: Benchmarks #2199
Commit: d434be7

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	29.62ms - 33.48ms	-	unsure 🔍 -3% - +10% -0.88ms - +3.14ms	faster ✔ 7% - 19% 2.29ms - 6.85ms
tip-of-tree tip-of-tree	29.85ms - 30.99ms	unsure 🔍 -10% - +3% -3.14ms - +0.88ms	-	faster ✔ 13% - 19% 4.36ms - 7.05ms
previous-release previous-release	34.90ms - 37.34ms	slower ❌ 7% - 22% 2.29ms - 6.85ms	slower ❌ 14% - 23% 4.36ms - 7.05ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	83.16ms - 88.64ms	-	unsure 🔍 -3% - +5% -2.36ms - +4.23ms	faster ✔ 0% - 9% 0.18ms - 8.13ms
tip-of-tree tip-of-tree	83.14ms - 86.80ms	unsure 🔍 -5% - +3% -4.23ms - +2.36ms	-	faster ✔ 2% - 9% 1.68ms - 8.51ms
previous-release previous-release	87.18ms - 92.94ms	slower ❌ 0% - 10% 0.18ms - 8.13ms	slower ❌ 2% - 10% 1.68ms - 8.51ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	24.71ms - 26.27ms	-	unsure 🔍 -4% - +4% -0.95ms - +1.12ms	faster ✔ 12% - 19% 3.64ms - 5.82ms
tip-of-tree tip-of-tree	24.73ms - 26.08ms	unsure 🔍 -4% - +4% -1.12ms - +0.95ms	-	faster ✔ 13% - 19% 3.80ms - 5.83ms
previous-release previous-release	29.46ms - 30.98ms	slower ❌ 14% - 23% 3.64ms - 5.82ms	slower ❌ 15% - 23% 3.80ms - 5.83ms	-

lit-html-repeat

Browser: chrome-headless 96.0.4664.110
Sample size: 30
Built by: Benchmarks #2199
Commit: d434be7

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	11.32ms - 11.81ms	-	unsure 🔍 -6% - +1% -0.76ms - +0.10ms	faster ✔ 6% - 11% 0.75ms - 1.39ms
tip-of-tree tip-of-tree	11.54ms - 12.24ms	unsure 🔍 -1% - +7% -0.10ms - +0.76ms	-	faster ✔ 3% - 9% 0.34ms - 1.15ms
previous-release previous-release	12.43ms - 12.84ms	slower ❌ 6% - 12% 0.75ms - 1.39ms	slower ❌ 3% - 10% 0.34ms - 1.15ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	317.14ms - 325.56ms	-	unsure 🔍 -3% - +1% -8.92ms - +2.22ms	faster ✔ 30% - 32% 137.35ms - 153.01ms
tip-of-tree tip-of-tree	321.06ms - 328.34ms	unsure 🔍 -1% - +3% -2.22ms - +8.92ms	-	faster ✔ 29% - 32% 134.29ms - 149.37ms
previous-release previous-release	459.93ms - 473.13ms	slower ❌ 42% - 48% 137.35ms - 153.01ms	slower ❌ 41% - 46% 134.29ms - 149.37ms	-

lit-html-template-heavy

Browser: chrome-headless 96.0.4664.110
Sample size: 50
Built by: Benchmarks #2199
Commit: d434be7

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	56.76ms - 58.95ms	-	unsure 🔍 -2% - +4% -0.91ms - +2.23ms	faster ✔ 14% - 19% 9.38ms - 13.60ms
tip-of-tree tip-of-tree	56.07ms - 58.31ms	unsure 🔍 -4% - +2% -2.23ms - +0.91ms	-	faster ✔ 15% - 20% 10.03ms - 14.28ms
previous-release previous-release	67.54ms - 71.15ms	slower ❌ 16% - 24% 9.38ms - 13.60ms	slower ❌ 17% - 25% 10.03ms - 14.28ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	133.97ms - 138.56ms	-	unsure 🔍 -3% - +2% -3.94ms - +2.70ms	faster ✔ 11% - 15% 17.14ms - 23.87ms
tip-of-tree tip-of-tree	134.49ms - 139.28ms	unsure 🔍 -2% - +3% -2.70ms - +3.94ms	-	faster ✔ 11% - 15% 16.46ms - 23.32ms
previous-release previous-release	154.31ms - 159.22ms	slower ❌ 12% - 18% 17.14ms - 23.87ms	slower ❌ 12% - 17% 16.46ms - 23.32ms	-

reactive-element-list

Browser: chrome-headless 96.0.4664.110
Sample size: 50
Built by: Benchmarks #2199
Commit: d434be7

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	56.17ms - 58.27ms	-	unsure 🔍 -3% - +3% -1.99ms - +1.44ms	unsure 🔍 -4% - +1% -2.37ms - +0.80ms
tip-of-tree tip-of-tree	56.13ms - 58.85ms	unsure 🔍 -3% - +3% -1.44ms - +1.99ms	-	unsure 🔍 -4% - +2% -2.32ms - +1.29ms
previous-release previous-release	56.81ms - 59.19ms	unsure 🔍 -1% - +4% -0.80ms - +2.37ms	unsure 🔍 -2% - +4% -1.29ms - +2.32ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	869.86ms - 890.76ms	-	unsure 🔍 -2% - +2% -19.36ms - +13.86ms	unsure 🔍 -2% - +2% -13.94ms - +17.31ms
tip-of-tree tip-of-tree	870.16ms - 895.97ms	unsure 🔍 -2% - +2% -13.86ms - +19.36ms	-	unsure 🔍 -1% - +2% -12.93ms - +21.80ms
previous-release previous-release	867.01ms - 890.25ms	unsure 🔍 -2% - +2% -17.31ms - +13.94ms	unsure 🔍 -2% - +1% -21.80ms - +12.93ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	988.99ms - 1013.39ms	-	unsure 🔍 -4% - +1% -38.69ms - +7.05ms	unsure 🔍 -2% - +2% -19.02ms - +15.46ms
tip-of-tree tip-of-tree	997.67ms - 1036.35ms	unsure 🔍 -1% - +4% -7.05ms - +38.69ms	-	unsure 🔍 -1% - +4% -8.81ms - +36.90ms
previous-release previous-release	990.79ms - 1015.15ms	unsure 🔍 -2% - +2% -15.46ms - +19.02ms	unsure 🔍 -4% - +1% -36.90ms - +8.81ms	-

_{tachometer-reporter-action v2 for Benchmarks}

packages/reactive-element/src/decorators/query-assigned-elements.ts

This provides a better experience on modern browsers.

aomarks

Looks good to me! Would be nice to get a stamp from @kevinpschaaf too on this approach.

.changeset/stupid-rockets-deliver.md

Using != will check against null and undefined. opts passed into slotAssignedElementsPolyfill can also be undefined.

AndrewJakubowicz · 2022-01-09T05:47:33Z

Note, this has passed isolated tgp with the new commit.

kevinpschaaf

LGTM

packages/reactive-element/src/decorators/query-assigned-elements.ts

AndrewJakubowicz added 2 commits January 7, 2022 13:06

Add changeset

0cb3ee5

AndrewJakubowicz marked this pull request as ready for review January 7, 2022 21:51

AndrewJakubowicz requested a review from justinfagnani as a code owner January 7, 2022 21:51

AndrewJakubowicz requested a review from aomarks January 7, 2022 22:32

aomarks reviewed Jan 7, 2022

View reviewed changes

packages/reactive-element/src/decorators/query-assigned-elements.ts Outdated Show resolved Hide resolved

Add aomarks suggested module scoped polyfill for assignedElements.

f69390f

This provides a better experience on modern browsers.

aomarks approved these changes Jan 8, 2022

View reviewed changes

aomarks reviewed Jan 8, 2022

View reviewed changes

.changeset/stupid-rockets-deliver.md Outdated Show resolved Hide resolved

Update changeset to be clearer.

03ae5b0

AndrewJakubowicz requested a review from kevinpschaaf January 8, 2022 22:19

Fix types found in internal tests.

d5f79fa

Using != will check against null and undefined. opts passed into slotAssignedElementsPolyfill can also be undefined.

kevinpschaaf approved these changes Jan 10, 2022

View reviewed changes

kevinpschaaf requested a review from sorvell January 10, 2022 18:02

sorvell requested changes Jan 10, 2022

View reviewed changes

packages/reactive-element/src/decorators/query-assigned-elements.ts Outdated Show resolved Hide resolved

Apply code review feedback to remove .call method.

c4592a5

AndrewJakubowicz requested a review from sorvell January 10, 2022 19:21

sorvell approved these changes Jan 12, 2022

View reviewed changes

AndrewJakubowicz merged commit dc3301c into main Jan 12, 2022

AndrewJakubowicz deleted the assigned-elements-compatibility branch January 12, 2022 00:19

github-actions bot mentioned this pull request Jan 12, 2022

Version Packages #2400

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve queryAssignedElements decorator browser compatibility #2392

Improve queryAssignedElements decorator browser compatibility #2392

AndrewJakubowicz commented Jan 7, 2022 •

edited

changeset-bot bot commented Jan 7, 2022 •

edited

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

aomarks left a comment

AndrewJakubowicz commented Jan 9, 2022 •

edited

kevinpschaaf left a comment

Improve queryAssignedElements decorator browser compatibility #2392

Improve queryAssignedElements decorator browser compatibility #2392

Conversation

AndrewJakubowicz commented Jan 7, 2022 • edited

Context

How

Testing

changeset-bot bot commented Jan 7, 2022 • edited

🦋 Changeset detected

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

aomarks left a comment

Choose a reason for hiding this comment

AndrewJakubowicz commented Jan 9, 2022 • edited

kevinpschaaf left a comment

Choose a reason for hiding this comment

AndrewJakubowicz commented Jan 7, 2022 •

edited

changeset-bot bot commented Jan 7, 2022 •

edited

github-actions bot commented Jan 7, 2022 •

edited

AndrewJakubowicz commented Jan 9, 2022 •

edited