Add @lit-labs/signals package #4615

justinfagnani · 2024-04-10T16:13:35Z

WIP: Still some rough edges with trying to sync watch() updates and regular updates.

changeset-bot · 2024-04-10T16:13:39Z

🦋 Changeset detected

Latest commit: 52dd697

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

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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 · 2024-04-10T16:13:50Z

📊 Tachometer Benchmark Results

Summary

nop-update

this-change, tip-of-tree, previous-release: unsure 🔍 -6% - +11% (-0.81ms - +1.39ms)
^{this-change vs tip-of-tree}

render

this-change: 46.64ms - 55.96ms
this-change, tip-of-tree, previous-release: unsure 🔍 -5% - +3% (-0.97ms - +0.52ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +2% (-0.90ms - +0.76ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: faster ✔ 4% - 43% (0.81ms - 31.93ms)
^{this-change vs tip-of-tree}

update

this-change: 513.58ms - 517.31ms
this-change, tip-of-tree, previous-release: unsure 🔍 -4% - +5% (-1.57ms - +1.92ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +3% (-1.30ms - +1.92ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +2% (-10.79ms - +9.59ms)
^{this-change vs tip-of-tree}

update-reflect

this-change: 511.91ms - 517.67ms
this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +0% (-9.88ms - +1.65ms)
^{this-change vs tip-of-tree}

Results

this-change

Browser: chrome-headless 129.0.0.0
Sample size: 50
Built by: Benchmarks #6437
Commit: 52dd697

render

Version	Avg time	vs
	46.64ms - 55.96ms	-

update

Version	Avg time	vs
	513.58ms - 517.31ms	-

update-reflect

Version	Avg time	vs
	511.91ms - 517.67ms	-

this-change, tip-of-tree, previous-release

Browser: chrome-headless 129.0.0.0
Sample size: 30
Built by: Benchmarks #6437
Commit: 52dd697

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	19.35ms - 20.32ms	-	unsure 🔍 -5% - +3% -0.97ms - +0.52ms	unsure 🔍 -4% - +2% -0.79ms - +0.48ms
tip-of-tree tip-of-tree	19.49ms - 20.62ms	unsure 🔍 -3% - +5% -0.52ms - +0.97ms	-	unsure 🔍 -3% - +4% -0.63ms - +0.77ms
previous-release previous-release	19.57ms - 20.40ms	unsure 🔍 -2% - +4% -0.48ms - +0.79ms	unsure 🔍 -4% - +3% -0.77ms - +0.63ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	39.11ms - 41.20ms	-	unsure 🔍 -4% - +5% -1.57ms - +1.92ms	unsure 🔍 -5% - +2% -2.00ms - +0.93ms
tip-of-tree tip-of-tree	38.59ms - 41.37ms	unsure 🔍 -5% - +4% -1.92ms - +1.57ms	-	unsure 🔍 -6% - +2% -2.44ms - +1.02ms
previous-release previous-release	39.66ms - 41.72ms	unsure 🔍 -2% - +5% -0.93ms - +2.00ms	unsure 🔍 -3% - +6% -1.02ms - +2.44ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	12.53ms - 13.97ms	-	unsure 🔍 -6% - +11% -0.81ms - +1.39ms	unsure 🔍 -10% - +6% -1.38ms - +0.79ms
tip-of-tree tip-of-tree	12.13ms - 13.79ms	unsure 🔍 -10% - +6% -1.39ms - +0.81ms	-	unsure 🔍 -13% - +4% -1.74ms - +0.58ms
previous-release previous-release	12.73ms - 14.35ms	unsure 🔍 -6% - +10% -0.79ms - +1.38ms	unsure 🔍 -5% - +14% -0.58ms - +1.74ms	-

this-change, tip-of-tree, previous-release

Browser: chrome-headless 129.0.0.0
Sample size: 50
Built by: Benchmarks #6437
Commit: 52dd697

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	35.70ms - 36.50ms	-	unsure 🔍 -2% - +2% -0.90ms - +0.76ms	unsure 🔍 -1% - +2% -0.24ms - +0.77ms
tip-of-tree tip-of-tree	35.45ms - 36.89ms	unsure 🔍 -2% - +2% -0.76ms - +0.90ms	-	unsure 🔍 -1% - +3% -0.46ms - +1.12ms
previous-release previous-release	35.53ms - 36.15ms	unsure 🔍 -2% - +1% -0.77ms - +0.24ms	unsure 🔍 -3% - +1% -1.12ms - +0.46ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	73.38ms - 75.83ms	-	unsure 🔍 -2% - +3% -1.30ms - +1.92ms	unsure 🔍 -1% - +3% -0.59ms - +2.52ms
tip-of-tree tip-of-tree	73.26ms - 75.34ms	unsure 🔍 -3% - +2% -1.92ms - +1.30ms	-	unsure 🔍 -1% - +3% -0.76ms - +2.07ms
previous-release previous-release	72.68ms - 74.61ms	unsure 🔍 -3% - +1% -2.52ms - +0.59ms	unsure 🔍 -3% - +1% -2.07ms - +0.76ms	-

this-change, tip-of-tree, previous-release

Browser: chrome-headless 129.0.0.0
Sample size: 50
Built by: Benchmarks #6437
Commit: 52dd697

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	43.19ms - 63.54ms	-	faster ✔ 4% - 43% 0.81ms - 31.93ms	unsure 🔍 -38% - +6% -25.15ms - +5.33ms
tip-of-tree tip-of-tree	57.97ms - 81.51ms	unsure 🔍 -3% - +64% +0.81ms - +31.93ms	-	unsure 🔍 -17% - +37% -9.89ms - +22.81ms
previous-release previous-release	51.93ms - 74.62ms	unsure 🔍 -12% - +50% -5.33ms - +25.15ms	unsure 🔍 -32% - +13% -22.81ms - +9.89ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	522.51ms - 529.32ms	-	unsure 🔍 -2% - +2% -10.79ms - +9.59ms	unsure 🔍 -1% - +1% -3.73ms - +6.57ms
tip-of-tree tip-of-tree	516.91ms - 536.12ms	unsure 🔍 -2% - +2% -9.59ms - +10.79ms	-	unsure 🔍 -2% - +2% -8.33ms - +12.37ms
previous-release previous-release	520.64ms - 528.36ms	unsure 🔍 -1% - +1% -6.57ms - +3.73ms	unsure 🔍 -2% - +2% -12.37ms - +8.33ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	553.18ms - 557.85ms	-	unsure 🔍 -2% - +0% -9.88ms - +1.65ms	unsure 🔍 -1% - +0% -4.72ms - +2.26ms
tip-of-tree tip-of-tree	554.36ms - 564.90ms	unsure 🔍 -0% - +2% -1.65ms - +9.88ms	-	unsure 🔍 -1% - +2% -2.99ms - +8.76ms
previous-release previous-release	554.16ms - 559.34ms	unsure 🔍 -0% - +1% -2.26ms - +4.72ms	unsure 🔍 -2% - +1% -8.76ms - +2.99ms	-

_{tachometer-reporter-action v2 for Benchmarks}

github-actions · 2024-04-10T16:15:38Z

The size of lit-html.js and lit-core.min.js are as expected.

sorvell

For watch, let's consider @e111077 's feedback and maybe have a way to auto-wrap a function (maybe if it's not an event part?).

And I really like using the html and not manually watching but if we do that we probably want unwatched which is just basically an echoing directive.

packages/labs/signals/README.md

packages/labs/signals/src/lib/signal-watcher.ts

packages/labs/signals/src/lib/watch.ts

packages/labs/signals/README.md

packages/labs/signals/rollup.config.js

packages/labs/signals/src/lib/html-tag.ts

packages/labs/signals/src/lib/signal-watcher.ts

packages/labs/signals/src/index.ts

packages/labs/signals/src/lib/signal-watcher.ts

packages/labs/signals/src/lib/watch.ts

JosefJezek · 2024-05-07T12:42:38Z

@justinfagnani any progress?

justinfagnani · 2024-07-31T20:24:35Z

@JosefJezek just pushed a major update. Hopefully we land this soon.

justinfagnani · 2024-07-31T20:25:33Z

@sorvell and @e111077 thanks for the first pass a while back! PTAL

I rewrote most of SignalWatcher and I think simplified things a lot, and I added a bunch of comments, so hopefully it's easier to follow now.

packages/labs/signals/src/lib/signal-watcher.ts

justinfagnani · 2024-07-31T20:29:36Z

When this is merged we'll need to create a Lit Labs page entry on lit.dev and a feedback discussion thread similar to #4115

packages/labs/signals/README.md

packages/labs/signals/src/lib/html-tag.ts

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

Co-authored-by: Pascal Schilp <pascalschilp@gmail.com>

packages/labs/signals/src/lib/signal-watcher.ts

packages/labs/signals/README.md

packages/labs/signals/src/index.ts

packages/labs/signals/src/lib/signal-watcher.ts

packages/labs/signals/src/lib/watch.ts

packages/labs/signals/src/lib/signal-watcher.ts

packages/labs/signals/src/test/signal-watcher_test.ts

packages/labs/signals/README.md

packages/labs/signals/src/lib/signal-watcher.ts

packages/labs/signals/src/lib/watch.ts

packages/labs/signals/src/lib/signal-watcher.ts

justinfagnani · 2024-08-30T20:54:05Z

@sorvell @e111077 I implemented the manual memory management code in the last two commits, if you can review that.

I'll review the last round of comments ASAP, if there are any left unresolved.

e111077

The memory approach is clever. Wonder if we might run into issues down the line with watchers running unnecessarily because of FinalizationRegistry timing. Though only things I could think of that might run into that are users writing brittle code that is probably in need of a rewrite anyway

packages/labs/signals/LICENSE

packages/labs/signals/README.md

justinfagnani · 2024-09-05T18:04:13Z

All the current test failures are from virtualizer. Re-running tests hasn't made them pass, so I'm not sure if they're flaky.

justinfagnani · 2024-09-06T02:22:23Z

I tried to update test dependencies in #4758 to see if that would help the virtualizer tests. That didn't work locally at least, so I'll keep re-running the test job for now.

justinfagnani

PTAL!

packages/labs/signals/README.md

packages/labs/signals/src/lib/signal-watcher.ts

packages/labs/signals/src/lib/watch.ts

justinfagnani · 2024-09-06T22:27:39Z

@graynorton I had to disable the virtualizer tests here because they wouldn't pass even on multiple test re-runs. This PR doesn't change any core or virtualizer code, nor does it update any relevant dependencies int he package lock. I'm not sure how to move forward there. Any advice would be helpful!

e111077

All my comments at this point are just doc nits. Code looks good to me

packages/labs/signals/README.md

packages/labs/signals/src/lib/signal-watcher.ts

Co-authored-by: Elliott Marquez <5981958+e111077@users.noreply.github.com>

justinfagnani requested a review from sorvell April 10, 2024 16:13

sorvell requested changes Apr 10, 2024

View reviewed changes

e111077 suggested changes Apr 11, 2024

View reviewed changes

hunterloftis mentioned this pull request May 27, 2024

Standard Signals lit/rfcs#39

Merged

[labs/signals] Add @lit-labs/signals package

e3528f6

justinfagnani force-pushed the lit-signals branch from afacc80 to e3528f6 Compare July 31, 2024 20:20

justinfagnani changed the title ~~WIP: Add @lit-labs/signals package~~ Add @lit-labs/signals package Jul 31, 2024

justinfagnani requested review from sorvell and e111077 July 31, 2024 20:20

justinfagnani marked this pull request as ready for review July 31, 2024 20:20

justinfagnani requested a review from kevinpschaaf as a code owner July 31, 2024 20:20

justinfagnani commented Jul 31, 2024

View reviewed changes

packages/labs/signals/src/lib/signal-watcher.ts Outdated Show resolved Hide resolved

packages/labs/signals/src/lib/signal-watcher.ts Outdated Show resolved Hide resolved

justinfagnani commented Jul 31, 2024

View reviewed changes

packages/labs/signals/README.md Show resolved Hide resolved

thepassle reviewed Jul 31, 2024

View reviewed changes

packages/labs/signals/README.md Outdated Show resolved Hide resolved

thepassle reviewed Jul 31, 2024

View reviewed changes

packages/labs/signals/README.md Show resolved Hide resolved

rictic reviewed Jul 31, 2024

View reviewed changes

packages/labs/signals/src/lib/html-tag.ts Show resolved Hide resolved

rictic reviewed Jul 31, 2024

View reviewed changes

packages/lit-html/src/async-directive.ts Outdated Show resolved Hide resolved

Update packages/labs/signals/README.md

79a7a52

Co-authored-by: Pascal Schilp <pascalschilp@gmail.com>

justinfagnani commented Jul 31, 2024

View reviewed changes

packages/labs/signals/src/lib/signal-watcher.ts Outdated Show resolved Hide resolved

Remove spurious comment

3f4b151

sorvell requested changes Aug 1, 2024

View reviewed changes

e111077 suggested changes Aug 1, 2024

View reviewed changes

e111077 reviewed Aug 1, 2024

View reviewed changes

packages/labs/signals/src/lib/signal-watcher.ts Show resolved Hide resolved

packages/labs/signals/src/lib/watch.ts Show resolved Hide resolved

Finish active memory management

95da9e1

e111077 reviewed Aug 30, 2024

View reviewed changes

packages/labs/signals/src/lib/signal-watcher.ts Show resolved Hide resolved

e111077 reviewed Aug 31, 2024

View reviewed changes

packages/labs/signals/LICENSE Outdated Show resolved Hide resolved

packages/labs/signals/README.md Show resolved Hide resolved

Update package lock

c76cf81

justinfagnani added 3 commits September 5, 2024 19:57

Remove virtualizer tests from test:not-starter-kits script

eb2e41f

Update the README

e56e026

Address feedback

df7c2d5

justinfagnani commented Sep 6, 2024

View reviewed changes

justinfagnani requested review from sorvell, thepassle, e111077 and rictic September 6, 2024 22:20

Add missing method declaration

9c50221

e111077 approved these changes Sep 6, 2024

View reviewed changes

justinfagnani added 2 commits September 13, 2024 08:52

Merge branch 'main' into lit-signals

ed21aa4

Re-enable virtualizer tests on CI

8f3c1e8

sorvell approved these changes Sep 22, 2024

View reviewed changes

justinfagnani and others added 7 commits October 1, 2024 15:42

Apply suggestions from code review

7f4d4ba

Co-authored-by: Elliott Marquez <5981958+e111077@users.noreply.github.com>

Address feedback

41b434d

Add test for disconnected element w/ pending update

ee47cc6

Merge branch 'main' into lit-signals

4fb491b

WIP: investigate minification issues

1dfbc38

Merge branch 'main' into lit-signals

8d592d5

Revert rollup-common debug changes

52dd697

justinfagnani merged commit a847717 into main Oct 3, 2024
10 checks passed

justinfagnani deleted the lit-signals branch October 3, 2024 03:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add @lit-labs/signals package #4615

Add @lit-labs/signals package #4615

justinfagnani commented Apr 10, 2024

changeset-bot bot commented Apr 10, 2024 •

edited

Loading

github-actions bot commented Apr 10, 2024 •

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

update-reflect

github-actions bot commented Apr 10, 2024 •

edited

Loading

sorvell left a comment

JosefJezek commented May 7, 2024

justinfagnani commented Jul 31, 2024

justinfagnani commented Jul 31, 2024

justinfagnani commented Jul 31, 2024

justinfagnani commented Aug 30, 2024

e111077 left a comment

justinfagnani commented Sep 5, 2024

justinfagnani commented Sep 6, 2024

justinfagnani left a comment

justinfagnani commented Sep 6, 2024

e111077 left a comment

Add @lit-labs/signals package #4615

Add @lit-labs/signals package #4615

Conversation

justinfagnani commented Apr 10, 2024

changeset-bot bot commented Apr 10, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Apr 10, 2024 • 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

update-reflect

github-actions bot commented Apr 10, 2024 • edited Loading

sorvell left a comment

Choose a reason for hiding this comment

JosefJezek commented May 7, 2024

justinfagnani commented Jul 31, 2024

justinfagnani commented Jul 31, 2024

justinfagnani commented Jul 31, 2024

justinfagnani commented Aug 30, 2024

e111077 left a comment

Choose a reason for hiding this comment

justinfagnani commented Sep 5, 2024

justinfagnani commented Sep 6, 2024

justinfagnani left a comment

Choose a reason for hiding this comment

justinfagnani commented Sep 6, 2024

e111077 left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Apr 10, 2024 •

edited

Loading

github-actions bot commented Apr 10, 2024 •

edited

Loading

github-actions bot commented Apr 10, 2024 •

edited

Loading