<Transition> wrapped by <Suspense> breaks entirely if interrupted before it completes

[danielroe]

Vue version

3.2.47

Link to minimal reproduction

https://stackblitz.com/edit/github-z3ry59-hbu5x7
SFC Playground

Steps to reproduce

<template>
  <transition name="page" mode="out-in" :duration="300">
    <Suspense>
      <component :is="Component" />
    </Suspense>
  </transition>
</template>

Click the button marked 'Trigger error'.

This will switch components within the Transition. On next tick, it will switch them back. (To reproduce, it's sufficient to switch them at any point before the transition has finished.)

Note that this follows the component order specified in https://vuejs.org/guide/built-ins/suspense.html#combining-with-other-components.

What is expected?

I expect no errors.

What is actually happening?

The following error is thrown:

Uncaught DOMException: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.

In addition, the content of the Suspense slot is removed and the page remains broken.

System Info

No response

Any additional comments?

No response

[posva]

This rings a bell, there could be another open issue about interrupting a transition here but I couldn't find it. It looks similar to #6835 but clearly not the same.

[kikuchan]

After some investigations, I've found a workaround to relief the issue with the following patch.

--- packages/runtime-dom/src/nodeOps.ts
+++ packages/runtime-dom/src/nodeOps.ts
@@ -6,8 +6,18 @@ const doc = (typeof document !== 'undefined' ? document : null) as Document

 const templateContainer = doc && /*#__PURE__*/ doc.createElement('template')

+function isSafeAnchor(parent: Element, anchor: Node | null | undefined) {
+  if (!anchor) return true;
+
+  for (let node: Node | null = anchor; node; node = node.parentNode) {
+    if (parent === node) return true;
+  }
+  return false;
+}
+
 export const nodeOps: Omit<RendererOptions<Node, Element>, 'patchProp'> = {
   insert: (child, parent, anchor) => {
+    if (!isSafeAnchor(parent, anchor)) anchor = null;
     parent.insertBefore(child, anchor || null)
   },

It also works on nuxt/nuxt#13350

I also came up with logs that what's happend internally;
(using modified reproduction code that toggle only toggles components once)

Good case:

1. click the toggle button
2. insert B to hiddenContainer created in mountSuspense (creation?)
3. remove A from real DOM
4. insert B to real DOM, without anchor
5. wait for a while, and click the toggle button again.
6. insert A to hiddenContainer created in mountSuspense (creation?)
7. remove B from real DOM
8. insert A to real DOM, without anchor

Bad case:

1. click the toggle button
2. insert B to hiddenContainer created in mountSuspense (creation?)
3. click the toggle button again quickly, then component swap occurs again too rapidly.
4. insert A to the same (!?) hiddenContainer created in mountSuspense (creation?)
5. remove B from the hiddenContainer
6. remove A from real DOM
7. insert A to real DOM, with anchor A (!)

I hope it helps.

[kikuchan]

core/packages/runtime-core/src/components/Suspense.ts

Lines 497 to 520 in d2c3d8b

	const delayEnter =
	activeBranch &&
	pendingBranch!.transition &&
	pendingBranch!.transition.mode === 'out-in'
	if (delayEnter) {
	activeBranch!.transition!.afterLeave = () => {
	if (pendingId === suspense.pendingId) {
	move(pendingBranch!, container, anchor, MoveType.ENTER)
	}
	}
	}
	// this is initial anchor on mount
	let { anchor } = suspense
	// unmount current active tree
	if (activeBranch) {
	// if the fallback tree was mounted, it may have been moved
	// as part of a parent suspense. get the latest anchor for insertion
	anchor = next(activeBranch)
	unmount(activeBranch, parentComponent, suspense, true)
	}
	if (!delayEnter) {
	// move content from off-dom container to actual container
	move(pendingBranch!, container, anchor, MoveType.ENTER)
	}

The issue happens when activeBranch is truthy, and regardless of whether delayEnter is true or not.
And, I think the anchor re-selection in the if (activeBranch) block needs to be robust (but I don't know how, yet)

Just to be confirmed, removing anchor = next(activeBranch) also resolves the issue without the previous patch. (it fails some tests)

[kikuchan]

After deep investigation, I've finally found the solution.

--- packages/runtime-core/src/renderer.ts
+++ packages/runtime-core/src/renderer.ts
@@ -2035,6 +2035,7 @@ function baseCreateRenderer(
     if (needTransition) {
       if (moveType === MoveType.ENTER) {
         transition!.beforeEnter(el!)
+        if (anchor && anchor.parent !== container) anchor = null;
         hostInsert(el!, container, anchor)
         queuePostRenderEffect(() => transition!.enter(el!), parentSuspense)
       } else {

The beforeEnter internally calls afterLeave hooks, and it actually removes the anchor from the container before use, thus hostInsert claims it doesn't exist in the tree.

Perhaps, it should be re-calculate anchor here, or you could call afterLeave hooks manually to remove a transition node before anchor selection, but I think it's hard to do so here.

[kikuchan]

I've posted a PR (based on another idea), and I've noticed that the same pattern causes the same error, btw.

For example, following lines cause a HMR issue. (failed to reload a page sometimes)

core/packages/runtime-core/src/renderer.ts

Lines 374 to 375 in 9f8e98a

	anchor = getNextHostNode(n1)
	unmount(n1, parentComponent, parentSuspense, true)

After patching the same way in the PR like this, it has been resolved.

      const anchorCands: RendererNode[] = []
      for (let node = getNextHostNode(n1); node; node = hostNextSibling(node)) {
        anchorCands.push(node)
      }
      unmount(n1, parentComponent, parentSuspense, true)
      anchor = anchorCands.find(x => hostParentNode(x) === container) || null;

In current code, unmount calls vnode's hooks internally, and they could remove multiple nodes at once.
Thus, I think saving a next node as an anchor before unmount is not a safe opearation anymore.

[kikuchan]

I'm trying to explain what's going on under the hood.

For this issue, there are 2 cases:

1. In this case, the activeBranch is still in the hiddenContainer (I don't know if this is a valid state).
   Then, move tries to move the pendingBranch where the activeBranch is, and it fails because the anchor (where the activeBranch was) is not in the container.

2. In the !delayEnter case, the anchor is properly in the container, but move triggers transition.beforeEnter and hooks alter the DOM tree, thus the anchor no longer exists in the container on the very next hostInsert.
   This could be another issue; it happens with my code at least, but I haven't created a minimal test code yet.

Additionally:

3. This is clearly another issue. unmount removes multiple nodes at once, so capturing a single anchor before the unmount is not a safe operation, especially reloading by HMR causes it sometimes (I haven't found out the issue number yet).

[avaz]

Yay!! 🙌🏼

[vanling]

🎉 Thank you @edison1105

[bgrand-ch]

Thank you to everyone involved in resolving this problem 🙏

[dargmuesli]

When I opened my notifications today

Thanks a lot everyone and thank you @yyx990803 for getting the merge train rolling!