fix(hydration): Suspender returns Fragment with multiple DOM-nodes #4438

JoviDeCroock · 2024-07-08T17:09:59Z

This is an issue that presents itself when a lazy boundary returns a Fragment-like which results in an issue with our current excessDomChildren restoration. At this point in time we just take oldDom which amounts to us restoring The first of said Fragment-like wrapper. The current solution is a bit of a "quick one" to highlight a solution to the problem.

The current solution however would introduce an issue when we have

<>
  <Suspense>
    <Lazy /> --> <div>hello</div><div>world</div>
  <Suspense>
  <div>baz</div> --> because we only mark excessDomChildren[indexOfOldDom] as null we could accidentally match <div>world</div> here
</>

This might however be acceptable, because React fails here as well

github-actions · 2024-07-08T17:10:12Z

📊 Tachometer Benchmark Results

Summary

duration

create10k: unsure 🔍 -1% - +1% (-9.56ms - +11.49ms)
^{preact-local vs preact-main}
filter-list: unsure 🔍 -4% - +1% (-0.75ms - +0.14ms)
^{preact-local vs preact-main}
hydrate1k: slower ❌ 1% - 5% (0.65ms - 3.36ms)
^{preact-local vs preact-main}
many-updates: unsure 🔍 -5% - +3% (-0.85ms - +0.57ms)
^{preact-local vs preact-main}
replace1k: faster ✔ 6% - 10% (4.43ms - 7.56ms)
^{preact-local vs preact-main}
text-update: unsure 🔍 -5% - +2% (-0.10ms - +0.05ms)
^{preact-local vs preact-main}
todo: unsure 🔍 -1% - +1% (-0.24ms - +0.31ms)
^{preact-local vs preact-main}
update10th1k: unsure 🔍 -1% - +2% (-0.37ms - +0.61ms)
^{preact-local vs preact-main}

usedJSHeapSize

create10k: slower ❌ 2% - 2% (0.44ms - 0.45ms)
^{preact-local vs preact-main}
filter-list: slower ❌ 1% - 1% (0.01ms - 0.02ms)
^{preact-local vs preact-main}
hydrate1k: unsure 🔍 -0% - +3% (-0.04ms - +0.39ms)
^{preact-local vs preact-main}
many-updates: slower ❌ 2% - 2% (0.08ms - 0.09ms)
^{preact-local vs preact-main}
replace1k: unsure 🔍 -0% - +1% (-0.01ms - +0.03ms)
^{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}
update10th1k: slower ❌ 1% - 1% (0.04ms - 0.05ms)
^{preact-local vs preact-main}

Results

create10k

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1577
Commit: 9e7473e

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	869.73ms - 884.20ms	-	unsure 🔍 -1% - +1% -9.56ms - +11.49ms
preact-main	868.36ms - 883.64ms	unsure 🔍 -1% - +1% -11.49ms - +9.56ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	25.59ms - 25.60ms	-	slower ❌ 2% - 2% 0.44ms - 0.45ms
preact-main	25.15ms - 25.15ms	faster ✔ 2% - 2% 0.44ms - 0.45ms	-

filter-list

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1577
Commit: 9e7473e

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	16.62ms - 16.72ms	-	unsure 🔍 -4% - +1% -0.75ms - +0.14ms
preact-main	16.54ms - 17.42ms	unsure 🔍 -1% - +5% -0.14ms - +0.75ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.70ms - 1.71ms	-	slower ❌ 1% - 1% 0.01ms - 0.02ms
preact-main	1.69ms - 1.69ms	faster ✔ 1% - 1% 0.01ms - 0.02ms	-

hydrate1k

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1577
Commit: 9e7473e

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	70.90ms - 73.12ms	-	slower ❌ 1% - 5% 0.65ms - 3.36ms
preact-main	69.22ms - 70.78ms	faster ✔ 1% - 5% 0.65ms - 3.36ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	13.35ms - 13.63ms	-	unsure 🔍 -0% - +3% -0.04ms - +0.39ms
preact-main	13.15ms - 13.48ms	unsure 🔍 -3% - +0% -0.39ms - +0.04ms	-

many-updates

Browser: chrome-headless
Sample size: 70
Built by: Benchmarks #1577
Commit: 9e7473e

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	18.10ms - 19.11ms	-	unsure 🔍 -5% - +3% -0.85ms - +0.57ms
preact-main	18.24ms - 19.24ms	unsure 🔍 -3% - +5% -0.57ms - +0.85ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	4.70ms - 4.70ms	-	slower ❌ 2% - 2% 0.08ms - 0.09ms
preact-main	4.61ms - 4.62ms	faster ✔ 2% - 2% 0.08ms - 0.09ms	-

