Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(runtime-core): fix suspense crash when patch non-resolved async setup component #7290

Merged
merged 13 commits into from
Dec 12, 2023

Conversation

mmis1000
Copy link
Contributor

@mmis1000 mmis1000 commented Dec 7, 2022

The previous one is closed because I rename the source patch.

Fixes: #6949
Fixes: #6463

#6949

This is a patch that intended to properly fix crash caused by #6949 , a crash caused by race condition between the component update and component hydration.

There are actually two bugs triggered by doing such actions.

  1. The suspense boundary try to render the fallback content even it totally shouldn't (We already have the content of default slot because SSR)
  2. The update of the component that wrap async component crashed
    a. Because host node relies on the subTree of async component it wraps.
    b. And the async component is not rendered yet.

This pull request does two things.

  1. Make patch of fallback content during hydration no-op
  2. Delay the patch of content when the subTree is missing until the async children resolved.

There are a few things that need to be addressed before this being a proper pull request.

  • Is delaying update a safe way to handle the race, would it be safer to just ignore it?
    • seems it will desync otherwise
  • Is go one level down for find a async root component deep enough? Do I need to go recursively?
    • It indeed need to
  • Is it okay to just ignore vNode of fallback content in a hydrating suspense boundary?

#6463

And this also fix #6463, a crash caused by missing placeholder element in async element initialVNode (thus subTree.el of parent component).

This pull request fix the initial mounting of non-resolved async element. So the el of initialVNode isn't null at initial mounting

