Skip to content

Commit bb0e242

Browse files
committed
fix(templateRef): prevent duplicate onScopeDispose registrations for dynamic template refs
1 parent 3932cb6 commit bb0e242

File tree

2 files changed

+96
-5
lines changed

2 files changed

+96
-5
lines changed

packages/runtime-vapor/__tests__/dom/templateRef.spec.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,85 @@ describe('api: template ref', () => {
756756
}
757757
})
758758

759+
it('should not register duplicate onScopeDispose callbacks for dynamic function refs', async () => {
760+
const fn1 = vi.fn()
761+
const fn2 = vi.fn()
762+
const toggle = ref(true)
763+
const t0 = template('<div></div>')
764+
765+
const { app } = define({
766+
render() {
767+
const n0 = t0()
768+
let r0: any
769+
renderEffect(() => {
770+
r0 = createTemplateRefSetter()(
771+
n0 as Element,
772+
toggle.value ? fn1 : fn2,
773+
r0,
774+
)
775+
})
776+
return n0
777+
},
778+
}).render()
779+
780+
expect(fn1).toHaveBeenCalledTimes(1)
781+
expect(fn2).toHaveBeenCalledTimes(0)
782+
783+
toggle.value = false
784+
await nextTick()
785+
expect(fn1).toHaveBeenCalledTimes(1)
786+
expect(fn2).toHaveBeenCalledTimes(1)
787+
788+
toggle.value = true
789+
await nextTick()
790+
expect(fn1).toHaveBeenCalledTimes(2)
791+
expect(fn2).toHaveBeenCalledTimes(1)
792+
793+
app.unmount()
794+
await nextTick()
795+
// expected fn1 to be called again during scope dispose
796+
expect(fn1).toHaveBeenCalledTimes(3)
797+
expect(fn2).toHaveBeenCalledTimes(1)
798+
})
799+
800+
it('should not register duplicate onScopeDispose callbacks for dynamic string refs', async () => {
801+
const el1 = ref(null)
802+
const el2 = ref(null)
803+
const toggle = ref(true)
804+
const t0 = template('<div></div>')
805+
806+
const { app, host } = define({
807+
setup() {
808+
return { ref1: el1, ref2: el2 }
809+
},
810+
render() {
811+
const n0 = t0()
812+
let r0: any
813+
renderEffect(() => {
814+
r0 = createTemplateRefSetter()(
815+
n0 as Element,
816+
toggle.value ? 'ref1' : 'ref2',
817+
r0,
818+
)
819+
})
820+
return n0
821+
},
822+
}).render()
823+
824+
expect(el1.value).toBe(host.children[0])
825+
expect(el2.value).toBe(null)
826+
827+
toggle.value = false
828+
await nextTick()
829+
expect(el1.value).toBe(null)
830+
expect(el2.value).toBe(host.children[0])
831+
832+
app.unmount()
833+
await nextTick()
834+
expect(el1.value).toBe(null)
835+
expect(el2.value).toBe(null)
836+
})
837+
759838
// TODO: can not reproduce in Vapor
760839
// // #2078
761840
// test('handling multiple merged refs', async () => {

packages/runtime-vapor/src/apiTemplateRef.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,27 @@ import {
1717
import {
1818
EMPTY_OBJ,
1919
NO,
20+
NOOP,
2021
isArray,
2122
isFunction,
2223
isString,
2324
remove,
2425
} from '@vue/shared'
2526
import { DynamicFragment, isFragment } from './fragment'
2627

28+
// track cleanup functions to prevent duplicate onScopeDispose registrations
29+
const refCleanups = new WeakMap<RefEl, { fn: () => void }>()
30+
31+
// ensure only register onScopeDispose once per element
32+
function ensureCleanup(el: RefEl): { fn: () => void } {
33+
let cleanupRef = refCleanups.get(el)
34+
if (!cleanupRef) {
35+
refCleanups.set(el, (cleanupRef = { fn: NOOP }))
36+
onScopeDispose(() => cleanupRef!.fn())
37+
}
38+
return cleanupRef
39+
}
40+
2741
export type NodeRef =
2842
| string
2943
| Ref
@@ -102,8 +116,7 @@ export function setRef(
102116
}
103117

104118
invokeRefSetter(refValue)
105-
// TODO this gets called repeatedly in renderEffect when it's dynamic ref?
106-
onScopeDispose(() => invokeRefSetter())
119+
ensureCleanup(el).fn = () => invokeRefSetter()
107120
} else {
108121
const _isString = isString(ref)
109122
const _isRef = isRef(ref)
@@ -150,8 +163,7 @@ export function setRef(
150163
}
151164
queuePostFlushCb(doSet, -1)
152165

153-
// TODO this gets called repeatedly in renderEffect when it's dynamic ref?
154-
onScopeDispose(() => {
166+
ensureCleanup(el).fn = () => {
155167
queuePostFlushCb(() => {
156168
if (isArray(existing)) {
157169
remove(existing, refValue)
@@ -165,7 +177,7 @@ export function setRef(
165177
if (refKey) refs[refKey] = null
166178
}
167179
})
168-
})
180+
}
169181
} else if (__DEV__) {
170182
warn('Invalid template ref type:', ref, `(${typeof ref})`)
171183
}

0 commit comments

Comments
 (0)