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(v-once): setting hasOnce to current block only when in v-once #12374

Merged
merged 3 commits into from
Nov 14, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ return function render(_ctx, _cache) {
const { setBlockTracking: _setBlockTracking, createElementVNode: _createElementVNode } = _Vue

return _cache[0] || (
_setBlockTracking(-1),
_setBlockTracking(-1, true),
(_cache[0] = _createElementVNode("div", { id: foo }, null, 8 /* PROPS */, ["id"])).cacheIndex = 0,
_setBlockTracking(1),
_cache[0]
Expand All @@ -28,7 +28,7 @@ return function render(_ctx, _cache) {

return (_openBlock(), _createElementBlock("div", null, [
_cache[0] || (
_setBlockTracking(-1),
_setBlockTracking(-1, true),
(_cache[0] = _createVNode(_component_Comp, { id: foo }, null, 8 /* PROPS */, ["id"])).cacheIndex = 0,
_setBlockTracking(1),
_cache[0]
Expand All @@ -47,7 +47,7 @@ return function render(_ctx, _cache) {

return (_openBlock(), _createElementBlock("div", null, [
_cache[0] || (
_setBlockTracking(-1),
_setBlockTracking(-1, true),
(_cache[0] = _createElementVNode("div", { id: foo }, null, 8 /* PROPS */, ["id"])).cacheIndex = 0,
_setBlockTracking(1),
_cache[0]
Expand All @@ -66,7 +66,7 @@ return function render(_ctx, _cache) {

return (_openBlock(), _createElementBlock("div", null, [
_cache[0] || (
_setBlockTracking(-1),
_setBlockTracking(-1, true),
(_cache[0] = _renderSlot($slots, "default")).cacheIndex = 0,
_setBlockTracking(1),
_cache[0]
Expand All @@ -85,7 +85,7 @@ return function render(_ctx, _cache) {

return (_openBlock(), _createElementBlock("div", null, [
_cache[0] || (
_setBlockTracking(-1),
_setBlockTracking(-1, true),
(_cache[0] = _createElementVNode("div")).cacheIndex = 0,
_setBlockTracking(1),
_cache[0]
Expand Down
3 changes: 3 additions & 0 deletions packages/compiler-core/src/ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ export interface CacheExpression extends Node {
index: number
value: JSChildNode
needPauseTracking: boolean
inVOnce: boolean
needArraySpread: boolean
}

Expand Down Expand Up @@ -774,12 +775,14 @@ export function createCacheExpression(
index: number,
value: JSChildNode,
needPauseTracking: boolean = false,
inVOnce: boolean = false,
): CacheExpression {
return {
type: NodeTypes.JS_CACHE_EXPRESSION,
index,
value,
needPauseTracking: needPauseTracking,
inVOnce,
needArraySpread: false,
loc: locStub,
}
Expand Down
4 changes: 3 additions & 1 deletion packages/compiler-core/src/codegen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,9 @@ function genCacheExpression(node: CacheExpression, context: CodegenContext) {
push(`_cache[${node.index}] || (`)
if (needPauseTracking) {
indent()
push(`${helper(SET_BLOCK_TRACKING)}(-1),`)
push(`${helper(SET_BLOCK_TRACKING)}(-1`)
if (node.inVOnce) push(`, true`)
push(`),`)
newline()
push(`(`)
}
Expand Down
5 changes: 3 additions & 2 deletions packages/compiler-core/src/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export interface TransformContext
addIdentifiers(exp: ExpressionNode | string): void
removeIdentifiers(exp: ExpressionNode | string): void
hoist(exp: string | JSChildNode | ArrayExpression): SimpleExpressionNode
cache(exp: JSChildNode, isVNode?: boolean): CacheExpression
cache(exp: JSChildNode, isVNode?: boolean, inVOnce?: boolean): CacheExpression
constantCache: WeakMap<TemplateChildNode, ConstantTypes>

// 2.x Compat only
Expand Down Expand Up @@ -297,11 +297,12 @@ export function createTransformContext(
identifier.hoisted = exp
return identifier
},
cache(exp, isVNode = false) {
cache(exp, isVNode = false, inVOnce = false) {
const cacheExp = createCacheExpression(
context.cached.length,
exp,
isVNode,
inVOnce,
)
context.cached.push(cacheExp)
return cacheExp
Expand Down
6 changes: 5 additions & 1 deletion packages/compiler-core/src/transforms/vOnce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ export const transformOnce: NodeTransform = (node, context) => {
context.inVOnce = false
const cur = context.currentNode as ElementNode | IfNode | ForNode
if (cur.codegenNode) {
cur.codegenNode = context.cache(cur.codegenNode, true /* isVNode */)
cur.codegenNode = context.cache(
cur.codegenNode,
true /* isVNode */,
true /* inVOnce */,
)
}
}
}
Expand Down
63 changes: 62 additions & 1 deletion packages/runtime-core/__tests__/rendererOptimizedMode.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
serializeInner as inner,
nextTick,
nodeOps,
onBeforeMount,
onBeforeUnmount,
onUnmounted,
openBlock,
Expand Down Expand Up @@ -1199,7 +1200,7 @@ describe('renderer: optimized mode', () => {
createBlock('div', null, [
createVNode('div', null, [
cache[0] ||
(setBlockTracking(-1),
(setBlockTracking(-1, true),
((cache[0] = createVNode('div', null, [
createVNode(Child),
])).cacheIndex = 0),
Expand Down Expand Up @@ -1233,4 +1234,64 @@ describe('renderer: optimized mode', () => {
expect(inner(root)).toBe('<!--v-if-->')
expect(spyUnmounted).toHaveBeenCalledTimes(2)
})

// #12371
test('unmount children when the user calls a compiled slot', async () => {
const beforeMountSpy = vi.fn()
const beforeUnmountSpy = vi.fn()

const Child = {
setup() {
onBeforeMount(beforeMountSpy)
onBeforeUnmount(beforeUnmountSpy)
return () => 'child'
},
}

const Wrapper = {
setup(_: any, { slots }: SetupContext) {
return () => (
openBlock(),
createElementBlock('section', null, [
(openBlock(),
createElementBlock('div', { key: 1 }, [
createTextVNode(slots.header!() ? 'foo' : 'bar', 1 /* TEXT */),
renderSlot(slots, 'content'),
])),
])
)
},
}

const show = ref(false)
const app = createApp({
render() {
return show.value
? (openBlock(),
createBlock(Wrapper, null, {
header: withCtx(() => [createVNode({})]),
content: withCtx(() => [createVNode(Child)]),
_: 1,
}))
: createCommentVNode('v-if', true)
},
})

app.mount(root)
expect(inner(root)).toMatchInlineSnapshot(`"<!--v-if-->"`)
expect(beforeMountSpy).toHaveBeenCalledTimes(0)
expect(beforeUnmountSpy).toHaveBeenCalledTimes(0)

show.value = true
await nextTick()
expect(inner(root)).toMatchInlineSnapshot(
`"<section><div>foochild</div></section>"`,
)
expect(beforeMountSpy).toHaveBeenCalledTimes(1)

show.value = false
await nextTick()
expect(inner(root)).toBe('<!--v-if-->')
expect(beforeUnmountSpy).toHaveBeenCalledTimes(1)
})
})
2 changes: 1 addition & 1 deletion packages/runtime-core/__tests__/vnode.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ describe('vnode', () => {
const vnode =
(openBlock(),
createBlock('div', null, [
setBlockTracking(-1),
setBlockTracking(-1, true),
(vnode1 = (openBlock(), createBlock('div'))),
setBlockTracking(1),
vnode1,
Expand Down
8 changes: 4 additions & 4 deletions packages/runtime-core/src/vnode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ export let isBlockTreeEnabled = 1
*
* ``` js
* _cache[1] || (
* setBlockTracking(-1),
* setBlockTracking(-1, true),
* _cache[1] = createVNode(...),
* setBlockTracking(1),
* _cache[1]
Expand All @@ -310,11 +310,11 @@ export let isBlockTreeEnabled = 1
*
* @private
*/
export function setBlockTracking(value: number): void {
export function setBlockTracking(value: number, inVOnce = false): void {
isBlockTreeEnabled += value
if (value < 0 && currentBlock) {
if (value < 0 && currentBlock && inVOnce) {
// mark current block so it doesn't take fast path and skip possible
// nested components duriung unmount
// nested components during unmount
currentBlock.hasOnce = true
}
}
Expand Down