fix: add timer to event handler so we can check whether it was attached during the current propagation #4126

JoviDeCroock · 2023-09-03T08:31:54Z

Browsers leave microticks in-between propagating an event up, this can be seen in research performed here. Our first attempt at fixing this was using setTimeout which caused other issues to occur. This fix is a bit different rather than changing our scheduling we place a timestamp on the event-handler so we can check whether the event is appropriate to fire.

This bug can occur when we switch in a dom-node i.e. when we go from

<div><div onClick

to

<div onClick><div

when the inner div changes the dom-structure then we would perform the patch before the event could bubble up.

This is entirely inspired by Vue.js

fixes #3927

You can see this approach working in the following sandbox - due to this being related to browser timings I could not create a test

github-actions · 2023-09-03T08:32:10Z

📊 Tachometer Benchmark Results

Summary

duration

02_replace1k: slower ❌ 0% - 3% (0.61ms - 6.31ms)
^{preact-local vs preact-main}
03_update10th1k_x16: unsure 🔍 -5% - +3% (-1.68ms - +0.90ms)
^{preact-local vs preact-main}
07_create10k: unsure 🔍 -1% - +2% (-12.17ms - +27.51ms)
^{preact-local vs preact-main}
filter_list: unsure 🔍 -1% - +2% (-0.11ms - +0.44ms)
^{preact-local vs preact-main}
hydrate1k: unsure 🔍 -0% - +2% (-0.28ms - +1.79ms)
^{preact-local vs preact-main}
many_updates: unsure 🔍 -1% - +3% (-0.33ms - +0.73ms)
^{preact-local vs preact-main}
text_update: unsure 🔍 -5% - +2% (-0.16ms - +0.08ms)
^{preact-local vs preact-main}
todo: faster ✔ 1% - 2% (0.49ms - 0.91ms)
^{preact-local vs preact-main}

usedJSHeapSize

02_replace1k: slower ❌ 2% - 2% (0.06ms - 0.07ms)
^{preact-local vs preact-main}
03_update10th1k_x16: slower ❌ 2% - 2% (0.06ms - 0.07ms)
^{preact-local vs preact-main}
07_create10k: slower ❌ 2% - 3% (0.63ms - 0.64ms)
^{preact-local vs preact-main}
filter_list: unsure 🔍 -2% - +2% (-0.03ms - +0.03ms)
^{preact-local vs preact-main}
hydrate1k: slower ❌ 2% - 3% (0.11ms - 0.15ms)
^{preact-local vs preact-main}
many_updates: unsure 🔍 -0% - +1% (-0.01ms - +0.04ms)
^{preact-local vs preact-main}
text_update: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
^{preact-local vs preact-main}
todo: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
^{preact-local vs preact-main}

Results

02_replace1k

Browser: chrome-headless 116.0.5845.96
Sample size: 80
Built by: Benchmarks #1180
Commit: 576b888

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	103.89ms - 106.92ms	-	unsure 🔍 -1% - +3% -0.97ms - +3.07ms	unsure 🔍 -2% - +2% -1.97ms - +2.22ms
preact-local	103.03ms - 105.69ms	unsure 🔍 -3% - +1% -3.07ms - +0.97ms	-	unsure 🔍 -3% - +1% -2.89ms - +1.04ms
preact-hooks	103.83ms - 106.73ms	unsure 🔍 -2% - +2% -2.22ms - +1.97ms	unsure 🔍 -1% - +3% -1.04ms - +2.89ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	3.29ms - 3.29ms	-	faster ✔ 2% - 2% 0.05ms - 0.06ms	faster ✔ 2% - 3% 0.08ms - 0.09ms
preact-local	3.35ms - 3.35ms	slower ❌ 2% - 2% 0.05ms - 0.06ms	-	faster ✔ 1% - 1% 0.02ms - 0.03ms
preact-hooks	3.37ms - 3.38ms	slower ❌ 2% - 3% 0.08ms - 0.09ms	slower ❌ 1% - 1% 0.02ms - 0.03ms	-

