From d343a0dc01663f91db42b4ddb693e6fffcb45873 Mon Sep 17 00:00:00 2001 From: Evan You Date: Sat, 13 Jul 2024 21:38:32 +0800 Subject: [PATCH] fix(v-once): properly unmount v-once cached trees close #5154 close #8809 --- .../__snapshots__/codegen.spec.ts.snap | 4 +- .../compiler-core/__tests__/codegen.spec.ts | 4 +- .../__snapshots__/vOnce.spec.ts.snap | 10 ++-- packages/compiler-core/src/codegen.ts | 3 +- .../__tests__/rendererOptimizedMode.spec.ts | 54 +++++++++++++++++++ packages/runtime-core/__tests__/vnode.spec.ts | 5 +- packages/runtime-core/src/renderer.ts | 6 +++ packages/runtime-core/src/vnode.ts | 11 ++-- 8 files changed, 83 insertions(+), 14 deletions(-) diff --git a/packages/compiler-core/__tests__/__snapshots__/codegen.spec.ts.snap b/packages/compiler-core/__tests__/__snapshots__/codegen.spec.ts.snap index a9591f922b4..db268af4f9b 100644 --- a/packages/compiler-core/__tests__/__snapshots__/codegen.spec.ts.snap +++ b/packages/compiler-core/__tests__/__snapshots__/codegen.spec.ts.snap @@ -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] ) diff --git a/packages/compiler-core/__tests__/codegen.spec.ts b/packages/compiler-core/__tests__/codegen.spec.ts index 4a5ba7d5c61..7724d507cb2 100644 --- a/packages/compiler-core/__tests__/codegen.spec.ts +++ b/packages/compiler-core/__tests__/codegen.spec.ts @@ -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, @@ -456,7 +456,7 @@ describe('compiler: codegen', () => { ` _cache[1] || ( _setBlockTracking(-1), - _cache[1] = foo, + (_cache[1] = foo).cacheIndex = 1, _setBlockTracking(1), _cache[1] ) diff --git a/packages/compiler-core/__tests__/transforms/__snapshots__/vOnce.spec.ts.snap b/packages/compiler-core/__tests__/transforms/__snapshots__/vOnce.spec.ts.snap index 1c1203552db..3d13c4066d9 100644 --- a/packages/compiler-core/__tests__/transforms/__snapshots__/vOnce.spec.ts.snap +++ b/packages/compiler-core/__tests__/transforms/__snapshots__/vOnce.spec.ts.snap @@ -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] ) @@ -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] ) @@ -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] ) @@ -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] ) @@ -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] ) diff --git a/packages/compiler-core/src/codegen.ts b/packages/compiler-core/src/codegen.ts index 392896d6082..b6535c5cef9 100644 --- a/packages/compiler-core/src/codegen.ts +++ b/packages/compiler-core/src/codegen.ts @@ -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() diff --git a/packages/runtime-core/__tests__/rendererOptimizedMode.spec.ts b/packages/runtime-core/__tests__/rendererOptimizedMode.spec.ts index 7abd94e1960..e0b49a5e7eb 100644 --- a/packages/runtime-core/__tests__/rendererOptimizedMode.spec.ts +++ b/packages/runtime-core/__tests__/rendererOptimizedMode.spec.ts @@ -25,6 +25,7 @@ import { renderList, renderSlot, serialize, + setBlockTracking, withCtx, } from '@vue/runtime-test' import { PatchFlags, SlotFlags } from '@vue/shared' @@ -1178,4 +1179,57 @@ describe('renderer: optimized mode', () => { await nextTick() expect(inner(root)).toBe('
bar
') }) + + 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( + '
Child
', + ) + + show.value = false + await nextTick() + expect(inner(root)).toBe('') + expect(spyUnmounted).toHaveBeenCalledTimes(1) + + show.value = true + await nextTick() + expect(inner(root)).toBe( + '
Child
', + ) + + // should unmount again, this verifies previous cache was properly cleared + show.value = false + await nextTick() + expect(inner(root)).toBe('') + expect(spyUnmounted).toHaveBeenCalledTimes(2) + }) }) diff --git a/packages/runtime-core/__tests__/vnode.spec.ts b/packages/runtime-core/__tests__/vnode.spec.ts index 39a7abd5f89..2e0eee1f280 100644 --- a/packages/runtime-core/__tests__/vnode.spec.ts +++ b/packages/runtime-core/__tests__/vnode.spec.ts @@ -2,6 +2,7 @@ import { Comment, Fragment, Text, + type VNode, cloneVNode, createBlock, createVNode, @@ -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', () => { diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index 3f496cd8dd6..8a64baad1c0 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -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)) diff --git a/packages/runtime-core/src/vnode.ts b/packages/runtime-core/src/vnode.ts index d05d7744fad..2210440e717 100644 --- a/packages/runtime-core/src/vnode.ts +++ b/packages/runtime-core/src/vnode.ts @@ -226,7 +226,7 @@ export interface VNode< /** * @internal */ - dynamicChildren: VNode[] | null + dynamicChildren: (VNode[] & { hasOnce?: boolean }) | null // application root node only appContext: AppContext | null @@ -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. @@ -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) {