fix: setting ref to null after updating it with new element by JoviDeCroock · Pull Request #4054 · preactjs/preact

JoviDeCroock · 2023-06-28T09:41:32Z

As explained in #4049 (comment) we did all unsetting/setting logic during diff which meant that if we changed the level of an element we would set --> unset if the level moved lower down.

To counter-act this we execute unsetting first and setting later, which is also inline with how react does it in React you see a series of unsets and only after you see a series of sets.

Because we queue useLayoutEffect to the commit phase we still ensures that refs are set correctly.

…or post diff

github-actions · 2023-06-28T09:41:48Z

📊 Tachometer Benchmark Results

Summary

duration

02_replace1k: unsure 🔍 -2% - +2% (-2.76ms - +2.30ms)
^{preact-local vs preact-master}
03_update10th1k_x16: unsure 🔍 -3% - +4% (-1.17ms - +1.48ms)
^{preact-local vs preact-master}
07_create10k: unsure 🔍 -1% - +1% (-10.96ms - +8.75ms)
^{preact-local vs preact-master}
filter_list: unsure 🔍 -4% - +2% (-0.98ms - +0.52ms)
^{preact-local vs preact-master}
hydrate1k: unsure 🔍 -1% - +1% (-1.22ms - +1.14ms)
^{preact-local vs preact-master}
many_updates: unsure 🔍 -2% - +3% (-0.51ms - +0.60ms)
^{preact-local vs preact-master}
text_update: unsure 🔍 -3% - +4% (-0.11ms - +0.14ms)
^{preact-local vs preact-master}
todo: unsure 🔍 -1% - +2% (-0.64ms - +0.99ms)
^{preact-local vs preact-master}

usedJSHeapSize

02_replace1k: unsure 🔍 -1% - +0% (-0.02ms - +0.01ms)
^{preact-local vs preact-master}
03_update10th1k_x16: unsure 🔍 -0% - +0% (-0.00ms - +0.01ms)
^{preact-local vs preact-master}
07_create10k: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
^{preact-local vs preact-master}
filter_list: unsure 🔍 -0% - +3% (-0.01ms - +0.04ms)
^{preact-local vs preact-master}
hydrate1k: unsure 🔍 -0% - +1% (-0.02ms - +0.04ms)
^{preact-local vs preact-master}
many_updates: unsure 🔍 -1% - +0% (-0.05ms - +0.02ms)
^{preact-local vs preact-master}
text_update: unsure 🔍 -1% - +1% (-0.00ms - +0.00ms)
^{preact-local vs preact-master}
todo: unsure 🔍 -1% - +0% (-0.01ms - +0.00ms)
^{preact-local vs preact-master}

Results

02_replace1k

Browser: chrome-headless 114.0.5735.133
Sample size: 70
Built by: Benchmarks #1135
Commit: 1f4e464

duration

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	137.22ms - 140.83ms	-	unsure 🔍 -2% - +2% -2.30ms - +2.76ms	unsure 🔍 -2% - +2% -2.84ms - +2.51ms
preact-local	137.02ms - 140.56ms	unsure 🔍 -2% - +2% -2.76ms - +2.30ms	-	unsure 🔍 -2% - +2% -3.04ms - +2.25ms
preact-hooks	137.22ms - 141.16ms	unsure 🔍 -2% - +2% -2.51ms - +2.84ms	unsure 🔍 -2% - +2% -2.25ms - +3.04ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	3.28ms - 3.30ms	-	unsure 🔍 -0% - +1% -0.01ms - +0.02ms	faster ✔ 0% - 1% 0.00ms - 0.02ms
preact-local	3.28ms - 3.30ms	unsure 🔍 -1% - +0% -0.02ms - +0.01ms	-	faster ✔ 0% - 1% 0.01ms - 0.02ms
preact-hooks	3.30ms - 3.31ms	slower ❌ 0% - 1% 0.00ms - 0.02ms	slower ❌ 0% - 1% 0.01ms - 0.02ms	-