run-warmup-0

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	42.63ms - 43.80ms	-	unsure 🔍 -3% - +2% -1.14ms - +0.70ms	faster ✔ 3% - 7% 1.39ms - 3.31ms
preact-local	42.73ms - 44.14ms	unsure 🔍 -2% - +3% -0.70ms - +1.14ms	-	faster ✔ 2% - 7% 1.09ms - 3.17ms
preact-hooks	44.81ms - 46.33ms	slower ❌ 3% - 8% 1.39ms - 3.31ms	slower ❌ 2% - 7% 1.09ms - 3.17ms	-

run-warmup-1

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	52.43ms - 54.07ms	-	slower ❌ 4% - 8% 1.81ms - 4.05ms	slower ❌ 1% - 6% 0.50ms - 2.88ms
preact-local	49.56ms - 51.07ms	faster ✔ 3% - 8% 1.81ms - 4.05ms	-	faster ✔ 0% - 5% 0.10ms - 2.39ms
preact-hooks	50.70ms - 52.42ms	faster ✔ 1% - 5% 0.50ms - 2.88ms	slower ❌ 0% - 5% 0.10ms - 2.39ms	-

run-warmup-2

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	36.81ms - 37.81ms	-	faster ✔ 2% - 5% 0.76ms - 2.12ms	faster ✔ 4% - 8% 1.54ms - 3.16ms
preact-local	38.29ms - 39.21ms	slower ❌ 2% - 6% 0.76ms - 2.12ms	-	faster ✔ 0% - 4% 0.12ms - 1.70ms
preact-hooks	39.02ms - 40.30ms	slower ❌ 4% - 9% 1.54ms - 3.16ms	slower ❌ 0% - 4% 0.12ms - 1.70ms	-

run-warmup-3

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	38.70ms - 40.15ms	-	unsure 🔍 -2% - +4% -0.60ms - +1.74ms	unsure 🔍 -6% - +0% -2.25ms - +0.12ms
preact-local	37.94ms - 39.77ms	unsure 🔍 -4% - +2% -1.74ms - +0.60ms	-	faster ✔ 1% - 7% 0.33ms - 2.94ms
preact-hooks	39.56ms - 41.42ms	unsure 🔍 -0% - +6% -0.12ms - +2.25ms	slower ❌ 1% - 8% 0.33ms - 2.94ms	-

run-warmup-4

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	38.44ms - 40.02ms	-	unsure 🔍 -4% - +0% -1.71ms - +0.08ms	faster ✔ 0% - 4% 0.07ms - 1.80ms
preact-local	39.62ms - 40.46ms	unsure 🔍 -0% - +4% -0.08ms - +1.71ms	-	unsure 🔍 -2% - +1% -0.67ms - +0.43ms
preact-hooks	39.81ms - 40.52ms	slower ❌ 0% - 5% 0.07ms - 1.80ms	unsure 🔍 -1% - +2% -0.43ms - +0.67ms	-

run-final

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	27.72ms - 28.72ms	-	unsure 🔍 -3% - +2% -0.87ms - +0.48ms	unsure 🔍 -4% - +1% -1.21ms - +0.19ms
preact-local	27.96ms - 28.87ms	unsure 🔍 -2% - +3% -0.48ms - +0.87ms	-	unsure 🔍 -3% - +1% -0.98ms - +0.36ms
preact-hooks	28.24ms - 29.22ms	unsure 🔍 -1% - +4% -0.19ms - +1.21ms	unsure 🔍 -1% - +3% -0.36ms - +0.98ms	-

03_update10th1k_x16

Browser: chrome-headless 116.0.5845.96
Sample size: 90
Built by: Benchmarks #1180
Commit: 576b888

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	43.16ms - 45.21ms	-	unsure 🔍 -2% - +5% -0.83ms - +2.06ms	unsure 🔍 -1% - +6% -0.33ms - +2.37ms
preact-local	42.55ms - 44.59ms	unsure 🔍 -5% - +2% -2.06ms - +0.83ms	-	unsure 🔍 -2% - +4% -0.95ms - +1.75ms
preact-hooks	42.28ms - 44.05ms	unsure 🔍 -5% - +1% -2.37ms - +0.33ms	unsure 🔍 -4% - +2% -1.75ms - +0.95ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	3.23ms - 3.24ms	-	faster ✔ 2% - 2% 0.06ms - 0.07ms	faster ✔ 2% - 3% 0.08ms - 0.09ms
preact-local	3.30ms - 3.31ms	slower ❌ 2% - 2% 0.06ms - 0.07ms	-	faster ✔ 0% - 1% 0.01ms - 0.02ms
preact-hooks	3.32ms - 3.33ms	slower ❌ 2% - 3% 0.08ms - 0.09ms	slower ❌ 0% - 1% 0.01ms - 0.02ms	-

