preact icon indicating copy to clipboard operation
preact copied to clipboard

Published 20 hours ago • verified publisher icon preactjs

Move renderCallbacks to VNode and queue refs as renderCallbacks

Open andrewiggins opened this issue 6 years ago • 5 comments

In the spirit of #2011, queue the applyRef calls as _renderCallbacks so they are invoked after the entire DOM is updated. This change effectively undos 40212ebea from #1658.

I always didn't like how I had to move ref handling to diffChildren. That had to be done in #1658 because refs needed to be unmounted first, then the new value applied, for example, if the VNode changed tag type from div to span. We would need to call the ref with null when the div is unmounted, then call it again with the span in that order. This PR accomplishes the same behavior by queuing the setting of the new ref value in _renderCallbacks after all unmounts and DOM manipulations are done.

andrewiggins avatar Nov 07 '19 04:11 andrewiggins

Coverage Status

Coverage decreased (-0.4%) to 99.426% when pulling acf8195dc65aa14c0dd25f6fa8eafaa943197de8 on feat/vnode-render-callbacks into 0fb3b1318b1906f0afb6d0c0fc8decad1e991a4b on master.

coveralls avatar Nov 07 '19 04:11 coveralls

Hey,

Could we add a test here (can be skipped) to see if the refs are bound in ComponentDidMount?

Specific example:

<Parent>
  <Child />
  <Child />
</Parent>