In non-async elements, the el of initialVNode gets set up during the setupRenderEffect( call. But async element mounting skips that, result in a initialVNode without el field set. This pull request address this by manually set the initialVNode.el to placeholder element it is currently using.

Besides these, proper tests are added so we can confirm that this issue is indeed patched.
Or it can be checkout into current version to see how it breaks currently.

@mmis1000 mmis1000 marked this pull request as ready for review December 14, 2022 17:26
@mmis1000 mmis1000 changed the title Fix/suspense hydration patch crash fix(runtime-dom): fix suspense crash when patch non-resolved async setup component during hydration Dec 14, 2022
@mmis1000 mmis1000 changed the title fix(runtime-dom): fix suspense crash when patch non-resolved async setup component during hydration fix(runtime-core): fix suspense crash when patch non-resolved async setup component during hydration Dec 14, 2022
@KaelWD
Copy link
Contributor

KaelWD commented Apr 9, 2023

This doesn't seem to fix #6095 when patched into vuetify, prevTree.el and instance.subTree are still null.
To reproduce:

  • clone vuetify
  • checkout fix/15207-suspense-groups
  • add the following patch to patches/@vue+runtime-core+3.2.47.patch
diff --git a/node_modules/@vue/runtime-core/dist/runtime-core.cjs.js b/node_modules/@vue/runtime-core/dist/runtime-core.cjs.js
index 8442995..b40bfd1 100644
--- a/node_modules/@vue/runtime-core/dist/runtime-core.cjs.js
+++ b/node_modules/@vue/runtime-core/dist/runtime-core.cjs.js
@@ -1246,9 +1246,15 @@ function patchSuspense(n1, n2, container, anchor, parentComponent, isSVG, slotSc
                 suspense.resolve();
             }
             else if (isInFallback) {
-                patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
-                isSVG, slotScopeIds, optimized);
-                setActiveBranch(suspense, newFallback);
+                // It's possible that the app is in hydrating state when patching the suspense instance
+                //   if someone update the dependency during component setup in children of suspense boundary
+                // And that would be problemtic because we aren't actually showing a fallback content when patchSuspense is called.
+                // In such case, patch of fallback content should be no op
+                if (!isHydrating) {
+                    patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
+                    isSVG, slotScopeIds, optimized);
+                    setActiveBranch(suspense, newFallback);
+                }
             }
         }
         else {
@@ -5534,6 +5540,7 @@ function baseCreateRenderer(options, createHydrationFns) {
     };
     const setupRenderEffect = (instance, initialVNode, container, anchor, parentSuspense, isSVG, optimized) => {
         const componentUpdateFn = () => {
+            var _a;
             if (!instance.isMounted) {
                 let vnodeHook;
                 const { el, props } = initialVNode;
@@ -5624,6 +5631,47 @@ function baseCreateRenderer(options, createHydrationFns) {
                 initialVNode = container = anchor = null;
             }
             else {
+                {
+                    const locateNonHydratedAsyncRoot = (instance) => {
+                        var _a, _b;
+                        if (instance.subTree.shapeFlag & 6 /* ShapeFlags.COMPONENT */) {
+                            if (
+                            // this happens only during hydration
+                            ((_a = instance.subTree.component) === null || _a === void 0 ? void 0 : _a.subTree) == null &&
+                                // we don't know the subTree yet because we haven't resolve it
+                                ((_b = instance.subTree.component) === null || _b === void 0 ? void 0 : _b.asyncResolved) === false) {
+                                return instance.subTree.component;
+                            }
+                            else {
+                                return locateNonHydratedAsyncRoot(instance.subTree.component);
+                            }
+                        }
+                        else {
+                            return null;
+                        }
+                    };
+                    const nonHydratedAsyncRoot = locateNonHydratedAsyncRoot(instance);
+                    // we are trying to update some async comp before hydration
+                    // this will cause crash because we don't know the root node yet
+                    if (nonHydratedAsyncRoot != null) {
+                        // only sync the properties and abort the rest of operations
+                        let { next, vnode } = instance;
+                        toggleRecurse(instance, false);
+                        if (next) {
+                            next.el = vnode.el;
+                            updateComponentPreRender(instance, next, optimized);
+                        }
+                        toggleRecurse(instance, true);
+                        // and continue the rest of operations once the deps are resolved
+                        (_a = nonHydratedAsyncRoot.asyncDep) === null || _a === void 0 ? void 0 : _a.then(() => {
+                            // the instance may be destroyed during the time period
+                            if (!instance.isUnmounted) {
+                                componentUpdateFn();
+                            }
+                        });
+                        return;
+                    }
+                }
                 // updateComponent
                 // This is triggered by mutation of component's own state (next: null)
                 // OR parent calling processComponent (next: VNode)
diff --git a/node_modules/@vue/runtime-core/dist/runtime-core.cjs.prod.js b/node_modules/@vue/runtime-core/dist/runtime-core.cjs.prod.js
index 34d0fa4..15b985b 100644
--- a/node_modules/@vue/runtime-core/dist/runtime-core.cjs.prod.js
+++ b/node_modules/@vue/runtime-core/dist/runtime-core.cjs.prod.js
@@ -721,9 +721,15 @@ function patchSuspense(n1, n2, container, anchor, parentComponent, isSVG, slotSc
                 suspense.resolve();
             }
             else if (isInFallback) {
-                patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
-                isSVG, slotScopeIds, optimized);
-                setActiveBranch(suspense, newFallback);
+                // It's possible that the app is in hydrating state when patching the suspense instance
+                //   if someone update the dependency during component setup in children of suspense boundary
+                // And that would be problemtic because we aren't actually showing a fallback content when patchSuspense is called.
+                // In such case, patch of fallback content should be no op
+                if (!isHydrating) {
+                    patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
+                    isSVG, slotScopeIds, optimized);
+                    setActiveBranch(suspense, newFallback);
+                }
             }
         }
         else {
@@ -4297,6 +4303,7 @@ function baseCreateRenderer(options, createHydrationFns) {
     };
     const setupRenderEffect = (instance, initialVNode, container, anchor, parentSuspense, isSVG, optimized) => {
         const componentUpdateFn = () => {
+            var _a;
             if (!instance.isMounted) {
                 let vnodeHook;
                 const { el, props } = initialVNode;
@@ -4360,6 +4367,47 @@ function baseCreateRenderer(options, createHydrationFns) {
                 initialVNode = container = anchor = null;
             }
             else {
+                {
+                    const locateNonHydratedAsyncRoot = (instance) => {
+                        var _a, _b;
+                        if (instance.subTree.shapeFlag & 6 /* ShapeFlags.COMPONENT */) {
+                            if (
+                            // this happens only during hydration
+                            ((_a = instance.subTree.component) === null || _a === void 0 ? void 0 : _a.subTree) == null &&
+                                // we don't know the subTree yet because we haven't resolve it
+                                ((_b = instance.subTree.component) === null || _b === void 0 ? void 0 : _b.asyncResolved) === false) {
+                                return instance.subTree.component;
+                            }
+                            else {
+                                return locateNonHydratedAsyncRoot(instance.subTree.component);
+                            }
+                        }
+                        else {
+                            return null;
+                        }
+                    };
+                    const nonHydratedAsyncRoot = locateNonHydratedAsyncRoot(instance);
+                    // we are trying to update some async comp before hydration
+                    // this will cause crash because we don't know the root node yet
+                    if (nonHydratedAsyncRoot != null) {
+                        // only sync the properties and abort the rest of operations
+                        let { next, vnode } = instance;
+                        toggleRecurse(instance, false);
+                        if (next) {
+                            next.el = vnode.el;
+                            updateComponentPreRender(instance, next, optimized);
+                        }
+                        toggleRecurse(instance, true);
+                        // and continue the rest of operations once the deps are resolved
+                        (_a = nonHydratedAsyncRoot.asyncDep) === null || _a === void 0 ? void 0 : _a.then(() => {
+                            // the instance may be destroyed during the time period
+                            if (!instance.isUnmounted) {
+                                componentUpdateFn();
+                            }
+                        });
+                        return;
+                    }
+                }
                 // updateComponent
                 // This is triggered by mutation of component's own state (next: null)
                 // OR parent calling processComponent (next: VNode)
diff --git a/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js b/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js
index f641a23..0ac88b1 100644
--- a/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js
+++ b/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js
@@ -1255,9 +1255,15 @@ function patchSuspense(n1, n2, container, anchor, parentComponent, isSVG, slotSc
                 suspense.resolve();
             }
             else if (isInFallback) {
-                patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
-                isSVG, slotScopeIds, optimized);
-                setActiveBranch(suspense, newFallback);
+                // It's possible that the app is in hydrating state when patching the suspense instance
+                //   if someone update the dependency during component setup in children of suspense boundary
+                // And that would be problemtic because we aren't actually showing a fallback content when patchSuspense is called.
+                // In such case, patch of fallback content should be no op
+                if (!isHydrating) {
+                    patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
+                    isSVG, slotScopeIds, optimized);
+                    setActiveBranch(suspense, newFallback);
+                }
             }
         }
         else {
@@ -5596,6 +5602,7 @@ function baseCreateRenderer(options, createHydrationFns) {
     };
     const setupRenderEffect = (instance, initialVNode, container, anchor, parentSuspense, isSVG, optimized) => {
         const componentUpdateFn = () => {
+            var _a;
             if (!instance.isMounted) {
                 let vnodeHook;
                 const { el, props } = initialVNode;
@@ -5686,6 +5693,47 @@ function baseCreateRenderer(options, createHydrationFns) {
                 initialVNode = container = anchor = null;
             }
             else {
+                {
+                    const locateNonHydratedAsyncRoot = (instance) => {
+                        var _a, _b;
+                        if (instance.subTree.shapeFlag & 6 /* ShapeFlags.COMPONENT */) {
+                            if (
+                            // this happens only during hydration
+                            ((_a = instance.subTree.component) === null || _a === void 0 ? void 0 : _a.subTree) == null &&
+                                // we don't know the subTree yet because we haven't resolve it
+                                ((_b = instance.subTree.component) === null || _b === void 0 ? void 0 : _b.asyncResolved) === false) {
+                                return instance.subTree.component;
+                            }
+                            else {
+                                return locateNonHydratedAsyncRoot(instance.subTree.component);
+                            }
+                        }
+                        else {
+                            return null;
+                        }
+                    };
+                    const nonHydratedAsyncRoot = locateNonHydratedAsyncRoot(instance);
+                    // we are trying to update some async comp before hydration
+                    // this will cause crash because we don't know the root node yet
+                    if (nonHydratedAsyncRoot != null) {
+                        // only sync the properties and abort the rest of operations
+                        let { next, vnode } = instance;
+                        toggleRecurse(instance, false);
+                        if (next) {
+                            next.el = vnode.el;
+                            updateComponentPreRender(instance, next, optimized);
+                        }
+                        toggleRecurse(instance, true);
+                        // and continue the rest of operations once the deps are resolved
+                        (_a = nonHydratedAsyncRoot.asyncDep) === null || _a === void 0 ? void 0 : _a.then(() => {
+                            // the instance may be destroyed during the time period
+                            if (!instance.isUnmounted) {
+                                componentUpdateFn();
+                            }
+                        });
+                        return;
+                    }
+                }
                 // updateComponent
                 // This is triggered by mutation of component's own state (next: null)
                 // OR parent calling processComponent (next: VNode)
  • run yarn
  • add the following template to packages/vuetify/dev/Playground.vue
<template>
  <v-app>
    <v-container>
      <v-card>
        <v-btn-toggle v-model="selected" multiple variant="outlined">
          <v-btn v-for="server in items" :key="server">
            Server {{ server }}
          </v-btn>
        </v-btn-toggle>
        <br>
        You select: {{ selected }}
      </v-card>

      <v-card>
        <v-tabs>
          <v-tab>One</v-tab>
          <v-tab>Two</v-tab>
          <v-tab>Three</v-tab>
        </v-tabs>
      </v-card>

      <v-item-group>
        <v-item>
          <div>a</div>
          <div>b</div>
        </v-item>
      </v-item-group>

    </v-container>
  </v-app>
</template>

<script setup>
  import { ref } from 'vue'

  const items = [0, 1, 2]
  const selected = ref([...items])
</script>
  • run yarn dev from packages/vuetify

@pikax
Copy link
Member

pikax commented Oct 20, 2023

Based on @KaelWD comments this does not seem to fix what's intended to fix, there's some checkboxes unchecked on the PR.

@mmis1000 are you still planning to work on this?

@mmis1000
Copy link
Contributor Author

mmis1000 commented Oct 24, 2023

Based on @KaelWD comments this does not seem to fix what's intended to fix, there's some checkboxes unchecked on the PR.

@mmis1000 are you still planning to work on this?

This pull request indeed does not address the problem caused by using suspense and transition (or keepalive?) at same time.

It only handles the one caused by hydration and update on setup. About transition and keepAlive, I may need to investigate what exactly path that cause it to be null to get a proper fix. Which I haven't done yet.

@mmis1000 mmis1000 force-pushed the fix/suspense-hydration-patch-crash branch from 59cfe63 to 084c2a4 Compare October 24, 2023 15:03
@netlify
Copy link

netlify bot commented Oct 24, 2023

Deploy Preview for vue-next-template-explorer failed.

Name Link
🔨 Latest commit 2c5447b
🔍 Latest deploy log https://app.netlify.com/sites/vue-next-template-explorer/deploys/6537e022406a930008767ded

@netlify
Copy link

netlify bot commented Oct 24, 2023

Deploy Preview for vue-sfc-playground failed.

Name Link
🔨 Latest commit 2c5447b
🔍 Latest deploy log https://app.netlify.com/sites/vue-sfc-playground/deploys/6537e0221ac96d00087ea17c

@mmis1000
Copy link
Contributor Author

mmis1000 commented Oct 24, 2023

@KaelWD I changed it to delay the update regard of what or why it caused update, can you try if this allow https://github.com/vuetifyjs/vuetify/tree/fix/15207-suspense-groups to run normally?

Besides that, it seems introducing that tick messed up the timing of certain transition test. I probably also need to figure out how to fix the test or transition code.

@mmis1000 mmis1000 force-pushed the fix/suspense-hydration-patch-crash branch from 75569ef to 390104d Compare October 27, 2023 18:43
@mmis1000
Copy link
Contributor Author

mmis1000 commented Oct 27, 2023

@pikax I think I also find the root cause of client side suspense crash caused by update before async dep resolved.

It seems the initialVNode.el of any async setup component is null. Which cause its parent component's instance.subTree.el to be null if its parent only contain exactly one children (which is the async setup component itself)

But given the root cause is actually a separated bug, I wonder if I should change the title so it actually indicate what it fixes.

@KaelWD can you try if this updated patch fix the crash? This should fix all client side suspense crash caused by missing el in instance.subTree

Updated patch
diff --git a/node_modules/@vue/runtime-core/dist/runtime-core.cjs.js b/node_modules/@vue/runtime-core/dist/runtime-core.cjs.js
index 8442995..beb70b9 100644
--- a/node_modules/@vue/runtime-core/dist/runtime-core.cjs.js
+++ b/node_modules/@vue/runtime-core/dist/runtime-core.cjs.js
@@ -1246,9 +1246,15 @@ function patchSuspense(n1, n2, container, anchor, parentComponent, isSVG, slotSc
                 suspense.resolve();
             }
             else if (isInFallback) {
-                patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
-                isSVG, slotScopeIds, optimized);
-                setActiveBranch(suspense, newFallback);
+                // It's possible that the app is in hydrating state when patching the suspense instance
+                //   if someone update the dependency during component setup in children of suspense boundary
+                // And that would be problemtic because we aren't actually showing a fallback content when patchSuspense is called.
+                // In such case, patch of fallback content should be no op
+                if (!isHydrating) {
+                    patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
+                    isSVG, slotScopeIds, optimized);
+                    setActiveBranch(suspense, newFallback);
+                }
             }
         }
         else {
@@ -5491,6 +5497,7 @@ function baseCreateRenderer(options, createHydrationFns) {
             if (!initialVNode.el) {
                 const placeholder = (instance.subTree = createVNode(Comment));
                 processCommentNode(null, placeholder, container, anchor);
+                initialVNode.el = placeholder.el;
             }
             return;
         }
@@ -5534,6 +5541,7 @@ function baseCreateRenderer(options, createHydrationFns) {
     };
     const setupRenderEffect = (instance, initialVNode, container, anchor, parentSuspense, isSVG, optimized) => {
         const componentUpdateFn = () => {
+            var _a;
             if (!instance.isMounted) {
                 let vnodeHook;
                 const { el, props } = initialVNode;
@@ -5624,6 +5632,47 @@ function baseCreateRenderer(options, createHydrationFns) {
                 initialVNode = container = anchor = null;
             }
             else {
+                {
+                    const locateNonHydratedAsyncRoot = (instance) => {
+                        var _a, _b;
+                        if (instance.subTree.shapeFlag & 6 /* ShapeFlags.COMPONENT */) {
+                            if (
+                            // this happens only during hydration
+                            ((_a = instance.subTree.component) === null || _a === void 0 ? void 0 : _a.subTree) == null &&
+                                // we don't know the subTree yet because we haven't resolve it
+                                ((_b = instance.subTree.component) === null || _b === void 0 ? void 0 : _b.asyncResolved) === false) {
+                                return instance.subTree.component;
+                            }
+                            else {
+                                return locateNonHydratedAsyncRoot(instance.subTree.component);
+                            }
+                        }
+                        else {
+                            return null;
+                        }
+                    };
+                    const nonHydratedAsyncRoot = locateNonHydratedAsyncRoot(instance);
+                    // we are trying to update some async comp before hydration
+                    // this will cause crash because we don't know the root node yet
+                    if (nonHydratedAsyncRoot != null) {
+                        // only sync the properties and abort the rest of operations
+                        let { next, vnode } = instance;
+                        toggleRecurse(instance, false);
+                        if (next) {
+                            next.el = vnode.el;
+                            updateComponentPreRender(instance, next, optimized);
+                        }
+                        toggleRecurse(instance, true);
+                        // and continue the rest of operations once the deps are resolved
+                        (_a = nonHydratedAsyncRoot.asyncDep) === null || _a === void 0 ? void 0 : _a.then(() => {
+                            // the instance may be destroyed during the time period
+                            if (!instance.isUnmounted) {
+                                componentUpdateFn();
+                            }
+                        });
+                        return;
+                    }
+                }
                 // updateComponent
                 // This is triggered by mutation of component's own state (next: null)
                 // OR parent calling processComponent (next: VNode)
diff --git a/node_modules/@vue/runtime-core/dist/runtime-core.cjs.prod.js b/node_modules/@vue/runtime-core/dist/runtime-core.cjs.prod.js
index 34d0fa4..93bcbb2 100644
--- a/node_modules/@vue/runtime-core/dist/runtime-core.cjs.prod.js
+++ b/node_modules/@vue/runtime-core/dist/runtime-core.cjs.prod.js
@@ -721,9 +721,15 @@ function patchSuspense(n1, n2, container, anchor, parentComponent, isSVG, slotSc
                 suspense.resolve();
             }
             else if (isInFallback) {
-                patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
-                isSVG, slotScopeIds, optimized);
-                setActiveBranch(suspense, newFallback);
+                // It's possible that the app is in hydrating state when patching the suspense instance
+                //   if someone update the dependency during component setup in children of suspense boundary
+                // And that would be problemtic because we aren't actually showing a fallback content when patchSuspense is called.
+                // In such case, patch of fallback content should be no op
+                if (!isHydrating) {
+                    patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
+                    isSVG, slotScopeIds, optimized);
+                    setActiveBranch(suspense, newFallback);
+                }
             }
         }
         else {
@@ -4266,6 +4272,7 @@ function baseCreateRenderer(options, createHydrationFns) {
             if (!initialVNode.el) {
                 const placeholder = (instance.subTree = createVNode(Comment));
                 processCommentNode(null, placeholder, container, anchor);
+                initialVNode.el = placeholder.el;
             }
             return;
         }
@@ -4297,6 +4304,7 @@ function baseCreateRenderer(options, createHydrationFns) {
     };
     const setupRenderEffect = (instance, initialVNode, container, anchor, parentSuspense, isSVG, optimized) => {
         const componentUpdateFn = () => {
+            var _a;
             if (!instance.isMounted) {
                 let vnodeHook;
                 const { el, props } = initialVNode;
@@ -4360,6 +4368,47 @@ function baseCreateRenderer(options, createHydrationFns) {
                 initialVNode = container = anchor = null;
             }
             else {
+                {
+                    const locateNonHydratedAsyncRoot = (instance) => {
+                        var _a, _b;
+                        if (instance.subTree.shapeFlag & 6 /* ShapeFlags.COMPONENT */) {
+                            if (
+                            // this happens only during hydration
+                            ((_a = instance.subTree.component) === null || _a === void 0 ? void 0 : _a.subTree) == null &&
+                                // we don't know the subTree yet because we haven't resolve it
+                                ((_b = instance.subTree.component) === null || _b === void 0 ? void 0 : _b.asyncResolved) === false) {
+                                return instance.subTree.component;
+                            }
+                            else {
+                                return locateNonHydratedAsyncRoot(instance.subTree.component);
+                            }
+                        }
+                        else {
+                            return null;
+                        }
+                    };
+                    const nonHydratedAsyncRoot = locateNonHydratedAsyncRoot(instance);
+                    // we are trying to update some async comp before hydration
+                    // this will cause crash because we don't know the root node yet
+                    if (nonHydratedAsyncRoot != null) {
+                        // only sync the properties and abort the rest of operations
+                        let { next, vnode } = instance;
+                        toggleRecurse(instance, false);
+                        if (next) {
+                            next.el = vnode.el;
+                            updateComponentPreRender(instance, next, optimized);
+                        }
+                        toggleRecurse(instance, true);
+                        // and continue the rest of operations once the deps are resolved
+                        (_a = nonHydratedAsyncRoot.asyncDep) === null || _a === void 0 ? void 0 : _a.then(() => {
+                            // the instance may be destroyed during the time period
+                            if (!instance.isUnmounted) {
+                                componentUpdateFn();
+                            }
+                        });
+                        return;
+                    }
+                }
                 // updateComponent
                 // This is triggered by mutation of component's own state (next: null)
                 // OR parent calling processComponent (next: VNode)
diff --git a/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js b/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js
index f641a23..a2956ed 100644
--- a/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js
+++ b/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js
@@ -1255,9 +1255,15 @@ function patchSuspense(n1, n2, container, anchor, parentComponent, isSVG, slotSc
                 suspense.resolve();
             }
             else if (isInFallback) {
-                patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
-                isSVG, slotScopeIds, optimized);
-                setActiveBranch(suspense, newFallback);
+                // It's possible that the app is in hydrating state when patching the suspense instance
+                //   if someone update the dependency during component setup in children of suspense boundary
+                // And that would be problemtic because we aren't actually showing a fallback content when patchSuspense is called.
+                // In such case, patch of fallback content should be no op
+                if (!isHydrating) {
+                    patch(activeBranch, newFallback, container, anchor, parentComponent, null, // fallback tree will not have suspense context
+                    isSVG, slotScopeIds, optimized);
+                    setActiveBranch(suspense, newFallback);
+                }
             }
         }
         else {
@@ -5553,6 +5559,7 @@ function baseCreateRenderer(options, createHydrationFns) {
             if (!initialVNode.el) {
                 const placeholder = (instance.subTree = createVNode(Comment));
                 processCommentNode(null, placeholder, container, anchor);
+                initialVNode.el = placeholder.el;
             }
             return;
         }
@@ -5596,6 +5603,7 @@ function baseCreateRenderer(options, createHydrationFns) {
     };
     const setupRenderEffect = (instance, initialVNode, container, anchor, parentSuspense, isSVG, optimized) => {
         const componentUpdateFn = () => {
+            var _a;
             if (!instance.isMounted) {
                 let vnodeHook;
                 const { el, props } = initialVNode;
@@ -5686,6 +5694,47 @@ function baseCreateRenderer(options, createHydrationFns) {
                 initialVNode = container = anchor = null;
             }
             else {
+                {
+                    const locateNonHydratedAsyncRoot = (instance) => {
+                        var _a, _b;
+                        if (instance.subTree.shapeFlag & 6 /* ShapeFlags.COMPONENT */) {
+                            if (
+                            // this happens only during hydration
+                            ((_a = instance.subTree.component) === null || _a === void 0 ? void 0 : _a.subTree) == null &&
+                                // we don't know the subTree yet because we haven't resolve it
+                                ((_b = instance.subTree.component) === null || _b === void 0 ? void 0 : _b.asyncResolved) === false) {
+                                return instance.subTree.component;
+                            }
+                            else {
+                                return locateNonHydratedAsyncRoot(instance.subTree.component);
+                            }
+                        }
+                        else {
+                            return null;
+                        }
+                    };
+                    const nonHydratedAsyncRoot = locateNonHydratedAsyncRoot(instance);
+                    // we are trying to update some async comp before hydration
+                    // this will cause crash because we don't know the root node yet
+                    if (nonHydratedAsyncRoot != null) {
+                        // only sync the properties and abort the rest of operations
+                        let { next, vnode } = instance;
+                        toggleRecurse(instance, false);
+                        if (next) {
+                            next.el = vnode.el;
+                            updateComponentPreRender(instance, next, optimized);
+                        }
+                        toggleRecurse(instance, true);
+                        // and continue the rest of operations once the deps are resolved
+                        (_a = nonHydratedAsyncRoot.asyncDep) === null || _a === void 0 ? void 0 : _a.then(() => {
+                            // the instance may be destroyed during the time period
+                            if (!instance.isUnmounted) {
+                                componentUpdateFn();
+                            }
+                        });
+                        return;
+                    }
+                }
                 // updateComponent
                 // This is triggered by mutation of component's own state (next: null)
                 // OR parent calling processComponent (next: VNode)

@mmis1000
Copy link
Contributor Author

mmis1000 commented Nov 2, 2023

I made a minimal reproduction of root cause of #6095

playground link

@edison1105
Copy link
Member

edison1105 commented Nov 2, 2023

I made a minimal reproduction of root cause of #6095

playground link

@mmis1000
Is this a reproduction of #6059? You seem to have changed your reproduction.
If it's true, it's similar to #6463. the root cause see #6463 (comment)
there is a workaround, add a dynamic key to the Comp. see

  <Suspense>
    <Comp :key="data" :data="data"/>
  </Suspense>

But this reproduction is not the same as the reproduction you updated

@mmis1000
Copy link
Contributor Author

mmis1000 commented Nov 2, 2023

I made a minimal reproduction of root cause of #6095
playground link

@mmis1000 Is this a reproduction of #6059? You seem to have changed your reproduction. If it's true, it's similar to #6463 there is a workaround, add a dynamic key to the Comp. see

  <Suspense>
    <Comp :key="data" :data="data"/>
  </Suspense>

But this reproduction is not the same as the reproduction you updated

They both crash on the same path for same reason. So I think I should use the simpler one as minimal reproduction. I haven't investigate the vnode.component is null error on the simpler one yet. So I don't know what is this from and whether it is part of this issue or not.

Any way, neither of the crashes will happen in latest pull request preview (also the simpler one).

btw: if you open the console, you would notice the simpler one also has Uncaught (in promise) TypeError: node is null error triggered in componentUpdateFn

@KaelWD
Copy link
Contributor

KaelWD commented Nov 2, 2023

Yep, this PR seems to fix vuetifyjs/vuetify#15215
I am getting hydration mismatch warnings without SSR now but that might just be a vite-ssr thing.

@mmis1000 mmis1000 changed the title fix(runtime-core): fix suspense crash when patch non-resolved async setup component during hydration fix(runtime-core): fix suspense crash when patch non-resolved async setup component Nov 3, 2023
@yyx990803 yyx990803 force-pushed the fix/suspense-hydration-patch-crash branch from eac3602 to fbcdc72 Compare December 12, 2023 13:43
@yyx990803 yyx990803 changed the base branch from main to minor December 12, 2023 13:43
Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 89.3 kB (+2.33 kB) 34 kB (+877 B) 30.6 kB (+774 B)
vue.global.prod.js 146 kB (+13.3 kB) 53.2 kB (+3.29 kB) 47.5 kB (+2.81 kB)

Usages

Name Size Gzip Brotli
createApp 49.6 kB (+1.33 kB) 19.4 kB (+439 B) 17.7 kB (+394 B)
createSSRApp 52.9 kB (+1.41 kB) 20.8 kB (+456 B) 18.9 kB (+434 B)
defineCustomElement 51.9 kB (+1.22 kB) 20.2 kB (+413 B) 18.4 kB (+354 B)
overall 63 kB (+1.36 kB) 24.3 kB (+487 B) 22.1 kB (+377 B)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
5 participants