07_create10k

Browser: chrome-headless 116.0.5845.96
Sample size: 50
Built by: Benchmarks #1180
Commit: 576b888

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	1522.63ms - 1560.43ms	-	unsure 🔍 -2% - +2% -23.29ms - +28.09ms	unsure 🔍 -3% - +0% -49.29ms - +3.38ms
preact-local	1521.72ms - 1556.53ms	unsure 🔍 -2% - +2% -28.09ms - +23.29ms	-	faster ✔ 0% - 3% 0.07ms - 50.64ms
preact-hooks	1546.14ms - 1582.83ms	unsure 🔍 -0% - +3% -3.38ms - +49.29ms	unsure 🔍 -0% - +3% +0.07ms - +50.64ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	25.33ms - 25.34ms	-	faster ✔ 2% - 2% 0.63ms - 0.64ms	faster ✔ 3% - 3% 0.65ms - 0.66ms
preact-local	25.97ms - 25.97ms	slower ❌ 2% - 3% 0.63ms - 0.64ms	-	unsure 🔍 -0% - -0% -0.02ms - -0.02ms
preact-hooks	25.99ms - 25.99ms	slower ❌ 3% - 3% 0.65ms - 0.66ms	unsure 🔍 +0% - +0% +0.02ms - +0.02ms	-

filter_list

Browser: chrome-headless 116.0.5845.96
Sample size: 50
Built by: Benchmarks #1180
Commit: 576b888

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	26.26ms - 27.05ms	-	unsure 🔍 -2% - +2% -0.59ms - +0.52ms	unsure 🔍 -3% - +2% -0.92ms - +0.43ms
preact-local	26.30ms - 27.08ms	unsure 🔍 -2% - +2% -0.52ms - +0.59ms	-	unsure 🔍 -3% - +2% -0.88ms - +0.46ms
preact-hooks	26.36ms - 27.45ms	unsure 🔍 -2% - +3% -0.43ms - +0.92ms	unsure 🔍 -2% - +3% -0.46ms - +0.88ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	1.44ms - 1.48ms	-	unsure 🔍 -2% - +1% -0.04ms - +0.02ms	unsure 🔍 -3% - +1% -0.05ms - +0.01ms
preact-local	1.45ms - 1.49ms	unsure 🔍 -1% - +3% -0.02ms - +0.04ms	-	unsure 🔍 -3% - +1% -0.04ms - +0.02ms
preact-hooks	1.46ms - 1.50ms	unsure 🔍 -1% - +3% -0.01ms - +0.05ms	unsure 🔍 -1% - +3% -0.02ms - +0.04ms	-

hydrate1k

Browser: chrome-headless 116.0.5845.96
Sample size: 60
Built by: Benchmarks #1180
Commit: 576b888

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	154.44ms - 159.98ms	-	unsure 🔍 -5% - +0% -7.84ms - +0.68ms	unsure 🔍 -2% - +3% -2.81ms - +5.30ms
preact-local	157.55ms - 164.02ms	unsure 🔍 -0% - +5% -0.68ms - +7.84ms	-	slower ❌ 0% - 6% 0.44ms - 9.21ms
preact-hooks	153.01ms - 158.92ms	unsure 🔍 -3% - +2% -5.30ms - +2.81ms	faster ✔ 0% - 6% 0.44ms - 9.21ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	5.91ms - 5.94ms	-	faster ✔ 2% - 2% 0.11ms - 0.15ms	faster ✔ 2% - 2% 0.12ms - 0.15ms
preact-local	6.04ms - 6.07ms	slower ❌ 2% - 2% 0.11ms - 0.15ms	-	unsure 🔍 -0% - +0% -0.02ms - +0.01ms
preact-hooks	6.05ms - 6.07ms	slower ❌ 2% - 3% 0.12ms - 0.15ms	unsure 🔍 -0% - +0% -0.01ms - +0.02ms	-

many_updates