run-warmup-0

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	46.94ms - 48.80ms	-	unsure 🔍 -3% - +3% -1.48ms - +1.34ms	unsure 🔍 -3% - +2% -1.43ms - +1.14ms
preact-local	46.88ms - 49.00ms	unsure 🔍 -3% - +3% -1.34ms - +1.48ms	-	unsure 🔍 -3% - +3% -1.46ms - +1.30ms
preact-hooks	47.13ms - 48.90ms	unsure 🔍 -2% - +3% -1.14ms - +1.43ms	unsure 🔍 -3% - +3% -1.30ms - +1.46ms	-

run-warmup-1

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	62.17ms - 64.46ms	-	unsure 🔍 -4% - +1% -2.29ms - +0.89ms	unsure 🔍 -5% - +1% -3.33ms - +0.41ms
preact-local	62.91ms - 65.13ms	unsure 🔍 -1% - +4% -0.89ms - +2.29ms	-	unsure 🔍 -4% - +2% -2.61ms - +1.09ms
preact-hooks	63.30ms - 66.26ms	unsure 🔍 -1% - +5% -0.41ms - +3.33ms	unsure 🔍 -2% - +4% -1.09ms - +2.61ms	-

run-warmup-2

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	52.65ms - 56.34ms	-	unsure 🔍 -5% - +5% -2.71ms - +2.66ms	unsure 🔍 -2% - +7% -1.29ms - +3.73ms
preact-local	52.57ms - 56.48ms	unsure 🔍 -5% - +5% -2.66ms - +2.71ms	-	unsure 🔍 -3% - +7% -1.35ms - +3.84ms
preact-hooks	51.57ms - 54.98ms	unsure 🔍 -7% - +2% -3.73ms - +1.29ms	unsure 🔍 -7% - +2% -3.84ms - +1.35ms	-

run-warmup-3

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	43.08ms - 46.53ms	-	unsure 🔍 -4% - +8% -1.55ms - +3.33ms	unsure 🔍 -8% - +3% -3.52ms - +1.47ms
preact-local	42.19ms - 45.65ms	unsure 🔍 -7% - +3% -3.33ms - +1.55ms	-	unsure 🔍 -10% - +1% -4.41ms - +0.59ms
preact-hooks	44.02ms - 47.63ms	unsure 🔍 -3% - +8% -1.47ms - +3.52ms	unsure 🔍 -1% - +10% -0.59ms - +4.41ms	-

run-warmup-4

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	51.99ms - 54.69ms	-	unsure 🔍 -6% - +1% -3.22ms - +0.83ms	unsure 🔍 -6% - +1% -3.21ms - +0.58ms
preact-local	53.03ms - 56.05ms	unsure 🔍 -2% - +6% -0.83ms - +3.22ms	-	unsure 🔍 -4% - +3% -2.13ms - +1.89ms
preact-hooks	53.33ms - 55.99ms	unsure 🔍 -1% - +6% -0.58ms - +3.21ms	unsure 🔍 -3% - +4% -1.89ms - +2.13ms	-

run-final

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	35.77ms - 37.51ms	-	unsure 🔍 -3% - +4% -1.05ms - +1.34ms	unsure 🔍 -4% - +3% -1.58ms - +0.99ms
preact-local	35.67ms - 37.32ms	unsure 🔍 -4% - +3% -1.34ms - +1.05ms	-	unsure 🔍 -5% - +2% -1.70ms - +0.82ms
preact-hooks	35.99ms - 37.89ms	unsure 🔍 -3% - +4% -0.99ms - +1.58ms	unsure 🔍 -2% - +5% -0.82ms - +1.70ms	-

03_update10th1k_x16

Browser: chrome-headless 114.0.5735.133
Sample size: 100
Built by: Benchmarks #1135
Commit: 1f4e464

