[lit-labs/observers] add base class #3386

AndrewJakubowicz · 2022-10-21T22:57:09Z

This PR adds a base class to IntersectionController, ResizeController, and MutationController.

Fixed a bug where we accept a ResizeController config and then never use it. Fixed by passing the config to ResizeController.observe which was not previously done.

The rest of this change is only a refactor. Adding a new base class ObserverController.

To support adding the base class some properties needed their visibility changed from private to protected.

Size

With this PR:

File name	Size	Gzipped	Brotli
resize_controller.js	509 B	298 B	247 B
mutation_controller.js	509 B	302 B	244 B
intersection_controller.js	590 B	328 B	268 B
observer_controller.js	534 B	309 B	262 B

Totals (including performance_controller)	3.03 kB	1.68 kB	1.37 kB

On main:

File name	Size	Gzipped	Brotli
resize_controller.js	860 B	427 B	359 B
mutation_controller.js	860 B	436 B	365 B
intersection_controller.js	902 B	445 B	369 B

Totals (including performance_controller)	3.51 kB	1.76 kB	1.44 kB

Test plan

Tested by ensuring all tests continue passing unchanged.

Focus of this change was to tackle the simple cases and remove all the duplicated logic.

changeset-bot · 2022-10-21T22:57:12Z

🦋 Changeset detected

Latest commit: 3d70087

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

This PR includes changesets to release 1 package

Name	Type
@lit-labs/observers	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-10-21T22:57:33Z

📊 Tachometer Benchmark Results

Summary

nop-update

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

render