Browser: chrome-headless 116.0.5845.96
Sample size: 70
Built by: Benchmarks #1180
Commit: 576b888

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	34.41ms - 36.25ms	-	unsure 🔍 -5% - +2% -1.79ms - +0.72ms	unsure 🔍 -3% - +4% -0.98ms - +1.46ms
preact-local	35.01ms - 36.73ms	unsure 🔍 -2% - +5% -0.72ms - +1.79ms	-	unsure 🔍 -1% - +6% -0.40ms - +1.95ms
preact-hooks	34.29ms - 35.90ms	unsure 🔍 -4% - +3% -1.46ms - +0.98ms	unsure 🔍 -5% - +1% -1.95ms - +0.40ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	4.50ms - 4.53ms	-	unsure 🔍 -0% - +1% -0.02ms - +0.02ms	unsure 🔍 -1% - +0% -0.03ms - +0.01ms
preact-local	4.49ms - 4.53ms	unsure 🔍 -1% - +0% -0.02ms - +0.02ms	-	unsure 🔍 -1% - +0% -0.03ms - +0.01ms
preact-hooks	4.50ms - 4.53ms	unsure 🔍 -0% - +1% -0.01ms - +0.03ms	unsure 🔍 -0% - +1% -0.01ms - +0.03ms	-

text_update

Browser: chrome-headless 116.0.5845.96
Sample size: 220
Built by: Benchmarks #1180
Commit: 576b888

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	2.61ms - 2.66ms	-	unsure 🔍 -2% - +1% -0.04ms - +0.03ms	faster ✔ 5% - 7% 0.13ms - 0.20ms
preact-local	2.62ms - 2.67ms	unsure 🔍 -1% - +2% -0.03ms - +0.04ms	-	faster ✔ 4% - 7% 0.12ms - 0.20ms
preact-hooks	2.77ms - 2.83ms	slower ❌ 5% - 8% 0.13ms - 0.20ms	slower ❌ 4% - 7% 0.12ms - 0.20ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	0.65ms - 0.65ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	faster ✔ 6% - 8% 0.04ms - 0.05ms
preact-local	0.65ms - 0.65ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-	faster ✔ 6% - 8% 0.04ms - 0.05ms
preact-hooks	0.69ms - 0.70ms	slower ❌ 6% - 8% 0.04ms - 0.05ms	slower ❌ 6% - 8% 0.04ms - 0.05ms	-

todo

Browser: chrome-headless 116.0.5845.96
Sample size: 50
Built by: Benchmarks #1180
Commit: 576b888

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	42.83ms - 43.16ms	-	slower ❌ 1% - 3% 0.59ms - 1.12ms	faster ✔ 1% - 2% 0.24ms - 0.90ms
preact-local	41.93ms - 42.35ms	faster ✔ 1% - 3% 0.59ms - 1.12ms	-	faster ✔ 2% - 4% 1.07ms - 1.78ms
preact-hooks	43.28ms - 43.85ms	slower ❌ 1% - 2% 0.24ms - 0.90ms	slower ❌ 3% - 4% 1.07ms - 1.78ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	0.83ms - 0.83ms	-	unsure 🔍 -1% - +0% -0.01ms - +0.00ms	faster ✔ 3% - 3% 0.02ms - 0.02ms
preact-local	0.83ms - 0.84ms	unsure 🔍 -0% - +1% -0.00ms - +0.01ms	-	faster ✔ 1% - 3% 0.01ms - 0.02ms
preact-hooks	0.86ms - 0.86ms	slower ❌ 3% - 3% 0.02ms - 0.02ms	slower ❌ 1% - 3% 0.01ms - 0.02ms	-

_{tachometer-reporter-action v2 for Benchmarks}

…ring the current propagation

github-actions · 2023-09-03T08:33:45Z

Size Change: +260 B (0%)

Total Size: 57.2 kB