duration

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	34.64ms - 36.47ms	-	unsure 🔍 -4% - +3% -1.48ms - +1.17ms	faster ✔ 1% - 8% 0.38ms - 3.08ms
preact-local	34.75ms - 36.67ms	unsure 🔍 -3% - +4% -1.17ms - +1.48ms	-	faster ✔ 1% - 8% 0.19ms - 2.96ms
preact-hooks	36.29ms - 38.28ms	slower ❌ 1% - 9% 0.38ms - 3.08ms	slower ❌ 0% - 8% 0.19ms - 2.96ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	3.23ms - 3.23ms	-	unsure 🔍 -0% - +0% -0.01ms - +0.00ms	faster ✔ 1% - 1% 0.03ms - 0.04ms
preact-local	3.23ms - 3.24ms	unsure 🔍 -0% - +0% -0.00ms - +0.01ms	-	faster ✔ 1% - 1% 0.02ms - 0.04ms
preact-hooks	3.26ms - 3.27ms	slower ❌ 1% - 1% 0.03ms - 0.04ms	slower ❌ 1% - 1% 0.02ms - 0.04ms	-

07_create10k

Browser: chrome-headless 114.0.5735.133
Sample size: 50
Built by: Benchmarks #1135
Commit: 1f4e464

duration

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	1292.71ms - 1305.87ms	-	unsure 🔍 -1% - +1% -8.75ms - +10.96ms	unsure 🔍 -1% - +1% -12.22ms - +8.22ms
preact-local	1290.85ms - 1305.52ms	unsure 🔍 -1% - +1% -10.96ms - +8.75ms	-	unsure 🔍 -1% - +1% -13.82ms - +7.62ms
preact-hooks	1293.46ms - 1309.10ms	unsure 🔍 -1% - +1% -8.22ms - +12.22ms	unsure 🔍 -1% - +1% -7.62ms - +13.82ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	25.32ms - 25.32ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	unsure 🔍 -0% - -0% -0.02ms - -0.02ms
preact-local	25.32ms - 25.32ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-	unsure 🔍 -0% - -0% -0.02ms - -0.02ms
preact-hooks	25.34ms - 25.34ms	unsure 🔍 +0% - +0% +0.02ms - +0.02ms	unsure 🔍 +0% - +0% +0.02ms - +0.02ms	-

filter_list

Browser: chrome-headless 114.0.5735.133
Sample size: 50
Built by: Benchmarks #1135
Commit: 1f4e464

duration

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	23.42ms - 24.81ms	-	unsure 🔍 -2% - +4% -0.52ms - +0.98ms	unsure 🔍 -3% - +4% -0.60ms - +0.94ms
preact-local	23.59ms - 24.18ms	unsure 🔍 -4% - +2% -0.98ms - +0.52ms	-	unsure 🔍 -2% - +2% -0.52ms - +0.39ms
preact-hooks	23.60ms - 24.29ms	unsure 🔍 -4% - +2% -0.94ms - +0.60ms	unsure 🔍 -2% - +2% -0.39ms - +0.52ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	1.39ms - 1.41ms	-	unsure 🔍 -3% - +0% -0.04ms - +0.01ms	faster ✔ 1% - 3% 0.01ms - 0.05ms
preact-local	1.40ms - 1.43ms	unsure 🔍 -0% - +3% -0.01ms - +0.04ms	-	unsure 🔍 -3% - +1% -0.04ms - +0.01ms
preact-hooks	1.42ms - 1.45ms	slower ❌ 1% - 4% 0.01ms - 0.05ms	unsure 🔍 -1% - +3% -0.01ms - +0.04ms	-

hydrate1k

Browser: chrome-headless 114.0.5735.133
Sample size: 50
Built by: Benchmarks #1135
Commit: 1f4e464

duration

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	110.16ms - 112.12ms	-	unsure 🔍 -1% - +1% -1.14ms - +1.22ms	unsure 🔍 -1% - +1% -1.60ms - +0.96ms
preact-local	110.45ms - 111.75ms	unsure 🔍 -1% - +1% -1.22ms - +1.14ms	-	unsure 🔍 -1% - +1% -1.41ms - +0.69ms
preact-hooks	110.63ms - 112.28ms	unsure 🔍 -1% - +1% -0.96ms - +1.60ms	unsure 🔍 -1% - +1% -0.69ms - +1.41ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	5.96ms - 6.00ms	-	unsure 🔍 -1% - +0% -0.04ms - +0.02ms	unsure 🔍 -1% - +0% -0.05ms - +0.02ms
preact-local	5.97ms - 6.01ms	unsure 🔍 -0% - +1% -0.02ms - +0.04ms	-	unsure 🔍 -1% - +0% -0.03ms - +0.03ms
preact-hooks	5.97ms - 6.01ms	unsure 🔍 -0% - +1% -0.02ms - +0.05ms	unsure 🔍 -0% - +1% -0.03ms - +0.03ms	-