lit-element-list: 110.19ms - 113.96ms
lit-html-kitchen-sink: unsure 🔍 -4% - +3% (-1.94ms - +1.52ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -5% - +5% (-0.75ms - +0.84ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -4% - +4% (-3.15ms - +3.11ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +3% (-0.85ms - +2.22ms)
^{this-change vs tip-of-tree}

update

lit-element-list: 1029.15ms - 1057.51ms
lit-html-kitchen-sink: unsure 🔍 -6% - +4% (-7.51ms - +5.60ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -4% - +2% (-16.44ms - +6.05ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +4% (-3.51ms - +6.71ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -4% - +2% (-40.40ms - +17.14ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: 1046.49ms - 1076.64ms
reactive-element-list: unsure 🔍 -3% - +1% (-37.39ms - +11.71ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 107.0.5304.121
Sample size: 50
Built by: Benchmarks #4155
Commit: 67a69b1

render

Version	Avg time	vs
	110.19ms - 113.96ms	-

update

Version	Avg time	vs
	1029.15ms - 1057.51ms	-

update-reflect

Version	Avg time	vs
	1046.49ms - 1076.64ms	-

lit-html-kitchen-sink

Browser: chrome-headless 107.0.5304.121
Sample size: 30
Built by: Benchmarks #4155
Commit: 67a69b1

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	45.50ms - 47.96ms	-	unsure 🔍 -4% - +3% -1.94ms - +1.52ms	unsure 🔍 -5% - +2% -2.55ms - +1.17ms
tip-of-tree tip-of-tree	45.73ms - 48.15ms	unsure 🔍 -3% - +4% -1.52ms - +1.94ms	-	unsure 🔍 -5% - +3% -2.33ms - +1.37ms
previous-release previous-release	46.03ms - 48.81ms	unsure 🔍 -3% - +5% -1.17ms - +2.55ms	unsure 🔍 -3% - +5% -1.37ms - +2.33ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	123.20ms - 131.78ms	-	unsure 🔍 -6% - +4% -7.51ms - +5.60ms	unsure 🔍 -5% - +4% -5.75ms - +5.42ms
tip-of-tree tip-of-tree	123.49ms - 133.41ms	unsure 🔍 -4% - +6% -5.60ms - +7.51ms	-	unsure 🔍 -4% - +5% -5.32ms - +6.90ms
previous-release previous-release	124.09ms - 131.23ms	unsure 🔍 -4% - +5% -5.42ms - +5.75ms	unsure 🔍 -5% - +4% -6.90ms - +5.32ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	30.02ms - 31.76ms	-	unsure 🔍 -4% - +6% -1.23ms - +1.71ms	unsure 🔍 -5% - +4% -1.39ms - +1.34ms
tip-of-tree tip-of-tree	29.47ms - 31.83ms	unsure 🔍 -6% - +4% -1.71ms - +1.23ms	-	unsure 🔍 -6% - +4% -1.85ms - +1.31ms
previous-release previous-release	29.86ms - 31.96ms	unsure 🔍 -4% - +5% -1.34ms - +1.39ms	unsure 🔍 -4% - +6% -1.31ms - +1.85ms	-

lit-html-repeat

Browser: chrome-headless 107.0.5304.121
Sample size: 30
Built by: Benchmarks #4155
Commit: 67a69b1

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	15.11ms - 15.91ms	-	unsure 🔍 -5% - +5% -0.75ms - +0.84ms	unsure 🔍 -7% - +2% -1.13ms - +0.37ms
tip-of-tree tip-of-tree	14.78ms - 16.15ms	unsure 🔍 -5% - +5% -0.84ms - +0.75ms	-	unsure 🔍 -8% - +3% -1.36ms - +0.50ms
previous-release previous-release	15.26ms - 16.52ms	unsure 🔍 -2% - +7% -0.37ms - +1.13ms	unsure 🔍 -3% - +9% -0.50ms - +1.36ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	358.86ms - 373.36ms	-	unsure 🔍 -4% - +2% -16.44ms - +6.05ms	unsure 🔍 -3% - +2% -12.45ms - +8.64ms
tip-of-tree tip-of-tree	362.71ms - 379.91ms	unsure 🔍 -2% - +5% -6.05ms - +16.44ms	-	unsure 🔍 -2% - +4% -8.22ms - +14.80ms
previous-release previous-release	360.36ms - 375.67ms	unsure 🔍 -2% - +3% -8.64ms - +12.45ms	unsure 🔍 -4% - +2% -14.80ms - +8.22ms	-

lit-html-template-heavy

Browser: chrome-headless 107.0.5304.121
Sample size: 50
Built by: Benchmarks #4155
Commit: 67a69b1

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	71.79ms - 76.32ms	-	unsure 🔍 -4% - +4% -3.15ms - +3.11ms	unsure 🔍 -3% - +5% -2.03ms - +3.94ms
tip-of-tree tip-of-tree	71.92ms - 76.23ms	unsure 🔍 -4% - +4% -3.11ms - +3.15ms	-	unsure 🔍 -3% - +5% -1.92ms - +3.88ms
previous-release previous-release	71.15ms - 75.04ms	unsure 🔍 -5% - +3% -3.94ms - +2.03ms	unsure 🔍 -5% - +3% -3.88ms - +1.92ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	149.52ms - 157.45ms	-	unsure 🔍 -2% - +4% -3.51ms - +6.71ms	unsure 🔍 -1% - +6% -1.56ms - +8.89ms
tip-of-tree tip-of-tree	148.66ms - 155.11ms	unsure 🔍 -4% - +2% -6.71ms - +3.51ms	-	unsure 🔍 -2% - +5% -2.63ms - +6.75ms
previous-release previous-release	146.42ms - 153.23ms	unsure 🔍 -6% - +1% -8.89ms - +1.56ms	unsure 🔍 -4% - +2% -6.75ms - +2.63ms	-

reactive-element-list

Browser: chrome-headless 107.0.5304.121
Sample size: 50
Built by: Benchmarks #4155
Commit: 67a69b1

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	76.96ms - 79.26ms	-	unsure 🔍 -1% - +3% -0.85ms - +2.22ms	unsure 🔍 -2% - +2% -1.46ms - +1.78ms
tip-of-tree tip-of-tree	76.41ms - 78.44ms	unsure 🔍 -3% - +1% -2.22ms - +0.85ms	-	unsure 🔍 -3% - +1% -2.05ms - +1.00ms
previous-release previous-release	76.81ms - 79.08ms	unsure 🔍 -2% - +2% -1.78ms - +1.46ms	unsure 🔍 -1% - +3% -1.00ms - +2.05ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	1097.06ms - 1137.19ms	-	unsure 🔍 -4% - +2% -40.40ms - +17.14ms	unsure 🔍 -4% - +1% -41.34ms - +17.10ms
tip-of-tree tip-of-tree	1108.14ms - 1149.38ms	unsure 🔍 -2% - +4% -17.14ms - +40.40ms	-	unsure 🔍 -3% - +3% -30.09ms - +29.11ms
previous-release previous-release	1108.00ms - 1150.49ms	unsure 🔍 -2% - +4% -17.10ms - +41.34ms	unsure 🔍 -3% - +3% -29.11ms - +30.09ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	1135.36ms - 1169.16ms	-	unsure 🔍 -3% - +1% -37.39ms - +11.71ms	unsure 🔍 -3% - +1% -35.82ms - +15.38ms
tip-of-tree tip-of-tree	1147.30ms - 1182.90ms	unsure 🔍 -1% - +3% -11.71ms - +37.39ms	-	unsure 🔍 -2% - +2% -23.58ms - +28.83ms
previous-release previous-release	1143.25ms - 1181.70ms	unsure 🔍 -1% - +3% -15.38ms - +35.82ms	unsure 🔍 -2% - +2% -28.83ms - +23.58ms	-

_{tachometer-reporter-action v2 for Benchmarks}

packages/labs/observers/src/base_controller.ts

nickschaap · 2022-12-17T20:01:16Z

packages/labs/observers/src/intersection_controller.ts

+import {
+  ObserverController,
+  type ObserverControllerConfig,
+} from './observer_controller.js';


nit: I noticed a lot of other lit packages use kebab case instead of snake case for file naming. Is it worth changing the names of these controllers to normalize consumption with the remainder of the lit repo?

I just did this here: #3771

shaal · 2023-03-29T20:19:50Z

Lit's ResizeController always call render() twice, before any size changes.
Simplified example - https://lit.dev/playground/#gist=d26a19ef6637e12160c703079e37c9af

When using {skipInitial = true}, does this PR solve that problem?

AndrewJakubowicz added 2 commits October 21, 2022 15:51

add labs/observers base class to remove duplicated logic

cc1a183

Focus of this change was to tackle the simple cases and remove all the duplicated logic.

add ignore for distributed files

96b51ba

AndrewJakubowicz requested a review from justinfagnani as a code owner October 21, 2022 22:57

sorvell reviewed Oct 21, 2022

View reviewed changes

packages/labs/observers/src/base_controller.ts Outdated Show resolved Hide resolved

Code review feedback and some minor type improvements

094f560

AndrewJakubowicz force-pushed the observers-base-class branch from b6b3719 to 094f560 Compare October 21, 2022 23:29

add changeset

843c7cd

AndrewJakubowicz requested a review from sorvell October 21, 2022 23:45

AndrewJakubowicz added 2 commits October 26, 2022 15:18

Merge branch 'main' of github.com:lit/lit into observers-base-class

24398c9

Merge branch 'main' of github.com:lit/lit into observers-base-class

3d70087

nickschaap reviewed Dec 17, 2022

View reviewed changes

a11delavar mentioned this pull request Mar 22, 2024

[labs/observers] Prevent failing in SSR #4591

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lit-labs/observers] add base class #3386

[lit-labs/observers] add base class #3386

AndrewJakubowicz commented Oct 21, 2022 •

edited

Loading

changeset-bot bot commented Oct 21, 2022 •

edited

Loading

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

nickschaap Dec 17, 2022

justinfagnani Mar 30, 2023

shaal commented Mar 29, 2023

[lit-labs/observers] add base class #3386

Are you sure you want to change the base?

[lit-labs/observers] add base class #3386

Conversation

AndrewJakubowicz commented Oct 21, 2022 • edited Loading

Size

Test plan

changeset-bot bot commented Oct 21, 2022 • edited Loading

🦋 Changeset detected

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

nickschaap Dec 17, 2022

Choose a reason for hiding this comment

justinfagnani Mar 30, 2023

Choose a reason for hiding this comment

shaal commented Mar 29, 2023

AndrewJakubowicz commented Oct 21, 2022 •

edited

Loading

changeset-bot bot commented Oct 21, 2022 •

edited

Loading

github-actions bot commented Oct 21, 2022 •

edited

Loading