Filename	Size	Change
`dist/preact.js`	4.41 kB	+42 B (0%)
`dist/preact.min.js`	4.45 kB	+44 B (0%)
`dist/preact.min.module.js`	4.45 kB	+45 B (1%)
`dist/preact.min.umd.js`	4.47 kB	+42 B (0%)
`dist/preact.module.js`	4.44 kB	+44 B (0%)
`dist/preact.umd.js`	4.48 kB	+43 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`compat/dist/compat.js`	3.95 kB	0 B
`compat/dist/compat.module.js`	3.87 kB	0 B
`compat/dist/compat.umd.js`	4.01 kB	0 B
`debug/dist/debug.js`	3.53 kB	0 B
`debug/dist/debug.module.js`	3.52 kB	0 B
`debug/dist/debug.umd.js`	3.61 kB	0 B
`devtools/dist/devtools.js`	232 B	0 B
`devtools/dist/devtools.module.js`	240 B	0 B
`devtools/dist/devtools.umd.js`	315 B	0 B
`hooks/dist/hooks.js`	1.53 kB	0 B
`hooks/dist/hooks.module.js`	1.56 kB	0 B
`hooks/dist/hooks.umd.js`	1.62 kB	0 B
`jsx-runtime/dist/jsxRuntime.js`	360 B	0 B
`jsx-runtime/dist/jsxRuntime.module.js`	326 B	0 B
`jsx-runtime/dist/jsxRuntime.umd.js`	441 B	0 B
`test-utils/dist/testUtils.js`	453 B	0 B
`test-utils/dist/testUtils.module.js`	454 B	0 B
`test-utils/dist/testUtils.umd.js`	536 B	0 B

_{compressed-size-action}

marvinhagemeister

As discussed in slack I think this is a great fix. Our benchmark setup doesn't account for ticks iirc so it always prefers sync code.

JoviDeCroock requested review from developit, andrewiggins and marvinhagemeister September 3, 2023 08:31

add timer to event handler so we can check whether it was attached du…

30fa4b4

…ring the current propagation

JoviDeCroock force-pushed the fix-event-bug branch from 16e32f4 to 30fa4b4 Compare September 3, 2023 08:33

JoviDeCroock force-pushed the fix-event-bug branch from c701524 to 8cef2f2 Compare September 3, 2023 11:21

improve performance by only applying _dispatched on a bubbling event

3773747

JoviDeCroock force-pushed the fix-event-bug branch from 8cef2f2 to 3773747 Compare September 3, 2023 11:24

remove bubbles

576b888

JoviDeCroock force-pushed the fix-event-bug branch from 7e7ad49 to 576b888 Compare September 3, 2023 11:26

marvinhagemeister approved these changes Sep 3, 2023

View reviewed changes

JoviDeCroock merged commit 29abdf7 into main Sep 3, 2023
12 of 13 checks passed

JoviDeCroock deleted the fix-event-bug branch September 3, 2023 14:25

JoviDeCroock mentioned this pull request Sep 28, 2023

10.18.0 #4143

Merged

jramanat mentioned this pull request Oct 19, 2023

Event bubbling can break in test conditions #4161

Closed

1 task

JoviDeCroock mentioned this pull request Dec 15, 2023

Fix event bubbling can break in test conditions (#4161) #4232

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add timer to event handler so we can check whether it was attached during the current propagation #4126

fix: add timer to event handler so we can check whether it was attached during the current propagation #4126

JoviDeCroock commented Sep 3, 2023 •

edited

Loading

github-actions bot commented Sep 3, 2023 •

edited

Loading

📊 Tachometer Benchmark Results

Summary

duration

usedJSHeapSize

Results

duration

usedJSHeapSize

run-warmup-0

run-warmup-1

run-warmup-2

run-warmup-3

run-warmup-4

run-final

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

github-actions bot commented Sep 3, 2023 •

edited

Loading

marvinhagemeister left a comment

fix: add timer to event handler so we can check whether it was attached during the current propagation #4126

fix: add timer to event handler so we can check whether it was attached during the current propagation #4126

Conversation

JoviDeCroock commented Sep 3, 2023 • edited Loading

github-actions bot commented Sep 3, 2023 • edited Loading

📊 Tachometer Benchmark Results

Summary

duration

usedJSHeapSize

Results

duration

usedJSHeapSize

run-warmup-0

run-warmup-1

run-warmup-2

run-warmup-3

run-warmup-4

run-final

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

github-actions bot commented Sep 3, 2023 • edited Loading

marvinhagemeister left a comment

Choose a reason for hiding this comment

JoviDeCroock commented Sep 3, 2023 •

edited

Loading

github-actions bot commented Sep 3, 2023 •

edited

Loading

github-actions bot commented Sep 3, 2023 •

edited

Loading