Subscribe on changes!

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

avatar
Apr 17th 2023

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

avatar
Apr 18th 2023

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 https://github.com/vuejs/core/issues/6835 but clearly not the same.

avatar
Jun 21st 2023

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 https://github.com/nuxt/nuxt/issues/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.

avatar
Jun 22nd 2023

https://github.com/vuejs/core/blob/d2c3d8b70b2df6e16f053a7ac58e6b04e7b2078f/packages/runtime-core/src/components/Suspense.ts#L497-L520

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)

avatar
Jun 25th 2023

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.

avatar
Jun 27th 2023

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) https://github.com/vuejs/core/blob/9f8e98af891f456cc8cc9019a31704e5534d1f08/packages/runtime-core/src/renderer.ts#L374-L375 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.

avatar
Jun 29th 2023

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:

  1. 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).
avatar
Dec 4th 2023

Yay!! 🙌🏼

avatar
Dec 4th 2023

🎉 Thank you @edison1105

avatar
Dec 4th 2023

Thank you to everyone involved in resolving this problem 🙏

avatar
Dec 4th 2023

When I opened my notifications today

Alt Text

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