The Child for example has a span with ref={ref} and in componentDidMount if (!ref) throw new Error(à

JoviDeCroock avatar Nov 07 '19 08:11 JoviDeCroock

Oh good call! Will do

andrewiggins avatar Nov 07 '19 19:11 andrewiggins

Size Change: -37 B (0%)

Total Size: 42.3 kB

Filename Size Change
dist/preact.js 3.95 kB -13 B (0%)
dist/preact.min.js 3.98 kB -19 B (0%)
dist/preact.module.js 3.98 kB -19 B (0%)
dist/preact.umd.js 4.02 kB -20 B (0%)
hooks/dist/hooks.js 1.14 kB +10 B (0%)
hooks/dist/hooks.module.js 1.16 kB +12 B (1%)
hooks/dist/hooks.umd.js 1.21 kB +9 B (0%)
jsx-runtime/dist/jsxRuntime.js 328 B +1 B
jsx-runtime/dist/jsxRuntime.module.js 336 B +1 B
jsx-runtime/dist/jsxRuntime.umd.js 406 B +1 B
ℹ️ View Unchanged
Filename Size Change
compat/dist/compat.js 3.49 kB 0 B
compat/dist/compat.module.js 3.5 kB 0 B
compat/dist/compat.umd.js 3.54 kB 0 B
debug/dist/debug.js 2.99 kB 0 B
debug/dist/debug.module.js 2.98 kB 0 B
debug/dist/debug.umd.js 3.07 kB 0 B
devtools/dist/devtools.js 232 B 0 B
devtools/dist/devtools.module.js 241 B 0 B
devtools/dist/devtools.umd.js 308 B 0 B
test-utils/dist/testUtils.js 437 B 0 B
test-utils/dist/testUtils.module.js 439 B 0 B
test-utils/dist/testUtils.umd.js 515 B 0 B

compressed-size-action

github-actions[bot] avatar Feb 02 '20 02:02 github-actions[bot]

📊 Tachometer Benchmark Results

Summary

duration

  • 02_replace1k: slower ❌ 2% - 5% (4.47ms - 9.40ms)
    preact-local vs preact-master
  • 03_update10th1k_x16: unsure 🔍 -2% - +6% (-0.61ms - +1.70ms)
    preact-local vs preact-master
  • 07_create10k: unsure 🔍 -2% - +1% (-35.91ms - +22.56ms)
    preact-local vs preact-master
  • filter_list: unsure 🔍 -4% - +1% (-1.04ms - +0.27ms)
    preact-local vs preact-master
  • hydrate1k: unsure 🔍 -3% - +4% (-3.99ms - +4.76ms)
    preact-local vs preact-master
  • many_updates: slower ❌ 1% - 16% (0.40ms - 6.06ms)
    preact-local vs preact-master
  • text_update: slower ❌ 2% - 5% (0.06ms - 0.17ms)
    preact-local vs preact-master

usedJSHeapSize

  • 02_replace1k: unsure 🔍 -0% - +0% (-0.00ms - +0.01ms)
    preact-local vs preact-master
  • 03_update10th1k_x16: slower ❌ 1% - 2% (0.05ms - 0.06ms)
    preact-local vs preact-master
  • 07_create10k: slower ❌ 1% - 1% (0.24ms - 0.24ms)
    preact-local vs preact-master
  • filter_list: unsure 🔍 +0% - +0% (+0.00ms - +0.01ms)
    preact-local vs preact-master
  • hydrate1k: slower ❌ 1% - 2% (0.05ms - 0.11ms)
    preact-local vs preact-master
  • many_updates: slower ❌ 2% - 2% (0.08ms - 0.08ms)
    preact-local vs preact-master
  • text_update: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
    preact-local vs preact-master

Results

02_replace1k
  • Browser: chrome-headless 88.0.4324.150
  • Sample size: 80
  • Built by: Benchmarks #109

  • Commit: 956464a

duration

VersionAvg timevs preact-mastervs preact-local
preact-master193.76ms - 197.52ms-faster ✔
2% - 5%
4.47ms - 9.40ms
preact-local200.98ms - 204.18msslower ❌
2% - 5%
4.47ms - 9.40ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master3.58ms - 3.58ms-unsure 🔍
-0% - +0%
-0.01ms - +0.00ms
preact-local3.58ms - 3.59msunsure 🔍
-0% - +0%
-0.00ms - +0.01ms		-

run-warmup-0

VersionAvg timevs preact-mastervs preact-local
preact-master73.82ms - 75.47ms-slower ❌
0% - 3%
0.32ms - 2.46ms
preact-local72.57ms - 73.94msfaster ✔
0% - 3%
0.32ms - 2.46ms		-

run-warmup-1

VersionAvg timevs preact-mastervs preact-local
preact-master109.71ms - 111.16ms-unsure 🔍
-2% - +0%
-2.45ms - +0.33ms
preact-local110.31ms - 112.68msunsure 🔍
-0% - +2%
-0.33ms - +2.45ms		-

run-warmup-2

VersionAvg timevs preact-mastervs preact-local
preact-master125.15ms - 129.91ms-slower ❌
32% - 43%
30.56ms - 38.96ms
preact-local89.30ms - 96.22msfaster ✔
24% - 30%
30.56ms - 38.96ms		-

run-warmup-3

VersionAvg timevs preact-mastervs preact-local
preact-master75.51ms - 79.83ms-faster ✔
22% - 27%
21.72ms - 28.30ms
preact-local100.19ms - 105.16msslower ❌
27% - 37%
21.72ms - 28.30ms		-

run-warmup-4

VersionAvg timevs preact-mastervs preact-local
preact-master125.04ms - 133.13ms-slower ❌
1% - 8%
0.88ms - 9.77ms
preact-local121.92ms - 125.61msfaster ✔
1% - 7%
0.88ms - 9.77ms		-

run-final

VersionAvg timevs preact-mastervs preact-local
preact-master78.90ms - 81.13ms-faster ✔
5% - 8%
4.44ms - 7.19ms
preact-local85.02ms - 86.64msslower ❌
5% - 9%
4.44ms - 7.19ms		-
03_update10th1k_x16
  • Browser: chrome-headless 88.0.4324.150
  • Sample size: 130
  • Built by: Benchmarks #109

  • Commit: 956464a

duration

VersionAvg timevs preact-mastervs preact-local
preact-master28.17ms - 29.91ms-unsure 🔍
-6% - +2%
-1.70ms - +0.61ms
preact-local28.83ms - 30.35msunsure 🔍
-2% - +6%
-0.61ms - +1.70ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master3.49ms - 3.50ms-faster ✔
1% - 2%
0.05ms - 0.06ms
preact-local3.54ms - 3.55msslower ❌
1% - 2%
0.05ms - 0.06ms		-
07_create10k
  • Browser: chrome-headless 88.0.4324.150
  • Sample size: 50
  • Built by: Benchmarks #109

  • Commit: 956464a

duration

VersionAvg timevs preact-mastervs preact-local
preact-master1872.87ms - 1917.11ms-unsure 🔍
-1% - +2%
-22.56ms - +35.91ms
preact-local1869.21ms - 1907.44msunsure 🔍
-2% - +1%
-35.91ms - +22.56ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master25.97ms - 25.97ms-faster ✔
1% - 1%
0.24ms - 0.24ms
preact-local26.21ms - 26.21msslower ❌
1% - 1%
0.24ms - 0.24ms		-
filter_list
  • Browser: chrome-headless 88.0.4324.150
  • Sample size: 60
  • Built by: Benchmarks #109

  • Commit: 956464a

duration

VersionAvg timevs preact-mastervs preact-local
preact-master26.24ms - 27.25ms-unsure 🔍
-1% - +4%
-0.27ms - +1.04ms
preact-local25.95ms - 26.78msunsure 🔍
-4% - +1%
-1.04ms - +0.27ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master1.59ms - 1.59ms-unsure 🔍
-0% - -0%
-0.01ms - -0.00ms
preact-local1.59ms - 1.59msunsure 🔍
+0% - +0%
+0.00ms - +0.01ms		-
hydrate1k
  • Browser: chrome-headless 88.0.4324.150
  • Sample size: 50
  • Built by: Benchmarks #109

  • Commit: 956464a

duration

VersionAvg timevs preact-mastervs preact-local
preact-master127.00ms - 133.09ms-unsure 🔍
-4% - +3%
-4.76ms - +3.99ms
preact-local127.28ms - 133.56msunsure 🔍
-3% - +4%
-3.99ms - +4.76ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master6.17ms - 6.20ms-faster ✔
1% - 2%
0.05ms - 0.11ms
preact-local6.24ms - 6.29msslower ❌
1% - 2%
0.05ms - 0.11ms		-
many_updates
  • Browser: chrome-headless 88.0.4324.150
  • Sample size: 80
  • Built by: Benchmarks #109

  • Commit: 956464a

duration

VersionAvg timevs preact-mastervs preact-local
preact-master36.66ms - 40.24ms-faster ✔
1% - 14%
0.40ms - 6.06ms
preact-local39.48ms - 43.87msslower ❌
1% - 16%
0.40ms - 6.06ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master4.83ms - 4.83ms-faster ✔
2% - 2%
0.08ms - 0.08ms
preact-local4.92ms - 4.92msslower ❌
2% - 2%
0.08ms - 0.08ms		-
text_update
  • Browser: chrome-headless 88.0.4324.150
  • Sample size: 220
  • Built by: Benchmarks #109

  • Commit: 956464a

duration

VersionAvg timevs preact-mastervs preact-local
preact-master3.41ms - 3.48ms-faster ✔
2% - 5%
0.06ms - 0.17ms
preact-local3.52ms - 3.60msslower ❌
2% - 5%
0.06ms - 0.17ms		-

usedJSHeapSize

VersionAvg timevs preact-mastervs preact-local
preact-master0.82ms - 0.82ms-unsure 🔍
-0% - -0%
-0.00ms - -0.00ms
preact-local0.82ms - 0.82msunsure 🔍
+0% - +0%
+0.00ms - +0.00ms		-

tachometer-reporter-action v2 for Benchmarks

github-actions[bot] avatar Feb 17 '21 00:02 github-actions[bot]

Superseded by #3860

andrewiggins avatar Jan 12 '23 19:01 andrewiggins