many_updates

Browser: chrome-headless 114.0.5735.133
Sample size: 50
Built by: Benchmarks #1135
Commit: 1f4e464

duration

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	20.51ms - 21.26ms	-	unsure 🔍 -3% - +2% -0.60ms - +0.51ms	unsure 🔍 -2% - +2% -0.41ms - +0.48ms
preact-local	20.52ms - 21.34ms	unsure 🔍 -2% - +3% -0.51ms - +0.60ms	-	unsure 🔍 -2% - +3% -0.40ms - +0.56ms
preact-hooks	20.60ms - 21.09ms	unsure 🔍 -2% - +2% -0.48ms - +0.41ms	unsure 🔍 -3% - +2% -0.56ms - +0.40ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	4.46ms - 4.51ms	-	unsure 🔍 -0% - +1% -0.02ms - +0.05ms	unsure 🔍 -1% - +1% -0.04ms - +0.03ms
preact-local	4.45ms - 4.49ms	unsure 🔍 -1% - +0% -0.05ms - +0.02ms	-	unsure 🔍 -1% - +0% -0.05ms - +0.01ms
preact-hooks	4.47ms - 4.51ms	unsure 🔍 -1% - +1% -0.03ms - +0.04ms	unsure 🔍 -0% - +1% -0.01ms - +0.05ms	-

text_update

Browser: chrome-headless 114.0.5735.133
Sample size: 180
Built by: Benchmarks #1135
Commit: 1f4e464

duration

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	3.34ms - 3.53ms	-	unsure 🔍 -4% - +3% -0.14ms - +0.11ms	faster ✔ 4% - 11% 0.16ms - 0.41ms
preact-local	3.36ms - 3.54ms	unsure 🔍 -3% - +4% -0.11ms - +0.14ms	-	faster ✔ 4% - 10% 0.15ms - 0.39ms
preact-hooks	3.64ms - 3.80ms	slower ❌ 4% - 12% 0.16ms - 0.41ms	slower ❌ 4% - 12% 0.15ms - 0.39ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	0.62ms - 0.62ms	-	unsure 🔍 -1% - +1% -0.00ms - +0.00ms	faster ✔ 10% - 12% 0.07ms - 0.09ms
preact-local	0.62ms - 0.62ms	unsure 🔍 -1% - +1% -0.00ms - +0.00ms	-	faster ✔ 10% - 12% 0.07ms - 0.09ms
preact-hooks	0.69ms - 0.71ms	slower ❌ 11% - 14% 0.07ms - 0.09ms	slower ❌ 11% - 14% 0.07ms - 0.09ms	-

todo

Browser: chrome-headless 114.0.5735.133
Sample size: 90
Built by: Benchmarks #1135
Commit: 1f4e464

duration

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	54.28ms - 55.37ms	-	unsure 🔍 -2% - +1% -0.99ms - +0.64ms	faster ✔ 6% - 9% 3.51ms - 5.12ms
preact-local	54.39ms - 55.60ms	unsure 🔍 -1% - +2% -0.64ms - +0.99ms	-	faster ✔ 6% - 8% 3.29ms - 4.99ms
preact-hooks	58.54ms - 59.73ms	slower ❌ 6% - 9% 3.51ms - 5.12ms	slower ❌ 6% - 9% 3.29ms - 4.99ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local	vs preact-hooks
preact-master	0.85ms - 0.86ms	-	unsure 🔍 -0% - +1% -0.00ms - +0.01ms	faster ✔ 3% - 5% 0.03ms - 0.04ms
preact-local	0.85ms - 0.85ms	unsure 🔍 -1% - +0% -0.01ms - +0.00ms	-	faster ✔ 3% - 5% 0.03ms - 0.04ms
preact-hooks	0.88ms - 0.90ms	slower ❌ 3% - 5% 0.03ms - 0.04ms	slower ❌ 4% - 5% 0.03ms - 0.04ms	-

