Skip to content

Commit

Permalink
fix(runtime-core): vnode hooks should not be called on async wrapper (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
HcySunYang authored Aug 16, 2021
1 parent 3201224 commit cd2d984
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 5 deletions.
55 changes: 55 additions & 0 deletions packages/runtime-core/__tests__/apiAsyncComponent.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -744,4 +744,59 @@ describe('api: defineAsyncComponent', () => {
expect(serializeInner(root)).toBe('<!---->')
expect(fooRef.value).toBe(null)
})

test('vnode hooks on async wrapper', async () => {
let resolve: (comp: Component) => void
const Foo = defineAsyncComponent(
() =>
new Promise(r => {
resolve = r as any
})
)
const updater = ref(0)

const vnodeHooks = {
onVnodeBeforeMount: jest.fn(),
onVnodeMounted: jest.fn(),
onVnodeBeforeUpdate: jest.fn(),
onVnodeUpdated: jest.fn(),
onVnodeBeforeUnmount: jest.fn(),
onVnodeUnmounted: jest.fn()
}

const toggle = ref(true)

const root = nodeOps.createElement('div')
createApp({
render: () => (toggle.value ? [h(Foo, vnodeHooks), updater.value] : null)
}).mount(root)

expect(serializeInner(root)).toBe('<!---->0')

resolve!({
data() {
return {
id: 'foo'
}
},
render: () => 'resolved'
})

await timeout()
expect(serializeInner(root)).toBe('resolved0')
expect(vnodeHooks.onVnodeBeforeMount).toHaveBeenCalledTimes(1)
expect(vnodeHooks.onVnodeMounted).toHaveBeenCalledTimes(1)

updater.value++
await nextTick()
expect(serializeInner(root)).toBe('resolved1')
expect(vnodeHooks.onVnodeBeforeUpdate).toHaveBeenCalledTimes(1)
expect(vnodeHooks.onVnodeUpdated).toHaveBeenCalledTimes(1)

toggle.value = false
await nextTick()
expect(serializeInner(root)).toBe('<!---->')
expect(vnodeHooks.onVnodeBeforeUnmount).toHaveBeenCalledTimes(1)
expect(vnodeHooks.onVnodeUnmounted).toHaveBeenCalledTimes(1)
})
})
25 changes: 20 additions & 5 deletions packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1316,14 +1316,18 @@ function baseCreateRenderer(
let vnodeHook: VNodeHook | null | undefined
const { el, props } = initialVNode
const { bm, m, parent } = instance
const isAsyncWrapperVNode = isAsyncWrapper(initialVNode)

effect.allowRecurse = false
// beforeMount hook
if (bm) {
invokeArrayFns(bm)
}
// onVnodeBeforeMount
if ((vnodeHook = props && props.onVnodeBeforeMount)) {
if (
!isAsyncWrapperVNode &&
(vnodeHook = props && props.onVnodeBeforeMount)
) {
invokeVNodeHook(vnodeHook, parent, initialVNode)
}
if (
Expand Down Expand Up @@ -1359,7 +1363,7 @@ function baseCreateRenderer(
}
}

if (isAsyncWrapper(initialVNode)) {
if (isAsyncWrapperVNode) {
;(initialVNode.type as ComponentOptions).__asyncLoader!().then(
// note: we are moving the render call into an async callback,
// which means it won't track dependencies - but it's ok because
Expand Down Expand Up @@ -1400,7 +1404,10 @@ function baseCreateRenderer(
queuePostRenderEffect(m, parentSuspense)
}
// onVnodeMounted
if ((vnodeHook = props && props.onVnodeMounted)) {
if (
!isAsyncWrapperVNode &&
(vnodeHook = props && props.onVnodeMounted)
) {
const scopedInitialVNode = initialVNode
queuePostRenderEffect(
() => invokeVNodeHook(vnodeHook!, parent, scopedInitialVNode),
Expand Down Expand Up @@ -2085,9 +2092,13 @@ function baseCreateRenderer(
}

const shouldInvokeDirs = shapeFlag & ShapeFlags.ELEMENT && dirs
const shouldInvokeVnodeHook = !isAsyncWrapper(vnode)

let vnodeHook: VNodeHook | undefined | null
if ((vnodeHook = props && props.onVnodeBeforeUnmount)) {
if (
shouldInvokeVnodeHook &&
(vnodeHook = props && props.onVnodeBeforeUnmount)
) {
invokeVNodeHook(vnodeHook, parentComponent, vnode)
}

Expand Down Expand Up @@ -2140,7 +2151,11 @@ function baseCreateRenderer(
}
}

if ((vnodeHook = props && props.onVnodeUnmounted) || shouldInvokeDirs) {
if (
(shouldInvokeVnodeHook &&
(vnodeHook = props && props.onVnodeUnmounted)) ||
shouldInvokeDirs
) {
queuePostRenderEffect(() => {
vnodeHook && invokeVNodeHook(vnodeHook, parentComponent, vnode)
shouldInvokeDirs &&
Expand Down

0 comments on commit cd2d984

Please sign in to comment.