[labs/task] Add pluggable task args equality functions by justinfagnani · Pull Request #4013 · lit/lit

justinfagnani · 2023-07-11T23:01:39Z

Adds argsEqual config option
Removes shouldRun() and performTask() protected methods
Adds shallowArrayEquals, deepArrayEquals, and deepEquals functions

The deepEquals() is inspired by fast-deep-equals, but rewritten to use modern JS, Object.is(), and have a different ordering of cases that should better match expected data (Dates would be more likely than RegExps). I did not yet include Map and Set comparison. I didn't find a nice deep equals package to depend on that fit our needs.

Talking to @rictic I think we may also want to generalize args to just "data" and not assume it's array, but this can be a step towards that with the array-assuming equality functions.

changeset-bot · 2023-07-11T23:01:42Z

🦋 Changeset detected

Latest commit: 313019f

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/task	Major

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-07-11T23:02:06Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -12% - +6% (-3.23ms - +1.61ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -5% - +11% (-1.07ms - +2.18ms)
^{this-change vs tip-of-tree}

render

lit-element-list: 77.03ms - 80.17ms
lit-html-kitchen-sink: slower ❌ 3% - 20% (1.15ms - 6.80ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -5% - +14% (-0.58ms - +1.67ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -4% - +0% (-2.58ms - +0.31ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -3% - +1% (-1.56ms - +0.54ms)
^{this-change vs tip-of-tree}
this-change: 71.95ms - 76.02ms
this-change, tip-of-tree, previous-release: unsure 🔍 -3% - +7% (-0.89ms - +1.93ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +3% (-0.38ms - +1.73ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +3% (-1.05ms - +1.44ms)
^{this-change vs tip-of-tree}

update

lit-element-list: 887.66ms - 900.25ms
lit-html-kitchen-sink: unsure 🔍 -6% - +1% (-5.58ms - +1.41ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -13% - +9% (-50.93ms - +35.01ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +1% (-2.60ms - +1.32ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +1% (-7.72ms - +10.96ms)
^{this-change vs tip-of-tree}
this-change: 821.49ms - 837.42ms
this-change, tip-of-tree, previous-release: unsure 🔍 -6% - +5% (-5.24ms - +3.89ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -3% - +2% (-3.55ms - +2.95ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -0% - +2% (-0.41ms - +18.32ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: 832.90ms - 843.81ms
reactive-element-list: unsure 🔍 -1% - +1% (-8.43ms - +8.78ms)
^{this-change vs tip-of-tree}
this-change: 788.54ms - 800.91ms
this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-7.12ms - +7.52ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 114.0.5735.198
Sample size: 50
Built by: Benchmarks #5035
Commit: 2bb29b6

render

Version	Avg time	vs
	77.03ms - 80.17ms	-

update

Version	Avg time	vs
	887.66ms - 900.25ms	-

update-reflect

Version	Avg time	vs
	832.90ms - 843.81ms	-

lit-html-kitchen-sink

Browser: chrome-headless 114.0.5735.198
Sample size: 30
Built by: Benchmarks #5035
Commit: 2bb29b6

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	35.92ms - 40.06ms	-	slower ❌ 3% - 20% 1.15ms - 6.80ms	unsure 🔍 -1% - +17% -0.29ms - +5.76ms
tip-of-tree tip-of-tree	32.10ms - 35.94ms	faster ✔ 3% - 17% 1.15ms - 6.80ms	-	unsure 🔍 -12% - +5% -4.17ms - +1.69ms
previous-release previous-release	33.05ms - 37.46ms	unsure 🔍 -15% - +1% -5.76ms - +0.29ms	unsure 🔍 -5% - +12% -1.69ms - +4.17ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	90.51ms - 94.75ms	-	unsure 🔍 -6% - +1% -5.58ms - +1.41ms	unsure 🔍 -7% - +1% -6.35ms - +0.79ms
tip-of-tree tip-of-tree	91.94ms - 97.49ms	unsure 🔍 -2% - +6% -1.41ms - +5.58ms	-	unsure 🔍 -5% - +3% -4.69ms - +3.30ms
previous-release previous-release	92.54ms - 98.28ms	unsure 🔍 -1% - +7% -0.79ms - +6.35ms	unsure 🔍 -3% - +5% -3.30ms - +4.69ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	24.69ms - 27.90ms	-	unsure 🔍 -12% - +6% -3.23ms - +1.61ms	unsure 🔍 -9% - +8% -2.34ms - +2.10ms
tip-of-tree tip-of-tree	25.30ms - 28.92ms	unsure 🔍 -6% - +12% -1.61ms - +3.23ms	-	unsure 🔍 -6% - +12% -1.67ms - +3.06ms
previous-release previous-release	24.89ms - 27.95ms	unsure 🔍 -8% - +9% -2.10ms - +2.34ms	unsure 🔍 -11% - +6% -3.06ms - +1.67ms	-

lit-html-repeat

Browser: chrome-headless 114.0.5735.198
Sample size: 30
Built by: Benchmarks #5035
Commit: 2bb29b6

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	11.05ms - 13.26ms	-	unsure 🔍 -5% - +14% -0.58ms - +1.67ms	unsure 🔍 -5% - +15% -0.57ms - +1.69ms
tip-of-tree tip-of-tree	11.38ms - 11.84ms	unsure 🔍 -13% - +4% -1.67ms - +0.58ms	-	unsure 🔍 -3% - +3% -0.32ms - +0.35ms
previous-release previous-release	11.34ms - 11.84ms	unsure 🔍 -14% - +4% -1.69ms - +0.57ms	unsure 🔍 -3% - +3% -0.35ms - +0.32ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	356.31ms - 413.90ms	-	unsure 🔍 -13% - +9% -50.93ms - +35.01ms	unsure 🔍 -11% - +10% -42.03ms - +39.81ms
tip-of-tree tip-of-tree	361.16ms - 424.96ms	unsure 🔍 -9% - +13% -35.01ms - +50.93ms	-	unsure 🔍 -9% - +13% -36.31ms - +50.00ms
previous-release previous-release	357.14ms - 415.29ms	unsure 🔍 -10% - +11% -39.81ms - +42.03ms	unsure 🔍 -13% - +9% -50.00ms - +36.31ms	-

lit-html-template-heavy

Browser: chrome-headless 114.0.5735.198
Sample size: 50
Built by: Benchmarks #5035
Commit: 2bb29b6

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	59.04ms - 60.85ms	-	unsure 🔍 -4% - +0% -2.58ms - +0.31ms	unsure 🔍 -2% - +2% -1.45ms - +1.02ms
tip-of-tree tip-of-tree	59.95ms - 62.21ms	unsure 🔍 -1% - +4% -0.31ms - +2.58ms	-	unsure 🔍 -1% - +4% -0.48ms - +2.33ms
previous-release previous-release	59.32ms - 61.00ms	unsure 🔍 -2% - +2% -1.02ms - +1.45ms	unsure 🔍 -4% - +1% -2.33ms - +0.48ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	130.95ms - 133.96ms	-	unsure 🔍 -2% - +1% -2.60ms - +1.32ms	unsure 🔍 -3% - +1% -3.37ms - +0.97ms
tip-of-tree tip-of-tree	131.84ms - 134.35ms	unsure 🔍 -1% - +2% -1.32ms - +2.60ms	-	unsure 🔍 -2% - +1% -2.56ms - +1.44ms
previous-release previous-release	132.10ms - 135.21ms	unsure 🔍 -1% - +3% -0.97ms - +3.37ms	unsure 🔍 -1% - +2% -1.44ms - +2.56ms	-

reactive-element-list

Browser: chrome-headless 114.0.5735.198
Sample size: 50
Built by: Benchmarks #5035
Commit: 2bb29b6

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	50.11ms - 51.36ms	-	unsure 🔍 -3% - +1% -1.56ms - +0.54ms	unsure 🔍 -2% - +2% -1.04ms - +0.85ms
tip-of-tree tip-of-tree	50.40ms - 52.10ms	unsure 🔍 -1% - +3% -0.54ms - +1.56ms	-	unsure 🔍 -1% - +3% -0.69ms - +1.52ms
previous-release previous-release	50.12ms - 51.54ms	unsure 🔍 -2% - +2% -0.85ms - +1.04ms	unsure 🔍 -3% - +1% -1.52ms - +0.69ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	909.23ms - 922.93ms	-	unsure 🔍 -1% - +1% -7.72ms - +10.96ms	unsure 🔍 -1% - +1% -12.17ms - +6.62ms
tip-of-tree tip-of-tree	908.12ms - 920.81ms	unsure 🔍 -1% - +1% -10.96ms - +7.72ms	-	unsure 🔍 -1% - +1% -13.42ms - +4.63ms
previous-release previous-release	912.43ms - 925.28ms	unsure 🔍 -1% - +1% -6.62ms - +12.17ms	unsure 🔍 -1% - +1% -4.63ms - +13.42ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	878.27ms - 889.69ms	-	unsure 🔍 -1% - +1% -8.43ms - +8.78ms	unsure 🔍 -1% - +1% -8.80ms - +8.59ms
tip-of-tree tip-of-tree	877.37ms - 890.24ms	unsure 🔍 -1% - +1% -8.78ms - +8.43ms	-	unsure 🔍 -1% - +1% -9.47ms - +8.91ms
previous-release previous-release	877.53ms - 890.65ms	unsure 🔍 -1% - +1% -8.59ms - +8.80ms	unsure 🔍 -1% - +1% -8.91ms - +9.47ms	-

this-change

Browser: chrome-headless 115.0.5790.102
Sample size: 50
Built by: Benchmarks #5120
Commit: 313019f

render

Version	Avg time	vs
	71.95ms - 76.02ms	-

update

Version	Avg time	vs
	821.49ms - 837.42ms	-

update-reflect

Version	Avg time	vs
	788.54ms - 800.91ms	-

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

Browser: chrome-headless 115.0.5790.102
Sample size: 30
Built by: Benchmarks #5120
Commit: 313019f

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	28.64ms - 30.70ms	-	unsure 🔍 -3% - +7% -0.89ms - +1.93ms	unsure 🔍 -5% - +6% -1.46ms - +1.82ms
tip-of-tree tip-of-tree	28.19ms - 30.11ms	unsure 🔍 -6% - +3% -1.93ms - +0.89ms	-	unsure 🔍 -7% - +4% -1.94ms - +1.25ms
previous-release previous-release	28.22ms - 30.77ms	unsure 🔍 -6% - +5% -1.82ms - +1.46ms	unsure 🔍 -4% - +7% -1.25ms - +1.94ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	78.05ms - 83.47ms	-	unsure 🔍 -6% - +5% -5.24ms - +3.89ms	unsure 🔍 -7% - +2% -6.21ms - +2.04ms
tip-of-tree tip-of-tree	77.76ms - 85.11ms	unsure 🔍 -5% - +7% -3.89ms - +5.24ms	-	unsure 🔍 -7% - +4% -6.22ms - +3.41ms
previous-release previous-release	79.73ms - 85.96ms	unsure 🔍 -3% - +8% -2.04ms - +6.21ms	unsure 🔍 -4% - +8% -3.41ms - +6.22ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	20.24ms - 22.65ms	-	unsure 🔍 -5% - +11% -1.07ms - +2.18ms	unsure 🔍 -8% - +5% -1.80ms - +1.16ms
tip-of-tree tip-of-tree	19.80ms - 21.98ms	unsure 🔍 -10% - +5% -2.18ms - +1.07ms	-	unsure 🔍 -10% - +2% -2.26ms - +0.51ms
previous-release previous-release	20.91ms - 22.62ms	unsure 🔍 -5% - +8% -1.16ms - +1.80ms	unsure 🔍 -3% - +11% -0.51ms - +2.26ms	-

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

Browser: chrome-headless 115.0.5790.102
Sample size: 50
Built by: Benchmarks #5120
Commit: 313019f

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	52.66ms - 54.30ms	-	unsure 🔍 -1% - +3% -0.38ms - +1.73ms	unsure 🔍 -6% - +0% -3.12ms - +0.25ms
tip-of-tree tip-of-tree	52.13ms - 53.47ms	unsure 🔍 -3% - +1% -1.73ms - +0.38ms	-	faster ✔ 1% - 7% 0.50ms - 3.73ms
previous-release previous-release	53.44ms - 56.38ms	unsure 🔍 -0% - +6% -0.25ms - +3.12ms	slower ❌ 1% - 7% 0.50ms - 3.73ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	118.91ms - 124.17ms	-	unsure 🔍 -3% - +2% -3.55ms - +2.95ms	unsure 🔍 -4% - +2% -5.04ms - +2.23ms
tip-of-tree tip-of-tree	119.94ms - 123.75ms	unsure 🔍 -2% - +3% -2.95ms - +3.55ms	-	unsure 🔍 -3% - +2% -4.25ms - +2.04ms
previous-release previous-release	120.45ms - 125.45ms	unsure 🔍 -2% - +4% -2.23ms - +5.04ms	unsure 🔍 -2% - +4% -2.04ms - +4.25ms	-

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

Browser: chrome-headless 115.0.5790.102
Sample size: 50
Built by: Benchmarks #5120
Commit: 313019f

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	46.08ms - 47.68ms	-	unsure 🔍 -2% - +3% -1.05ms - +1.44ms	unsure 🔍 -0% - +4% -0.22ms - +1.84ms
tip-of-tree tip-of-tree	45.73ms - 47.63ms	unsure 🔍 -3% - +2% -1.44ms - +1.05ms	-	unsure 🔍 -1% - +4% -0.53ms - +1.77ms
previous-release previous-release	45.42ms - 46.72ms	unsure 🔍 -4% - +0% -1.84ms - +0.22ms	unsure 🔍 -4% - +1% -1.77ms - +0.53ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	838.24ms - 852.26ms	-	unsure 🔍 -0% - +2% -0.41ms - +18.32ms	unsure 🔍 -0% - +2% -0.89ms - +17.90ms
tip-of-tree tip-of-tree	830.09ms - 842.50ms	unsure 🔍 -2% - +0% -18.32ms - +0.41ms	-	unsure 🔍 -1% - +1% -9.25ms - +8.35ms
previous-release previous-release	830.50ms - 842.99ms	unsure 🔍 -2% - +0% -17.90ms - +0.89ms	unsure 🔍 -1% - +1% -8.35ms - +9.25ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	833.35ms - 843.85ms	-	unsure 🔍 -1% - +1% -7.12ms - +7.52ms	unsure 🔍 -2% - +0% -13.21ms - +3.03ms
tip-of-tree tip-of-tree	833.30ms - 843.49ms	unsure 🔍 -1% - +1% -7.52ms - +7.12ms	-	unsure 🔍 -2% - +0% -13.31ms - +2.73ms
previous-release previous-release	837.50ms - 849.88ms	unsure 🔍 -0% - +2% -3.03ms - +13.21ms	unsure 🔍 -0% - +2% -2.73ms - +13.31ms	-

_{tachometer-reporter-action v2 for Benchmarks}

packages/labs/task/src/task.ts

packages/labs/task/src/test/task_test.ts

rictic · 2023-07-11T23:30:19Z

packages/labs/task/src/task.ts

+const {keys: objectKeys} = Object;
+const {isArray} = Array;
+
+export const deepEquals = (a: unknown, b: unknown): boolean => {


Deep equality checks require a number of opinionated pieces. Do we want to maintain this? Do we want to ship these bytes, or are we assuming production builds are tree shaked?

Yes they do, but... it's quite hard to find a minimally good deep equals, especially that's shipped as JS modules. I think including some basic one is just good DX.

I do need to document the opinions here:

Object.is for primitives

Constructor check

Arrays

valueOf() and toString() checks

Map and Set (whichever way we go)

Own property requirement for objects

Cycles (currently we don't detect them and just iloop, which a lot of libraries do, but some don't)

WDYT about putting this in its own module?

packages/labs/task/src/task.ts

packages/labs/task/src/test/deep-equals_test.ts

packages/labs/task/src/test/task_test.ts

rictic · 2023-07-12T22:32:03Z

packages/labs/task/src/task.ts

+const {keys: objectKeys} = Object;
+const {isArray} = Array;
+
+export const deepEquals = (a: unknown, b: unknown): boolean => {


WDYT about putting this in its own module?

rictic · 2023-07-12T22:41:18Z

packages/labs/task/src/test/task_test.ts

+  // TODO: the name of this function is confusing. It doesn't wait for the task
+  // to complete. It just waits a rAF. That's why it can be used to check that
+  // a task is pending.
  const tasksUpdateComplete = nextFrame;


Good point. I'd consider dropping this in favor of just directly using nextFrame

justinfagnani · 2023-07-29T01:19:31Z

I'm a little stuck on the tests right now. Browser can't seem to load the new deep-equals test module. But I ran both the dev and prod tests in a live Sauce session and they all load and pass. The GitHub actions logs are giving nothing more than a "failed to load module" error. No browser logs that say what the specific error is.

edit: I read the logs wrong and this is happening in all browsers. This made me look for a Wireit, package.json, or Rollup config error for the new entry-point, but I can't see one yet.

Add pluggable args equality functions and deepArrayEquals

4debd41

justinfagnani requested a review from rictic July 11, 2023 23:01

justinfagnani requested a review from kevinpschaaf as a code owner July 11, 2023 23:01

rictic reviewed Jul 11, 2023

View reviewed changes

justinfagnani commented Jul 12, 2023

View reviewed changes

packages/labs/task/src/task.ts Show resolved Hide resolved

packages/labs/task/src/task.ts Outdated Show resolved Hide resolved

packages/labs/task/src/task.ts Outdated Show resolved Hide resolved

justinfagnani commented Jul 12, 2023

View reviewed changes

packages/labs/task/src/test/deep-equals_test.ts Show resolved Hide resolved

packages/labs/task/src/test/task_test.ts Show resolved Hide resolved

justinfagnani added 2 commits July 12, 2023 14:01

Add Map and Set support

99b5e3a

Address feedback

a168818

justinfagnani requested a review from rictic July 12, 2023 21:15

rictic approved these changes Jul 12, 2023

View reviewed changes

justinfagnani added 2 commits July 28, 2023 17:09

Merge branch 'main' into task-args-equality

a475b1e

Make separate deep-equals.js module

89e82a6

justinfagnani added 2 commits July 31, 2023 11:35

Update copyright

a3a1c9e

Fix import in test

313019f

justinfagnani merged commit a5bf250 into main Jul 31, 2023

justinfagnani deleted the task-args-equality branch July 31, 2023 18:54

AndrewJakubowicz mentioned this pull request Jul 31, 2023

[infra] improve error message for test:common #4059

Merged

lit-robot mentioned this pull request Aug 2, 2023

Version Packages #4066

Merged

Conversation

justinfagnani commented Jul 11, 2023

Uh oh!

changeset-bot bot commented Jul 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

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

render

update

update-reflect

render

update

nop-update

render

update

render

update

update-reflect

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rictic Jul 11, 2023

Choose a reason for hiding this comment

Uh oh!

justinfagnani Jul 12, 2023

Choose a reason for hiding this comment

Uh oh!

rictic Jul 12, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rictic Jul 12, 2023

Choose a reason for hiding this comment

Uh oh!

rictic Jul 12, 2023

Choose a reason for hiding this comment

Uh oh!

justinfagnani commented Jul 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented Jul 11, 2023 •

edited

Loading

github-actions bot commented Jul 11, 2023 •

edited

Loading

justinfagnani commented Jul 29, 2023 •

edited

Loading