_{tachometer-reporter-action v2 for Benchmarks}

coveralls · 2023-06-28T09:43:14Z

coverage: 99.561% (-0.001%) from 99.562% when pulling 1f4e464 on queue-refs into a5c810c on master.

github-actions · 2023-06-28T09:43:52Z

Size Change: +67 B (0%)

Total Size: 57.1 kB

Filename	Size	Change
`dist/preact.js`	4.35 kB	+13 B (0%)
`dist/preact.min.js`	4.38 kB	+12 B (0%)
`dist/preact.min.module.js`	4.38 kB	+13 B (0%)
`dist/preact.min.umd.js`	4.4 kB	+13 B (0%)
`dist/preact.module.js`	4.37 kB	+10 B (0%)
`dist/preact.umd.js`	4.41 kB	+6 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`compat/dist/compat.js`	3.92 kB	0 B
`compat/dist/compat.module.js`	3.84 kB	0 B
`compat/dist/compat.umd.js`	3.98 kB	0 B
`debug/dist/debug.js`	3.64 kB	0 B
`debug/dist/debug.module.js`	3.62 kB	0 B
`debug/dist/debug.umd.js`	3.71 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

I like the idea of having a ref queue 👍

andrewiggins

Oh nice! This fixes an issue I've been chasing for a lil while (#3860), but I like this better. It's quite bit a simpler and less impactful to stuff. Nice!

Would you mind if I pushed some tests from #3860 this PR fixes? I can also do a follow up PR if you prefer :)

JoviDeCroock · 2023-06-29T05:42:21Z

@andrewiggins feel free to push tests or a new PR however you prefer

andrewiggins · 2023-06-30T00:19:02Z

Went ahead and pushed the changes to this PR since they are all related and fixed by this change 🎉

JoviDeCroock added 2 commits June 28, 2023 11:29

queue refs up for applying after diffing complets

b35103a

execute setting ref to null immediately and queue up unsetting diff f…

e65cc81

…or post diff

JoviDeCroock requested review from andrewiggins, developit and marvinhagemeister June 28, 2023 09:41

save bytes

9422802

JoviDeCroock force-pushed the queue-refs branch from 504f025 to 9422802 Compare June 28, 2023 09:50

JoviDeCroock added 3 commits June 28, 2023 14:26

remove need for refs array

44cf765

save some more bytes

abc3e98

add types

ac7ed37

JoviDeCroock force-pushed the queue-refs branch from 269f2db to ac7ed37 Compare June 28, 2023 12:33

marvinhagemeister approved these changes Jun 28, 2023

View reviewed changes

andrewiggins approved these changes Jun 28, 2023

View reviewed changes

Add tests from #3860

1f4e464

JoviDeCroock merged commit 76d6909 into master Jun 30, 2023

JoviDeCroock deleted the queue-refs branch June 30, 2023 05:23

JoviDeCroock added a commit that referenced this pull request Jul 2, 2023

this was fixed by #4054

6553f47

JoviDeCroock added a commit that referenced this pull request Jul 3, 2023

this was fixed by #4054 (#4058)

0cdc9df

JoviDeCroock mentioned this pull request Jul 8, 2023

10.16.0 #4063

Merged

JoviDeCroock mentioned this pull request Sep 11, 2023

Move renderCallbacks to VNode and queue refs as renderCallbacks #3860

Closed

Uh oh!

Conversation

JoviDeCroock commented Jun 28, 2023

Uh oh!

github-actions Bot commented Jun 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 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

Uh oh!

coveralls commented Jun 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marvinhagemeister left a comment

Choose a reason for hiding this comment

Uh oh!

andrewiggins left a comment

Choose a reason for hiding this comment

Uh oh!

JoviDeCroock commented Jun 29, 2023

Uh oh!

andrewiggins commented Jun 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Jun 28, 2023 •

edited

Loading

coveralls commented Jun 28, 2023 •

edited

Loading

github-actions Bot commented Jun 28, 2023 •

edited

Loading