Test approach to comment markers

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

@@ -97,6 +97,104 @@ describe('suspense hydration', () => {
 		});
 	});
 
+	it('Should hydrate a fragment with multiple children correctly', () => {
+		scratch.innerHTML = '<!--$s--><div>Hello</div><div>World!</div><!--/$s-->';
+		clearLog();
+
+		const [Lazy, resolve] = createLazy();
+		hydrate(
+			<Suspense>
+				<Lazy />
+			</Suspense>,
+			scratch
+		);
+		rerender(); // Flush rerender queue to mimic what preact will really do
+		expect(scratch.innerHTML).to.equal(
+			'<!--$s--><div>Hello</div><div>World!</div><!--/$s-->'
+		);
+		expect(getLog()).to.deep.equal([]);
+		clearLog();
+
+		return resolve(() => (
+			<>
+				<div>Hello</div>
+				<div>World!</div>
+			</>
+		)).then(() => {
+			rerender();
+			expect(scratch.innerHTML).to.equal(
+				'<!--$s--><div>Hello</div><div>World!</div><!--/$s-->'
+			);
+			expect(getLog()).to.deep.equal([]);
+
+			clearLog();
+		});
+	});
+
+	it('Should hydrate a fragment with no children correctly', () => {
+		scratch.innerHTML = '<!--$s--><div>Hello</div><div>World!</div><!--/$s-->';
+		clearLog();
+
+		const [Lazy, resolve] = createLazy();
+		hydrate(
+			<Suspense>
+				<Lazy />
+			</Suspense>,
+			scratch
+		);
+		rerender(); // Flush rerender queue to mimic what preact will really do
+		expect(scratch.innerHTML).to.equal(
+			'<!--$s--><div>Hello</div><div>World!</div><!--/$s-->'
+		);
+		expect(getLog()).to.deep.equal([]);
+		clearLog();
+
+		return resolve(() => (
+			<>
+				<div>Hello</div>
+				<div>World!</div>
+			</>
+		)).then(() => {
+			rerender();
+			expect(scratch.innerHTML).to.equal(
+				'<!--$s--><div>Hello</div><div>World!</div><!--/$s-->'
+			);
+			expect(getLog()).to.deep.equal([]);
+
+			clearLog();
+		});
+	});
+
+	// This is in theory correct but still it shows that our oldDom becomes stale very quickly
+	// and moves DOM into weird places
+	it.skip('Should hydrate a fragment with no children and an adjacent node correctly', () => {
+		scratch.innerHTML = '<!--$s--><!--/$s--><div>Baz</div>';
+		clearLog();
+
+		const [Lazy, resolve] = createLazy();
+		hydrate(
+			<>
+				<Suspense>
+					<Lazy />
+				</Suspense>
+				<div>Baz</div>
+			</>,
+			scratch
+		);
+		rerender(); // Flush rerender queue to mimic what preact will really do
+		expect(scratch.innerHTML).to.equal('<!--$s--><!--/$s--><div>Baz</div>');
+		expect(getLog()).to.deep.equal([]);
+		clearLog();
+
+		return resolve(() => null).then(() => {
+			rerender();
+			expect(scratch.innerHTML).to.equal('<!--$s--><!--/$s--><div>Baz</div>');
+			expect(getLog()).to.deep.equal([]);
+
+			clearLog();
+		});
+	});
+
 	it('should properly attach event listeners when suspending while hydrating', () => {
 		scratch.innerHTML = '<div>Hello</div><div>World</div>';
 		clearLog();

jsx-runtime/src/index.js

@@ -59,6 +59,7 @@ function createVNode(type, props, key, isStaticChildren, __source, __self) {
 		_nextDom: undefined,
 		_component: null,
 		constructor: undefined,
+		_excess: null,
 		_original: --vnodeId,
 		_index: -1,
 		_flags: 0,

src/create-element.js

@@ -66,6 +66,7 @@ export function createVNode(type, props, key, ref, original) {
 		_parent: null,
 		_depth: 0,
 		_dom: null,
+		_excess: null,
 		// _nextDom must be initialized to undefined b/c it will eventually
 		// be set to dom.nextSibling which can return `null` and it is important
 		// to be able to distinguish between an uninitialized _nextDom and

src/diff/index.js

@@ -52,8 +52,13 @@ export function diff(
 	// If the previous diff bailed out, resume creating/hydrating.
 	if (oldVNode._flags & MODE_SUSPENDED) {
 		isHydrating = !!(oldVNode._flags & MODE_HYDRATE);
-		oldDom = newVNode._dom = oldVNode._dom;
-		excessDomChildren = [oldDom];
+		if (oldVNode._excess) {
+			excessDomChildren = oldVNode._excess;
+			oldDom = newVNode._dom = oldVNode._dom = excessDomChildren[1];
+		} else {
+			oldDom = newVNode._dom = oldVNode._dom;
+			excessDomChildren = [oldDom];
+		}
 	}
 
 	if ((tmp = options._diff)) tmp(newVNode);
@@ -273,13 +278,36 @@ export function diff(
 			newVNode._original = null;
 			// if hydrating or creating initial tree, bailout preserves DOM:
 			if (isHydrating || excessDomChildren != null) {
-				newVNode._dom = oldDom;
 				newVNode._flags |= isHydrating
 					? MODE_HYDRATE | MODE_SUSPENDED
 					: MODE_HYDRATE;
-				excessDomChildren[excessDomChildren.indexOf(oldDom)] = null;
-				// ^ could possibly be simplified to:
-				// excessDomChildren.length = 0;
+
+				let found = excessDomChildren.find(
+						child => child && child.nodeType == 8 && child.data == '$s'
+					),
+					index = excessDomChildren.indexOf(found) + 1;
+
+				newVNode._dom = oldDom;
+				if (found) {
+					let commentMarkersToFind = 1;
+					newVNode._excess = [found];
+					excessDomChildren[index - 1] = null;
+					while (commentMarkersToFind && index <= excessDomChildren.length) {
+						const node = excessDomChildren[index];
+						excessDomChildren[index] = null;
+						index++;
+						newVNode._excess.push(node);
+						if (node.nodeType == 8) {
+							if (node.data == '$s') {
+								commentMarkersToFind++;
+							} else if (node.data == '/$s') {
+								commentMarkersToFind--;
+							}
+						}
+					}
+				} else {
+					excessDomChildren[excessDomChildren.indexOf(oldDom)] = null;
+				}
 			} else {
 				newVNode._dom = oldVNode._dom;
 				newVNode._children = oldVNode._children;

src/internal.d.ts

@@ -145,6 +145,7 @@ declare global {
 		 * The [first (for Fragments)] DOM child of a VNode
 		 */
 		_dom: PreactElement | null;
+		_excess: PreactElement[] | null;
 		/**
 		 * The last dom child of a Fragment, or components that return a Fragment
 		 */