Separate internal component handling from component implementation

[andrewiggins]

trafficstars

My goal with this change is to better understand what parts of our component implementation are generic and which are specific to React-style components. So I've added a separate renderReactComponent which is intended to only contain code for rendering react-style components and renderComponent contains generic code for rendering components agnostically. There is a lot of cleanup I'd like to do and I'm not satisfied with some of the existing hacks here, but I thought I'd share to get some early feedback.

This PR is probably better understood by looking at the commits individually. I'll try to summarize the changes here:

• Add renderReactComponent function (481fad5)
• Add SKIP sentinel to communicate children should be skipped (481fad5) (bleh, currently not a fan of this)
• Add setGlobalContext function to allow a component implementation to change the globalContext passed to its children (481fad5) (bleh, currently not a fan of this)
• Always pass newVNode to renderComponent (c4c2fe5)
• Move VNode equality check into patch so we handle both component and DOM node VNode equality in the same place (3666939) (Note: b598683 first moves it outside of renderReactComponent)
• Move DIRTY_BIT handling out of component implementation (5e93a1b)
• Copy props from VNode to internal in renderComponent so component implementation doesn't have to (b687b49)

The remaining pieces to figure out are to determine how we can do react component things without importing from Preact internals, assuming that's something we are interested in pursuing 😅

[github-actions[bot]]

📊 Tachometer Benchmark Results

Summary

duration

• 02_replace1k: slower ❌ 0% - 5% (0.05ms - 6.89ms)
  ^{preact-local vs preact-master}
• 03_update10th1k_x16: unsure 🔍 -4% - +5% (-1.34ms - +1.54ms)
  ^{preact-local vs preact-master}
• 07_create10k: unsure 🔍 -1% - +1% (-8.12ms - +16.96ms)
  ^{preact-local vs preact-master}
• filter_list: unsure 🔍 -5% - +0% (-1.44ms - +0.06ms)
  ^{preact-local vs preact-master}
• hydrate1k: unsure 🔍 -1% - +3% (-0.65ms - +2.57ms)
  ^{preact-local vs preact-master}
• many_updates: faster ✔ 0% - 6% (0.03ms - 1.73ms)
  ^{preact-local vs preact-master}
• text_update: unsure 🔍 -3% - +5% (-0.10ms - +0.18ms)
  ^{preact-local vs preact-master}

usedJSHeapSize

• 02_replace1k: slower ❌ 1% - 2% (0.04ms - 0.07ms)
  ^{preact-local vs preact-master}
• 03_update10th1k_x16: slower ❌ 1% - 2% (0.04ms - 0.08ms)
  ^{preact-local vs preact-master}
• 07_create10k: slower ❌ 1% - 1% (0.38ms - 0.38ms)
  ^{preact-local vs preact-master}
• filter_list: unsure 🔍 -1% - +1% (-0.02ms - +0.02ms)
  ^{preact-local vs preact-master}
• hydrate1k: slower ❌ 1% - 2% (0.08ms - 0.13ms)
  ^{preact-local vs preact-master}
