Skip to content

Commit

Permalink
fix(v-once): properly unmount v-once cached trees
Browse files Browse the repository at this point in the history
close #5154
close #8809
  • Loading branch information
yyx990803 committed Jul 13, 2024
1 parent 3107b57 commit d343a0d
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ export function render(_ctx, _cache) {
}"
`;

exports[`compiler: codegen > CacheExpression w/ isVNode: true 1`] = `
exports[`compiler: codegen > CacheExpression w/ isVOnce: true 1`] = `
"
export function render(_ctx, _cache) {
return _cache[1] || (
_setBlockTracking(-1),
_cache[1] = foo,
(_cache[1] = foo).cacheIndex = 1,
_setBlockTracking(1),
_cache[1]
)
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-core/__tests__/codegen.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ describe('compiler: codegen', () => {
expect(code).toMatchSnapshot()
})

test('CacheExpression w/ isVNode: true', () => {
test('CacheExpression w/ isVOnce: true', () => {
const { code } = generate(
createRoot({
cached: 1,
Expand All @@ -456,7 +456,7 @@ describe('compiler: codegen', () => {
`
_cache[1] || (
_setBlockTracking(-1),
_cache[1] = foo,
(_cache[1] = foo).cacheIndex = 1,
_setBlockTracking(1),
_cache[1]
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ return function render(_ctx, _cache) {
return _cache[0] || (
_setBlockTracking(-1),
_cache[0] = _createElementVNode("div", { id: foo }, null, 8 /* PROPS */, ["id"]),
(_cache[0] = _createElementVNode("div", { id: foo }, null, 8 /* PROPS */, ["id"])).cacheIndex = 0,
_setBlockTracking(1),
_cache[0]
)
Expand All @@ -29,7 +29,7 @@ return function render(_ctx, _cache) {
return (_openBlock(), _createElementBlock("div", null, [
_cache[0] || (
_setBlockTracking(-1),
_cache[0] = _createVNode(_component_Comp, { id: foo }, null, 8 /* PROPS */, ["id"]),
(_cache[0] = _createVNode(_component_Comp, { id: foo }, null, 8 /* PROPS */, ["id"])).cacheIndex = 0,
_setBlockTracking(1),
_cache[0]
)
Expand All @@ -48,7 +48,7 @@ return function render(_ctx, _cache) {
return (_openBlock(), _createElementBlock("div", null, [
_cache[0] || (
_setBlockTracking(-1),
_cache[0] = _createElementVNode("div", { id: foo }, null, 8 /* PROPS */, ["id"]),
(_cache[0] = _createElementVNode("div", { id: foo }, null, 8 /* PROPS */, ["id"])).cacheIndex = 0,
_setBlockTracking(1),
_cache[0]
)
Expand All @@ -67,7 +67,7 @@ return function render(_ctx, _cache) {
return (_openBlock(), _createElementBlock("div", null, [
_cache[0] || (
_setBlockTracking(-1),
_cache[0] = _renderSlot($slots, "default"),
(_cache[0] = _renderSlot($slots, "default")).cacheIndex = 0,
_setBlockTracking(1),
_cache[0]
)
Expand All @@ -86,7 +86,7 @@ return function render(_ctx, _cache) {
return (_openBlock(), _createElementBlock("div", null, [
_cache[0] || (
_setBlockTracking(-1),
_cache[0] = _createElementVNode("div"),
(_cache[0] = _createElementVNode("div")).cacheIndex = 0,
_setBlockTracking(1),
_cache[0]
)
Expand Down
3 changes: 2 additions & 1 deletion packages/compiler-core/src/codegen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1041,11 +1041,12 @@ function genCacheExpression(node: CacheExpression, context: CodegenContext) {
indent()
push(`${helper(SET_BLOCK_TRACKING)}(-1),`)
newline()
push(`(`)
}
push(`_cache[${node.index}] = `)
genNode(node.value, context)
if (node.isVOnce) {
push(`,`)
push(`).cacheIndex = ${node.index},`)
newline()
push(`${helper(SET_BLOCK_TRACKING)}(1),`)
newline()
Expand Down
54 changes: 54 additions & 0 deletions packages/runtime-core/__tests__/rendererOptimizedMode.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
renderList,
renderSlot,
serialize,
setBlockTracking,
withCtx,
} from '@vue/runtime-test'
import { PatchFlags, SlotFlags } from '@vue/shared'
Expand Down Expand Up @@ -1178,4 +1179,57 @@ describe('renderer: optimized mode', () => {
await nextTick()
expect(inner(root)).toBe('<div><!--comment--><div>bar</div></div>')
})

test('should not take unmount children fast path if children contain cached nodes', async () => {
const show = ref(true)
const spyUnmounted = vi.fn()

const Child = {
setup() {
onUnmounted(spyUnmounted)
return () => createVNode('div', null, 'Child')
},
}

const app = createApp({
render(_: any, cache: any) {
return show.value
? (openBlock(),
createBlock('div', null, [
createVNode('div', null, [
cache[0] ||
(setBlockTracking(-1),
((cache[0] = createVNode('div', null, [
createVNode(Child),
])).cacheIndex = 0),
setBlockTracking(1),
cache[0]),
]),
]))
: createCommentVNode('v-if', true)
},
})

app.mount(root)
expect(inner(root)).toBe(
'<div><div><div><div>Child</div></div></div></div>',
)

show.value = false
await nextTick()
expect(inner(root)).toBe('<!--v-if-->')
expect(spyUnmounted).toHaveBeenCalledTimes(1)

show.value = true
await nextTick()
expect(inner(root)).toBe(
'<div><div><div><div>Child</div></div></div></div>',
)

// should unmount again, this verifies previous cache was properly cleared
show.value = false
await nextTick()
expect(inner(root)).toBe('<!--v-if-->')
expect(spyUnmounted).toHaveBeenCalledTimes(2)
})
})
5 changes: 4 additions & 1 deletion packages/runtime-core/__tests__/vnode.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
Comment,
Fragment,
Text,
type VNode,
cloneVNode,
createBlock,
createVNode,
Expand Down Expand Up @@ -633,7 +634,9 @@ describe('vnode', () => {
setBlockTracking(1),
vnode1,
]))
expect(vnode.dynamicChildren).toStrictEqual([])
const expected: VNode['dynamicChildren'] = []
expected.hasOnce = true
expect(vnode.dynamicChildren).toStrictEqual(expected)
})
// #5657
test('error of slot function execution should not affect block tracking', () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2164,6 +2164,12 @@ function baseCreateRenderer(
)
} else if (
dynamicChildren &&
// #5154
// when v-once is used inside a block, setBlockTracking(-1) marks the
// parent block with hasOnce: true
// so that it doesn't take the fast path during unmount - otherwise
// components nested in v-once are never unmounted.
!dynamicChildren.hasOnce &&
// #1153: fast path should not be taken for non-stable (v-for) fragments
(type !== Fragment ||
(patchFlag > 0 && patchFlag & PatchFlags.STABLE_FRAGMENT))
Expand Down
11 changes: 8 additions & 3 deletions packages/runtime-core/src/vnode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ export interface VNode<
/**
* @internal
*/
dynamicChildren: VNode[] | null
dynamicChildren: (VNode[] & { hasOnce?: boolean }) | null

// application root node only
appContext: AppContext | null
Expand Down Expand Up @@ -259,8 +259,8 @@ export interface VNode<
// can divide a template into nested blocks, and within each block the node
// structure would be stable. This allows us to skip most children diffing
// and only worry about the dynamic nodes (indicated by patch flags).
export const blockStack: (VNode[] | null)[] = []
export let currentBlock: VNode[] | null = null
export const blockStack: VNode['dynamicChildren'][] = []
export let currentBlock: VNode['dynamicChildren'] = null

/**
* Open a block.
Expand Down Expand Up @@ -311,6 +311,11 @@ export let isBlockTreeEnabled = 1
*/
export function setBlockTracking(value: number) {
isBlockTreeEnabled += value
if (value < 0 && currentBlock) {
// mark current block so it doesn't take fast path and skip possible
// nested components duriung unmount
currentBlock.hasOnce = true
}
}

function setupBlock(vnode: VNode) {
Expand Down

0 comments on commit d343a0d

Please sign in to comment.