Skip to content

fix(runtime-core): avoid setting direct ref of useTemplateRef in dev #13449

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
128 changes: 128 additions & 0 deletions packages/runtime-core/__tests__/helpers/useTemplateRef.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,134 @@ describe('useTemplateRef', () => {
expect(tRef!.value).toBe(null)
})

test('should work when used with direct ref value with ref_key', () => {
let tRef: ShallowRef
const key = 'refKey'
const Comp = {
setup() {
tRef = useTemplateRef(key)
return () => h('div', { ref: tRef, ref_key: key })
},
}
const root = nodeOps.createElement('div')
render(h(Comp), root)

expect('target is readonly').not.toHaveBeenWarned()
expect(tRef!.value).toBe(root.children[0])
})

test('should work when used with direct ref value with ref_key and ref_for', () => {
let tRef: ShallowRef
const key = 'refKey'
const Comp = {
setup() {
tRef = useTemplateRef(key)
},
render() {
return h(
'div',
[1, 2, 3].map(x =>
h('span', { ref: tRef, ref_key: key, ref_for: true }, x.toString()),
),
)
},
}
const root = nodeOps.createElement('div')
render(h(Comp), root)

expect('target is readonly').not.toHaveBeenWarned()
expect(tRef!.value).toHaveLength(3)
})

test('should work when used with direct ref value with ref_key and dynamic value', async () => {
const refMode = ref('h1-ref')

let tRef: ShallowRef
const key = 'refKey'

const Comp = {
setup() {
tRef = useTemplateRef(key)
},
render() {
switch (refMode.value) {
case 'h1-ref':
return h('h1', { ref: tRef, ref_key: key })
case 'h2-ref':
return h('h2', { ref: tRef, ref_key: key })
case 'no-ref':
return h('span')
case 'nothing':
return null
}
},
}

const root = nodeOps.createElement('div')
render(h(Comp), root)

expect(tRef!.value.tag).toBe('h1')

refMode.value = 'h2-ref'
await nextTick()
expect(tRef!.value.tag).toBe('h2')

refMode.value = 'no-ref'
await nextTick()
expect(tRef!.value).toBeNull()

refMode.value = 'nothing'
await nextTick()
expect(tRef!.value).toBeNull()

expect('target is readonly').not.toHaveBeenWarned()
})

test('should work when used with dynamic direct refs and ref_keys', async () => {
const refKey = ref('foo')

let tRefs: Record<string, ShallowRef>

const Comp = {
setup() {
tRefs = {
foo: useTemplateRef('foo'),
bar: useTemplateRef('bar'),
}
},
render() {
return h('div', { ref: tRefs[refKey.value], ref_key: refKey.value })
},
}

const root = nodeOps.createElement('div')
render(h(Comp), root)

expect(tRefs!['foo'].value).toBe(root.children[0])
expect(tRefs!['bar'].value).toBeNull()

refKey.value = 'bar'
await nextTick()
expect(tRefs!['foo'].value).toBeNull()
expect(tRefs!['bar'].value).toBe(root.children[0])

expect('target is readonly').not.toHaveBeenWarned()
})

test('should not work when used with direct ref value without ref_key (in dev mode)', () => {
let tRef: ShallowRef
const Comp = {
setup() {
tRef = useTemplateRef('refKey')
return () => h('div', { ref: tRef })
},
}
const root = nodeOps.createElement('div')
render(h(Comp), root)

expect(tRef!.value).toBeNull()
})

test('should work when used as direct ref value (compiled in prod mode)', () => {
__DEV__ = false
try {
Expand Down
34 changes: 28 additions & 6 deletions packages/runtime-core/src/rendererTemplateRef.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import type { SuspenseBoundary } from './components/Suspense'
import type { VNode, VNodeNormalizedRef, VNodeNormalizedRefAtom } from './vnode'
import type {
VNode,
VNodeNormalizedRef,
VNodeNormalizedRefAtom,
VNodeRef,
} from './vnode'
import {
EMPTY_OBJ,
ShapeFlags,
Expand Down Expand Up @@ -94,6 +99,10 @@ export function setRef(
return hasOwn(rawSetupState, key)
}

const canSetRef = (ref: VNodeRef) => {
return !__DEV__ || !knownTemplateRefs.has(ref as any)
}

// dynamic ref changed. unset old ref
if (oldRef != null && oldRef !== ref) {
if (isString(oldRef)) {
Expand All @@ -102,7 +111,13 @@ export function setRef(
setupState[oldRef] = null
}
} else if (isRef(oldRef)) {
oldRef.value = null
if (canSetRef(oldRef)) {
oldRef.value = null
}

// this type assertion is valid since `oldRef` has already been asserted to be non-null
const oldRawRefAtom = oldRawRef as VNodeNormalizedRefAtom
if (oldRawRefAtom.k) refs[oldRawRefAtom.k] = null
}
}

Expand All @@ -119,7 +134,9 @@ export function setRef(
? canSetSetupRef(ref)
? setupState[ref]
: refs[ref]
: ref.value
: canSetRef(ref) || !rawRef.k
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks good. canSetRef will be treeshaking in the PROD build.
The test cases don't seem to cover this change.

? ref.value
: refs[rawRef.k]
if (isUnmount) {
isArray(existing) && remove(existing, refValue)
} else {
Expand All @@ -130,8 +147,11 @@ export function setRef(
setupState[ref] = refs[ref]
}
} else {
ref.value = [refValue]
if (rawRef.k) refs[rawRef.k] = ref.value
const newVal = [refValue]
if (canSetRef(ref)) {
ref.value = newVal
}
if (rawRef.k) refs[rawRef.k] = newVal
}
} else if (!existing.includes(refValue)) {
existing.push(refValue)
Expand All @@ -143,7 +163,9 @@ export function setRef(
setupState[ref] = value
}
} else if (_isRef) {
ref.value = value
if (canSetRef(ref)) {
ref.value = value
}
if (rawRef.k) refs[rawRef.k] = value
} else if (__DEV__) {
warn('Invalid template ref type:', ref, `(${typeof ref})`)
Expand Down
1 change: 1 addition & 0 deletions packages/vue/__tests__/e2e/Transition.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3082,6 +3082,7 @@ describe('e2e: Transition', () => {

// enter
await classWhenTransitionStart()
await nextFrame()
await transitionFinish()

// leave
Expand Down