fix(runtime-core): fix suspense crash when patch non-resolved async setup component #7290
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mmis1000
changed the title
mmis1000
changed the title
|
This doesn't seem to fix #6095 when patched into vuetify,
<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>
|
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
force-pushed
the
fix/suspense-hydration-patch-crash
branch
from
59cfe63
to
084c2a4
Compare
❌ Deploy Preview for vue-next-template-explorer failed.
|
❌ Deploy Preview for vue-sfc-playground failed.
|
|
@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
force-pushed
the
fix/suspense-hydration-patch-crash
branch
from
75569ef
to
390104d
Compare
|
@pikax I think I also find the root cause of client side suspense crash caused by update before async dep resolved. It seems the 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 Updated patchdiff --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)
|
|
I made a minimal reproduction of root cause of #6095 |
@mmis1000 <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 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 |
|
Yep, this PR seems to fix vuetifyjs/vuetify#15215 |
edison1105
reviewed
Nov 3, 2023
mmis1000
changed the title
…nt is not hydrated yet
The null el can also happen during client rendering
The initialVNode of async setup missed the el property, which in turns cause its parent component to crash during update
yyx990803
force-pushed
the
fix/suspense-hydration-patch-crash
branch
from
eac3602
to
fbcdc72
Compare
Size ReportBundles
Usages
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.
There are a few things that need to be addressed before this being a proper pull request.
#6463
And this also fix #6463, a crash caused by missing placeholder element in async element
initialVNode(thussubTree.elof parent component).This pull request fix the initial mounting of non-resolved async element. So the
elofinitialVNodeisn't null at initial mountingIn non-async elements, the
elofinitialVNodegets set up during the setupRenderEffect( call. But async element mounting skips that, result in ainitialVNodewithoutelfield set. This pull request address this by manually set theinitialVNode.elto 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.