replace1k

Browser: chrome-headless
Sample size: 100
Built by: Benchmarks #1577
Commit: 9e7473e

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	66.63ms - 68.37ms	-	faster ✔ 6% - 10% 4.43ms - 7.56ms
preact-main	72.19ms - 74.79ms	slower ❌ 6% - 11% 4.43ms - 7.56ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	3.54ms - 3.56ms	-	unsure 🔍 -0% - +1% -0.01ms - +0.03ms
preact-main	3.52ms - 3.55ms	unsure 🔍 -1% - +0% -0.03ms - +0.01ms	-

run-warmup-0

Version	Avg time	vs preact-local	vs preact-main
preact-local	29.08ms - 29.79ms	-	slower ❌ 0% - 3% 0.10ms - 0.95ms
preact-main	28.68ms - 29.14ms	faster ✔ 0% - 3% 0.10ms - 0.95ms	-

run-warmup-1

Version	Avg time	vs preact-local	vs preact-main
preact-local	33.70ms - 35.54ms	-	unsure 🔍 -5% - +2% -1.94ms - +0.67ms
preact-main	34.32ms - 36.18ms	unsure 🔍 -2% - +6% -0.67ms - +1.94ms	-

run-warmup-2

Version	Avg time	vs preact-local	vs preact-main
preact-local	24.68ms - 25.11ms	-	unsure 🔍 -1% - +1% -0.30ms - +0.35ms
preact-main	24.62ms - 25.11ms	unsure 🔍 -1% - +1% -0.35ms - +0.30ms	-

run-warmup-3

Version	Avg time	vs preact-local	vs preact-main
preact-local	24.59ms - 25.48ms	-	faster ✔ 13% - 19% 3.82ms - 5.62ms
preact-main	28.97ms - 30.54ms	slower ❌ 15% - 23% 3.82ms - 5.62ms	-

run-warmup-4

Version	Avg time	vs preact-local	vs preact-main
preact-local	29.14ms - 31.07ms	-	slower ❌ 12% - 37% 3.40ms - 8.39ms
preact-main	21.91ms - 26.51ms	faster ✔ 12% - 28% 3.40ms - 8.39ms	-

run-final

Version	Avg time	vs preact-local	vs preact-main
preact-local	21.96ms - 22.78ms	-	unsure 🔍 -5% - +1% -1.17ms - +0.23ms
preact-main	22.27ms - 23.40ms	unsure 🔍 -1% - +5% -0.23ms - +1.17ms	-

text-update

Browser: chrome-headless
Sample size: 140
Built by: Benchmarks #1577
Commit: 9e7473e

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.95ms - 2.05ms	-	unsure 🔍 -5% - +2% -0.10ms - +0.05ms
preact-main	1.98ms - 2.08ms	unsure 🔍 -2% - +5% -0.05ms - +0.10ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	0.98ms - 0.98ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-main	0.98ms - 0.98ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

todo

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1577
Commit: 9e7473e

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	26.75ms - 27.11ms	-	unsure 🔍 -1% - +1% -0.24ms - +0.31ms
preact-main	26.68ms - 27.10ms	unsure 🔍 -1% - +1% -0.31ms - +0.24ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.25ms - 1.25ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms
preact-main	1.25ms - 1.25ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-

update10th1k

Browser: chrome-headless
Sample size: 50
Built by: Benchmarks #1577
Commit: 9e7473e

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	29.77ms - 30.55ms	-	unsure 🔍 -1% - +2% -0.37ms - +0.61ms
preact-main	29.74ms - 30.34ms	unsure 🔍 -2% - +1% -0.61ms - +0.37ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	3.60ms - 3.60ms	-	slower ❌ 1% - 1% 0.04ms - 0.05ms
preact-main	3.55ms - 3.55ms	faster ✔ 1% - 1% 0.04ms - 0.05ms	-

_{tachometer-reporter-action v2 for Benchmarks}

github-actions · 2024-07-08T17:11:03Z

Size Change: +91 B (+0.15%)

Total Size: 62 kB

Filename	Size	Change
`dist/preact.js`	4.72 kB	+15 B (+0.32%)
`dist/preact.min.js`	4.75 kB	+13 B (+0.27%)
`dist/preact.min.module.js`	4.75 kB	+14 B (+0.3%)
`dist/preact.min.umd.js`	4.77 kB	+13 B (+0.27%)
`dist/preact.module.js`	4.74 kB	+14 B (+0.3%)
`dist/preact.umd.js`	4.79 kB	+14 B (+0.29%)
`jsx-runtime/dist/jsxRuntime.js`	985 B	+4 B (+0.41%)
`jsx-runtime/dist/jsxRuntime.module.js`	957 B	+1 B (+0.1%)
`jsx-runtime/dist/jsxRuntime.umd.js`	1.06 kB	+3 B (+0.28%)

