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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions packages/runtime-core/__tests__/components/Suspense.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,51 @@ describe('Suspense', () => {
expect(unmounted).not.toHaveBeenCalled()
})

// vuetifyjs/vuetify#15207
test('update prop of async element before suspense resolve', async () => {
let resolve: () => void
const mounted = new Promise<void>(r => {
resolve = r
})
const Async = {
async setup() {
onMounted(() => {
resolve()
})
const p = new Promise(r => setTimeout(r, 1))
await p
return () => h('div', 'async')
}
}

const Comp: ComponentOptions<{ data: string }> = {
props: ['data'],
setup(props) {
return () => h(Async, { 'data-test': props.data })
}
}

const Root = {
setup() {
const data = ref('1')
onMounted(() => {
data.value = '2'
})
return () =>
h(Suspense, null, {
default: h(Comp, { data: data.value }),
fallback: h('div', 'fallback')
})
}
}

const root = nodeOps.createElement('div')
render(h(Root), root)
expect(serializeInner(root)).toBe(`<div>fallback</div>`)
await mounted
expect(serializeInner(root)).toBe(`<div data-test="2">async</div>`)
})

test('nested suspense (parent resolves first)', async () => {
const calls: string[] = []

Expand Down
147 changes: 144 additions & 3 deletions packages/runtime-core/__tests__/hydration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -812,17 +812,17 @@ describe('SSR hydration', () => {
})
)

const bol = ref(true)
const toggle = ref(true)
const App = {
setup() {
onMounted(() => {
// change state, this makes updateComponent(AsyncComp) execute before
// the async component is resolved
bol.value = false
toggle.value = false
})

return () => {
return [bol.value ? 'hello' : 'world', h(AsyncComp)]
return [toggle.value ? 'hello' : 'world', h(AsyncComp)]
}
}
}
Expand Down Expand Up @@ -859,6 +859,147 @@ describe('SSR hydration', () => {
)
})

test('hydrate safely when property used by async setup changed before render', async () => {
const toggle = ref(true)

const AsyncComp = {
async setup() {
await new Promise<void>(r => setTimeout(r, 10))
return () => h('h1', 'Async component')
}
}

const AsyncWrapper = {
render() {
return h(AsyncComp)
}
}

const SiblingComp = {
setup() {
toggle.value = false
return () => h('span')
}
}

const App = {
setup() {
return () =>
h(
Suspense,
{},
{
default: () => [
h('main', {}, [
h(AsyncWrapper, {
prop: toggle.value ? 'hello' : 'world'
}),
h(SiblingComp)
])
]
}
)
}
}

// server render
const html = await renderToString(h(App))

expect(html).toMatchInlineSnapshot(
`"<main><h1 prop="hello">Async component</h1><span></span></main>"`
)

expect(toggle.value).toBe(false)

// hydration

// reset the value
toggle.value = true
expect(toggle.value).toBe(true)

const container = document.createElement('div')
container.innerHTML = html
createSSRApp(App).mount(container)

await new Promise(r => setTimeout(r, 10))

expect(toggle.value).toBe(false)

// should be hydrated now
expect(container.innerHTML).toMatchInlineSnapshot(
`"<main><h1 prop="world">Async component</h1><span></span></main>"`
)
})

test('hydrate safely when property used by deep nested async setup changed before render', async () => {
const toggle = ref(true)

const AsyncComp = {
async setup() {
await new Promise<void>(r => setTimeout(r, 10))
return () => h('h1', 'Async component')
}
}

const AsyncWrapper = { render: () => h(AsyncComp) }
const AsyncWrapperWrapper = { render: () => h(AsyncWrapper) }

const SiblingComp = {
setup() {
toggle.value = false
return () => h('span')
}
}

const App = {
setup() {
return () =>
h(
Suspense,
{},
{
default: () => [
h('main', {}, [
h(AsyncWrapperWrapper, {
prop: toggle.value ? 'hello' : 'world'
}),
h(SiblingComp)
])
]
}
)
}
}

// server render
const html = await renderToString(h(App))

expect(html).toMatchInlineSnapshot(
`"<main><h1 prop="hello">Async component</h1><span></span></main>"`
)

expect(toggle.value).toBe(false)

// hydration

// reset the value
toggle.value = true
expect(toggle.value).toBe(true)

const container = document.createElement('div')
container.innerHTML = html
createSSRApp(App).mount(container)

await new Promise(r => setTimeout(r, 10))

expect(toggle.value).toBe(false)

// should be hydrated now
expect(container.innerHTML).toMatchInlineSnapshot(
`"<main><h1 prop="world">Async component</h1><span></span></main>"`
)
})

// #3787
test('unmount async wrapper before load', async () => {
let resolve: any
Expand Down
32 changes: 20 additions & 12 deletions packages/runtime-core/src/components/Suspense.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,18 +226,26 @@ function patchSuspense(
if (suspense.deps <= 0) {
suspense.resolve()
} else if (isInFallback) {
patch(
activeBranch,
newFallback,
container,
anchor,
parentComponent,
null, // fallback tree will not have suspense context
namespace,
slotScopeIds,
optimized
)
setActiveBranch(suspense, newFallback)
// It's possible that the app is in hydrating state when patching the
// suspense instance. If someone updates the dependency during component
// setup in children of suspense boundary, 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
namespace,
slotScopeIds,
optimized
)
setActiveBranch(suspense, newFallback)
}
}
} else {
// toggled before pending tree is resolved
Expand Down
43 changes: 42 additions & 1 deletion packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,10 @@ function baseCreateRenderer(
if (!initialVNode.el) {
const placeholder = (instance.subTree = createVNode(Comment))
processCommentNode(null, placeholder, container!, anchor)
// This noramlly gets setup by the following `setupRenderEffect`.
// But the call is skipped in initial mounting of async element.
// Thus, manually patching is required here or it will result in a crash during parent component update.
initialVNode.el = placeholder.el
mmis1000 marked this conversation as resolved.
Show resolved Hide resolved
}
return
}
Expand Down Expand Up @@ -1447,10 +1451,34 @@ function baseCreateRenderer(
// #2458: deference mount-only object parameters to prevent memleaks
initialVNode = container = anchor = null as any
} else {
let { next, bu, u, parent, vnode } = instance

if (__FEATURE_SUSPENSE__) {
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) {
// only sync the properties and abort the rest of operations
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
nonHydratedAsyncRoot.asyncDep!.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)
let { next, bu, u, parent, vnode } = instance
let originNext = next
let vnodeHook: VNodeHook | null | undefined
if (__DEV__) {
Expand Down Expand Up @@ -2489,3 +2517,16 @@ function getSequence(arr: number[]): number[] {
}
return result
}

function locateNonHydratedAsyncRoot(
instance: ComponentInternalInstance
): ComponentInternalInstance | undefined {
const subComponent = instance.subTree.component
if (subComponent) {
if (subComponent.asyncDep && !subComponent.asyncResolved) {
return subComponent
} else {
return locateNonHydratedAsyncRoot(subComponent)
}
}
}