feat: use comments instead of element as marker

[jacob-ebey]

Summary:

• use comments to denote placeholders to not break semantic HTML structures
• use a custom-element to handle moving into place
  • this remove the need for a mutation observer
  • seems like a cool use of a ce and I figured why not? should probably perf test it though
• adds onError for the underlying renderToChunks implementation
• adds renderToPipeableStream

[changeset-bot]

⚠️ No Changeset found

Latest commit: 3ad1d62

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[jacob-ebey]

Is there a better way to get HTML comments than this?

[jacob-ebey]

Ok, this should be more performant now: https://github.com/preactjs/preact-render-to-string/pull/260/files#diff-627fe08d8bba6a4fa76e7c5ed36ef6f16d126733e65e19236b391de0d34f1fe0R14

Decided to go with a NodeIterator. TIL

[jacob-ebey]

If we are worried about perf we can switch to TreeWalker, but NodeIterator has better browser support if I remember right:

[jacob-ebey]

More benchmarks if we want to switch:
Chome:

Name         Operations per second    Average time, ms
walker       1.0 x 10^5               0.01                ==============================>
iterator     5.0 x 10^4               0.02                ===============>
recursion    7.7 x 10^3               0.13                ==>

Safari:

Name         Operations per second    Average time, ms (main.ts, line 70)
walker       9.1 x 10^4               0.01                ==============================>
iterator     6.3 x 10^4               0.02                =====================>
recursion    3.6 x 10^3               0.28                =>

Walker with a filter is also significantly faster than without and that assumption holds true for iterator as well:

Name                Operations per second    Average time, ms
walker filter       9.1 x 10^4               0.01                ==============================>
walker no filter    1.0 x 10^4               0.10                ===>

[jacob-ebey]

Here's the source for the benchmark:

import Benchmark from "micro-benchmark";

function nest(_parent: HTMLElement, content: string, n: number) {
  let _el = document.createElement("div");
  _el.innerText = content;
  _parent.appendChild(_el);

  if (--n) nest(_el, content, n);
  else {
    _el.appendChild(document.createComment("FIND_ME"));
  }
}

nest(document.body, "👻", 2000);

function findCommentRecursive(node: ChildNode, text: string): Node | null {
  if (node.nodeType === Node.COMMENT_NODE && node.nodeValue === text) {
    return node;
  }

  let child = node.firstChild;
  while (child) {
    const found = findCommentRecursive(child, text);
    if (found) return found;
    child = child.nextSibling;
  }

  return null;
}

function findCommentNodeIterator(node: ChildNode, text: string): Node | null {
  const iterator = document.createNodeIterator(node, NodeFilter.SHOW_COMMENT);
  let current = iterator.nextNode();
  while (current) {
    if (current.nodeValue === text) return current;
    current = iterator.nextNode();
  }
  return null;
}

function findCommentTreeWalker(node: ChildNode, text: string): Node | null {
  const walker = document.createTreeWalker(node, NodeFilter.SHOW_COMMENT);
  let current = walker.nextNode();
  while (current) {
    if (current.nodeValue === text) return current;
    current = walker.nextNode();
  }
  return null;
}

let result = Benchmark.suite({
  specs: [
    {
      name: "recursion",
      fn: () => {
        findCommentRecursive(document.body, "FIND_ME");
      },
    },
    {
      name: "iterator",
      fn: () => {
        findCommentNodeIterator(document.body, "FIND_ME");
      },
    },
    {
      name: "walker",
      fn: () => {
        findCommentTreeWalker(document.body, "FIND_ME");
      },
    },
  ],
});

let report = Benchmark.report(result);
console.log(report);

[developit]

Doing a deep walk here seems expensive. What about using a hidden element to denote the start of any island, followed by a comment sibling to denote its end?

  <link id="preact-island-00">
  <h1>hello world</h1>
  this is some text
  <!--preact-island-00-->

Suggested change

	var s,
	e,
	c = document.createNodeIterator(document, 128);
	while (c.nextNode()) {
	let n = c.referenceNode;
	if (n.data == 'preact-island:' + i) s = n;
	else if (n.data == '/preact-island:' + i) e = n;
	if (s && e) break;
	}
	if (s && e) {
	var p = e.previousSibling;
	while (p != s) {
	if (!p || p == s) break;
	e.parentNode.removeChild(p);
	p = e.previousSibling;
	}
	while (d.firstChild) e.parentNode.insertBefore(d.firstChild, e);
	d.parentNode.removeChild(d);
	}
	var n = 'preact-island-' + id,
	s = document.getElementById(n),
	p = s.parentNode,
	e = s,
	b;
	while (e) {
	b = e.nextSibling;
	p.removeChild(e);
	if (e.nodeType === 8 && e.data === n) break;
	e = b;
	}
	while (b = d.firstChild) p.insertBefore(b, e);

[jacob-ebey]

Breaks semantic HTML for lists and whatnot. Same reason to not use a CE for denoting the fallback.

[marvinhagemeister]

Yeah I think there is no way around using comments as we always risk breaking HTML or CSS selectors (:nth-child, etc)

[developit]

Does this matter much if the elements are removed immediately?

[jacob-ebey]

Yes because when the fallback is displayed (potentially a long while) styles / semantics will be broken.

[marvinhagemeister]

LGTM, the more I think about it, the more I agree that the comment approach is the right one to take. Also good idea on using CE semantics instead of a MutationObserver 👍