• many_updates: slower ❌ 2% - 3% (0.11ms - 0.15ms)
  ^{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 90.0.4430.93
• Sample size: 80
• Built by: Benchmarks #258

• Commit: dd40f03

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	141.80ms - 146.98ms	-	faster ✔ 0% - 5% 0.05ms - 6.89ms
preact-local	145.63ms - 150.09ms	slower ❌ 0% - 5% 0.05ms - 6.89ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	3.87ms - 3.88ms	-	faster ✔ 1% - 2% 0.04ms - 0.07ms
preact-local	3.92ms - 3.94ms	slower ❌ 1% - 2% 0.04ms - 0.07ms	-

run-warmup-0

Version	Avg time	vs preact-master	vs preact-local
preact-master	62.25ms - 63.98ms	-	faster ✔ 2% - 6% 1.01ms - 3.68ms
preact-local	64.43ms - 66.47ms	slower ❌ 2% - 6% 1.01ms - 3.68ms	-

run-warmup-1

Version	Avg time	vs preact-master	vs preact-local
preact-master	78.14ms - 80.96ms	-	faster ✔ 5% - 10% 4.18ms - 8.23ms
preact-local	84.31ms - 87.20ms	slower ❌ 5% - 10% 4.18ms - 8.23ms	-

run-warmup-2

Version	Avg time	vs preact-master	vs preact-local
preact-master	75.17ms - 78.44ms	-	unsure 🔍 -3% - +2% -2.57ms - +1.88ms
preact-local	75.64ms - 78.65ms	unsure 🔍 -2% - +3% -1.88ms - +2.57ms	-

run-warmup-3

Version	Avg time	vs preact-master	vs preact-local
preact-master	65.85ms - 68.18ms	-	unsure 🔍 -1% - +4% -0.71ms - +2.74ms
preact-local	64.73ms - 67.27ms	unsure 🔍 -4% - +1% -2.74ms - +0.71ms	-

run-warmup-4

Version	Avg time	vs preact-master	vs preact-local
preact-master	84.62ms - 87.71ms	-	slower ❌ 7% - 13% 5.72ms - 10.37ms
preact-local	76.38ms - 79.85ms	faster ✔ 7% - 12% 5.72ms - 10.37ms	-

run-final

Version	Avg time	vs preact-master	vs preact-local
preact-master	62.21ms - 65.48ms	-	faster ✔ 1% - 7% 0.41ms - 4.89ms
preact-local	64.96ms - 68.03ms	slower ❌ 1% - 8% 0.41ms - 4.89ms	-

03_update10th1k_x16

• Browser: chrome-headless 90.0.4430.93
• Sample size: 60
• Built by: Benchmarks #258

• Commit: dd40f03

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	31.10ms - 33.21ms	-	unsure 🔍 -5% - +4% -1.54ms - +1.34ms
preact-local	31.27ms - 33.24ms	unsure 🔍 -4% - +5% -1.34ms - +1.54ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	3.82ms - 3.85ms	-	faster ✔ 1% - 2% 0.04ms - 0.08ms
preact-local	3.88ms - 3.91ms	slower ❌ 1% - 2% 0.04ms - 0.08ms	-

07_create10k

• Browser: chrome-headless 90.0.4430.93
• Sample size: 50
• Built by: Benchmarks #258

• Commit: dd40f03

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	1473.07ms - 1490.73ms	-	unsure 🔍 -1% - +1% -16.96ms - +8.12ms
preact-local	1477.41ms - 1495.23ms	unsure 🔍 -1% - +1% -8.12ms - +16.96ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	27.27ms - 27.27ms	-	faster ✔ 1% - 1% 0.38ms - 0.38ms
preact-local	27.65ms - 27.65ms	slower ❌ 1% - 1% 0.38ms - 0.38ms	-

filter_list

• Browser: chrome-headless 90.0.4430.93
• Sample size: 90
• Built by: Benchmarks #258

• Commit: dd40f03

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	28.95ms - 30.12ms	-	unsure 🔍 -0% - +5% -0.06ms - +1.44ms
preact-local	28.37ms - 29.32ms	unsure 🔍 -5% - +0% -1.44ms - +0.06ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	1.85ms - 1.89ms	-	unsure 🔍 -1% - +1% -0.02ms - +0.02ms
preact-local	1.85ms - 1.89ms	unsure 🔍 -1% - +1% -0.02ms - +0.02ms	-

hydrate1k

• Browser: chrome-headless 90.0.4430.93
• Sample size: 50
• Built by: Benchmarks #258

• Commit: dd40f03

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	87.71ms - 89.82ms	-	unsure 🔍 -3% - +1% -2.57ms - +0.65ms
preact-local	88.51ms - 90.94ms	unsure 🔍 -1% - +3% -0.65ms - +2.57ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	6.44ms - 6.47ms	-	faster ✔ 1% - 2% 0.08ms - 0.13ms
preact-local	6.53ms - 6.57ms	slower ❌ 1% - 2% 0.08ms - 0.13ms	-

many_updates

• Browser: chrome-headless 90.0.4430.93
• Sample size: 80
• Built by: Benchmarks #258

• Commit: dd40f03

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	28.59ms - 29.43ms	-	slower ❌ 0% - 6% 0.03ms - 1.73ms
preact-local	27.40ms - 28.87ms	faster ✔ 0% - 6% 0.03ms - 1.73ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	5.40ms - 5.43ms	-	faster ✔ 2% - 3% 0.11ms - 0.15ms
preact-local	5.53ms - 5.56ms	slower ❌ 2% - 3% 0.11ms - 0.15ms	-

text_update

• Browser: chrome-headless 90.0.4430.93
• Sample size: 250
• Built by: Benchmarks #258

• Commit: dd40f03

duration

Version	Avg time	vs preact-master	vs preact-local
preact-master	3.48ms - 3.65ms	-	unsure 🔍 -5% - +3% -0.18ms - +0.10ms
preact-local	3.50ms - 3.71ms	unsure 🔍 -3% - +5% -0.10ms - +0.18ms	-

usedJSHeapSize

Version	Avg time	vs preact-master	vs preact-local
preact-master	0.84ms - 0.84ms	-	unsure 🔍 -0% - -0% -0.00ms - -0.00ms
preact-local	0.84ms - 0.84ms	unsure 🔍 +0% - +0% +0.00ms - +0.00ms	-

_{tachometer-reporter-action v2 for Benchmarks}

[github-actions[bot]]

Size Change: +438 B (0%)

Total Size: 44.9 kB

Filename	Size	Change
compat/dist/compat.js	3.34 kB	+12 B (0%)
compat/dist/compat.module.js	3.37 kB	+12 B (0%)
compat/dist/compat.umd.js	3.39 kB	+13 B (0%)
dist/preact.js	4.67 kB	+97 B (2%)
dist/preact.min.js	4.68 kB	+84 B (1%)
dist/preact.module.js	4.69 kB	+107 B (2%)
dist/preact.umd.js	4.74 kB	+96 B (2%)
hooks/dist/hooks.js	1.17 kB	+5 B (0%)
hooks/dist/hooks.module.js	1.18 kB	+6 B (0%)
hooks/dist/hooks.umd.js	1.23 kB	+6 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
debug/dist/debug.js	3.06 kB	0 B
debug/dist/debug.module.js	3.05 kB	0 B
debug/dist/debug.umd.js	3.15 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
jsx-runtime/dist/jsxRuntime.js	294 B	0 B
jsx-runtime/dist/jsxRuntime.module.js	303 B	0 B
jsx-runtime/dist/jsxRuntime.umd.js	373 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}

[coveralls]

Coverage increased (+0.1%) to 96.174% when pulling dd40f0360284f44070fc98abba0a93206e75bcd2 on restructure-component-prep into a8009a5077fba6b72cae6e341acdca6a62325a01 on restructure.

[developit]

This all looks good (and like a boatload of work, nice job). One thing that was gnawing at me as I read through though: what was the reason for moving globalContext to be global instead of being an object passed down through the tree as a function parameter? I recall we talked about this, but I don't remember the reasoning.