ℹ️ View Unchanged

Filename	Size
`compat/dist/compat.js`	4.1 kB
`compat/dist/compat.module.js`	4.03 kB
`compat/dist/compat.umd.js`	4.17 kB
`debug/dist/debug.js`	3.7 kB
`debug/dist/debug.module.js`	3.71 kB
`debug/dist/debug.umd.js`	3.78 kB
`devtools/dist/devtools.js`	259 B
`devtools/dist/devtools.module.js`	273 B
`devtools/dist/devtools.umd.js`	345 B
`hooks/dist/hooks.js`	1.53 kB
`hooks/dist/hooks.module.js`	1.56 kB
`hooks/dist/hooks.umd.js`	1.6 kB
`test-utils/dist/testUtils.js`	451 B
`test-utils/dist/testUtils.module.js`	456 B
`test-utils/dist/testUtils.umd.js`	536 B

_{compressed-size-action}

compat/test/browser/suspense-hydration.test.js

JoviDeCroock · 2024-07-08T17:39:54Z

src/diff/index.js

@@ -277,6 +277,7 @@ export function diff(
 				newVNode._flags |= isHydrating
 					? MODE_HYDRATE | MODE_SUSPENDED
 					: MODE_HYDRATE;
+				newVNode._excess = [...excessDomChildren];


An alternative to copying here would be to mark certain nodes i.e. we mark it with the oldVNode signature and then when we resume hydration in the initial src/diff/index.js check we unlock it by checking. Or we start marking the DOM-node as consumed so we rely less on these null entries

coveralls · 2024-07-08T17:40:45Z

coverage: 99.612%. remained the same
when pulling 9e7473e on highlight-fragment-suspension
into 212b1f6 on main.

Highlight issue when a lazy boundary returns a fragment

c4b3387

highlight described issue

18e57dc

JoviDeCroock commented Jul 8, 2024

View reviewed changes

compat/test/browser/suspense-hydration.test.js Outdated Show resolved Hide resolved

JoviDeCroock force-pushed the highlight-fragment-suspension branch from b917e40 to 42db9fa Compare July 8, 2024 17:31

Update compat/test/browser/suspense-hydration.test.js

9e7473e

JoviDeCroock force-pushed the highlight-fragment-suspension branch from 42db9fa to 9e7473e Compare July 8, 2024 17:38

JoviDeCroock requested review from developit, andrewiggins and marvinhagemeister July 8, 2024 17:38

JoviDeCroock commented Jul 8, 2024

View reviewed changes

marvinhagemeister approved these changes Jul 9, 2024

View reviewed changes

This was referenced Jul 10, 2024

[bug]: empty result of lazy component doubles next node #4075

Open

RFC: Hydration 2.0 #4442

Open

JoviDeCroock closed this Jul 16, 2024

JoviDeCroock deleted the highlight-fragment-suspension branch July 16, 2024 15:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(hydration): Suspender returns Fragment with multiple DOM-nodes #4438

fix(hydration): Suspender returns Fragment with multiple DOM-nodes #4438

JoviDeCroock commented Jul 8, 2024 •

edited

Loading

github-actions bot commented Jul 8, 2024 •

edited

Loading

📊 Tachometer Benchmark Results

Summary

duration

usedJSHeapSize

Results

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

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

github-actions bot commented Jul 8, 2024 •

edited

Loading

JoviDeCroock Jul 8, 2024 •

edited

Loading

coveralls commented Jul 8, 2024 •

edited

Loading

fix(hydration): Suspender returns Fragment with multiple DOM-nodes #4438

fix(hydration): Suspender returns Fragment with multiple DOM-nodes #4438

Conversation

JoviDeCroock commented Jul 8, 2024 • edited Loading

github-actions bot commented Jul 8, 2024 • edited Loading

📊 Tachometer Benchmark Results

Summary

duration

usedJSHeapSize

Results

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

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

github-actions bot commented Jul 8, 2024 • edited Loading

JoviDeCroock Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

coveralls commented Jul 8, 2024 • edited Loading

JoviDeCroock commented Jul 8, 2024 •

edited

Loading

github-actions bot commented Jul 8, 2024 •

edited

Loading

github-actions bot commented Jul 8, 2024 •

edited

Loading

JoviDeCroock Jul 8, 2024 •

edited

Loading

coveralls commented Jul 8, 2024 •

edited

Loading