Suspense: re-rendering a parked component throws in insertBefore #5251

Hello there, while working on a project I had an issue, with the help of an IA I've generated the following report which shows nicely what the issue seems to be.

Summary

When a component sits inside a <Suspense> whose subtree is currently parked (a sibling suspended), and that component re-renders and mounts new DOM, insert() calls parentDom.insertBefore(newDom, oldDom) with parentDom = the detached container Suspense created, and oldDom = a DOM node that belongs to another container (a node removed from the document when the subtree was parked). The browser throws NotFoundError.

That first problem is loud. The second one is silent and permanent: the throw escapes renderComponent before commitRoot, but diff() had already set c._vnode = newVNode. The next re-render of that component diffs against the children created by the failed render. They are treated as already mounted: their ref identity is unchanged so no ref is queued, they are not flagged INSERT_VNODE so their DOM is never placed, and their mount effects are gone with the truncated commitQueue. When the Suspense boundary resolves, removeOriginal appends that DOM into the document and the restore diff positions it, so the elements end up in the document with the right text but every ref callback / layout effect of their mount is lost for the life of the tree.

Reproduced on 10.29.8, 11.0.0-beta.2 and 11.0.0-rc.2 (each run from its src/ through an import map, Chromium via Playwright 1.62).

Reproduction

Standalone page: https://codepen.io/editor/dmail/pen/01a084df-fbb4-7574-87de-b35192c8525a

Output:

1. Page suspends: the subtree is parked
2. Slot re-renders while parked and mounts <header>
window error: Uncaught NotFoundError: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.
3. Slot re-renders again while parked (any state change)
4. Page resolves: the subtree is restored
result: header in document = true, inline style = null

Expected: no error at step 2, and after step 4 the <header> is in the document with its ref called once (so style="color: red").

Actual: step 2 throws, and after step 4 the <header> is in the document but barRef was never called and no mount effect ran. Nothing recovers it afterwards: later re-renders of Slot diff the same <header> vnode, whose ref identity has not changed.

Step 3 is what makes the damage permanent. Without it (steps 1, 2, 4 only) the throw still happens but the restore diff re-creates the header from the pre-park vnode and the ref fires.

Wrapping Node.prototype.insertBefore at step 2 shows the operands:

Analysis (line numbers from 11.0.0-beta.2 src/)

1. insert() throws

compat/src/suspense.js detachedClone re-points _parentDom of every component directly under the boundary to a fresh detached <div>, so their later renders go into that container. diffChildren / insert (src/diff/children.js:349-379) computes the reference node from the vnode tree (getDomSibling), which still points at DOM nodes that were in the document before parking. The guard only handles a reference node with no parent at all, and getDomSibling(parentVNode) returns the same foreign node:

if (oldDom && parentVNode.type && !oldDom.parentNode) {
	oldDom = getDomSibling(parentVNode);
}
parentDom.insertBefore(parentVNode._dom, oldDom || NULL);

insertBefore requires the reference node to be a child of parentDom; here it is not, whether its parentNode is null or another container.

2. The failed vnode is adopted by the next render

• diff() sets c._vnode = newVNode before rendering (src/diff/index.js:178).
• The throw in insert is caught by the component's catch in diff() (src/diff/index.js:325-392): commitQueue is truncated, newVNode._dom = oldVNode._dom, options._catchError rethrows when no boundary catches it.
• renderComponent (src/component.js:128-160) is unwound before newVNode._parent._children[newVNode._index] = newVNode and before commitRoot(commitQueue, newVNode, refQueue). So the refs queued for the freshly created DOM and the mount effects are dropped, and the parent's _children still holds the old vnode, but the component's _vnode is the failed one, whose _children are the vnodes created during the failed render (with _dom pointing at created, never-inserted elements).
• The next renderComponent uses component._vnode as oldVNode. Its children are matched as existing vnodes: oldVNode.ref == childVNode.ref so nothing is pushed to refQueue (children.js:113), oldVNode._original is set so they are not treated as mounting (INSERT_VNODE not set), and diffElementNodes reuses their _dom. This render succeeds and writes itself into the parent's _children.
• On resolution, removeOriginal (compat/src/suspense.js:73-96) does originalParent.appendChild(vnode._dom) for that component, and the restore diff (every restored vnode has _original == null, hence INSERT_VNODE) places the element at the right position. The element is now in the document, with props applied, but its ref and mount effects never ran and never will.

Suggested fix for 1 (validated on the reproduction)

Treat a reference node that is not a child of parentDom like a missing one:

--- a/src/diff/children.js
+++ b/src/diff/children.js
@@ -367,6 +367,11 @@
 			if (oldDom && parentVNode.type && !oldDom.parentNode) {
 				oldDom = getDomSibling(parentVNode);
 			}
+			// A reference node living in another container cannot be a reference
+			// here: append instead, like a missing reference.
+			if (oldDom && oldDom.parentNode !== parentDom) {
+				oldDom = NULL;
+			}
 			parentDom.insertBefore(parentVNode._dom, oldDom || NULL);
 		}
 		oldDom = parentVNode._dom;

With this patch the reproduction logs no error, barRef is called during step 2, and after step 4 the document is <header id="bar" style="color: red;">bar</header><div id="page">…</div><footer>foot</footer> (correct order, ref fired once). Verified on 11.0.0-beta.2 src/; the same lines exist in 10.29.8 (children.js:363-366) and 11.0.0-rc.2 (children.js:404-412, before the moveBefore branch).

Note on 2

Even with 1 fixed, any exception thrown from inside diffChildren during renderComponent leaves c._vnode pointing at a vnode whose subtree was created but never committed, and the next render of that component adopts it as mounted. Restoring c._vnode = oldVNode (or discarding newVNode._children) in the catch of diff() when the error is not a suspension would keep a failed render from being mistaken for a committed one. I have not validated that change; mentioning it because it is what turns a one-off exception into permanent, invisible damage.

Where this was found

An app where a fixed top bar is rendered by a slot component (a Box writing its style from a ref callback) next to a routed page area, all under one <Suspense>. A component at that level delegated its wait to the boundary, so the whole shell was parked for the duration of a request; @preact/signals kept re-rendering the parked components as the URL changed, the slot mounted the bar while parked, and the bar stayed unstyled for the life of the document (ref never called, layout effect never run). Only a reload fixed it.