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(runtime-core): setRef should not update setupState's TemplateRef #11810

Closed
wants to merge 15 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,14 @@ describe('useTemplateRef', () => {
const Comp = {
setup() {
tRef = useTemplateRef(key)
const foo = useTemplateRef('bar')
return {
[key]: tRef,
foo,
}
},
render() {
return h('div', { ref: key })
return [h('div', { ref: key }), h('div', { ref: 'foo' })]
},
}
const root = nodeOps.createElement('div')
Expand Down
4 changes: 4 additions & 0 deletions packages/runtime-core/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,10 @@ export function handleSetupResult(
}
instance.setupState = proxyRefs(setupResult)
if (__DEV__) {
// dev only
Object.defineProperty(setupResult, '__v__setupResult', {
value: setupResult,
})
exposeSetupStateOnRenderContext(instance)
}
} else if (__DEV__ && setupResult !== undefined) {
Expand Down
2 changes: 2 additions & 0 deletions packages/runtime-core/src/helpers/useTemplateRef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ export function useTemplateRef<T = unknown, Keys extends string = string>(
): Readonly<ShallowRef<T | null>> {
const i = getCurrentInstance()
const r = shallowRef(null)
// @ts-expect-error
r.__v__TemplateRef = true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
r.__v__TemplateRef = true
if(__DEV__) r.__v__TemplateRef = true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to modify it like this before, but it caused the problem mentioned in #11816

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or I could modify it like this, but I'm not sure which way is more reasonable.
image

I also think this is a very extreme use case, maybe it's just wrong, but I still think we should handle this extreme situation.

if (i) {
const refs = i.refs === EMPTY_OBJ ? (i.refs = {}) : i.refs

Expand Down
23 changes: 19 additions & 4 deletions packages/runtime-core/src/rendererTemplateRef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,22 @@ export function setRef(
const canSetSetupRef =
setupState === EMPTY_OBJ
? () => false
: (key: string) =>
hasOwn(setupState, key) &&
!(Object.getOwnPropertyDescriptor(refs, key) || EMPTY_OBJ).get
: (key: string) => {
if (!hasOwn(setupState, key)) return false
if (__DEV__) {
if (
setupState.__v__setupResult &&
// @ts-expect-error
setupState.__v__setupResult[ref] &&
// @ts-expect-error
setupState.__v__setupResult[ref].__v__TemplateRef
) {
return false
}
return true
}
return true
}

// dynamic ref changed. unset old ref
if (oldRef != null && oldRef !== ref) {
Expand Down Expand Up @@ -118,7 +131,9 @@ export function setRef(
if (canSetSetupRef(ref)) {
setupState[ref] = value
}
} else if (_isRef) {
}
// @ts-expect-error
else if (_isRef && !ref.__v__TemplateRef) {
ref.value = value
if (rawRef.k) refs[rawRef.k] = value
} else if (__DEV__) {
Expand Down