deep watch traverse all dom if reactive element has ref to dom element
Version
3.0.11
Reproduction link
https://jsfiddle.net/iagafonov/dyngkbq6/41/
Steps to reproduce
open console and see error at application startup
What is expected?
expected that ui elements it not traversed as normal objects
What is actually happening?
ui elements inside reactive objects traversed as normal objects, that caused all dom elements in page beeing traversed
Actual error is happening inside traverse function of apiWatch: https://github.com/vuejs/vue-next/blob/870f2a7ba35245fd8c008d2ff666ea130a7e4704/packages/runtime-core/src/apiWatch.ts#L372
I wonder is "is dom node ?" is a good criteria to determine an object can be traversed or not. As DOM Node is nothing special than other normal objects and the PR above is made in runtime-core package which I think should involve environment specific code as less as possible and that's why there's a runtime-dom package.
Here is my idea.
- don't traverse object which is marked as
ReactiveFlags.SKIP
withmarkRaw
- wrap with
markRaw
inreactive
const state = Vue.reactive({
inputRef: Vue.markRaw(inputRef),
someVal: 0,
})
@edison1105
You should use a shallowReactive
or markRaw()
https://v3.vuejs.org/api/basic-reactivity.html#markraw to mark the element (or any other complex object) when passed to Vue. This will also improve the perf.
I don't think we should treat the DOM element in any special way. Maybe there is a way to document this so it appears when searching the docs
Official documentation uses simple refs for this purpose: https://v3.vuejs.org/guide/composition-api-template-refs.html#template-refs
@Sociosarbis When using refs on HTMLElement or anything else, that is not common data container it will not be replaced by proxy, because code checks concrete type of that value. That's why vue reactivity system already not consider dom elements as normal objects, and they really are not normal objects. And I think this is special case when reactivity system not behaving correctly.
I have a bug using library @posva I absolutly agree with you about shallowRef, but shallows and markRaw will not help in this case, html element already not marked as reactive and it should not be traversed by watch functions
Before I submit the PR, I have realized that using Node
is inappropriate. When I saw the code below, I still submitted a PR.
https://github.com/vuejs/vue-next/blob/master/packages/runtime-core/src/components/Teleport.ts#L29
const isTargetSVG = (target: RendererElement): boolean =>
typeof SVGElement !== 'undefined' && target instanceof SVGElement
@Sociosarbis @posva I agree with this solution and document it in the docs.
const state = Vue.reactive({
inputRef: Vue.markRaw(inputRef),
someVal: 0,
})
Use markRaw
still need to modify the code in the traverse
function.
But I prefer the solution in my PR, which reduces the burden on users and solves the problem under the hood.
As there is already a previous case involved environment specific code, "determine an object is intanceof Node" could be accpected. But when we changed the code to:
app.component('comp-2', {
template: `<input :value="value" />`,
props: {
value: Number
},
setup() {
}
})
app.component('comp', {
template: `<comp-2 ref="inputRef" />`,
setup() {
const inputRef = Vue.ref()
const state = Vue.reactive({
inputRef,
someVal: 0,
})
Vue.watch(
() => state,
() => {},
{deep: true}
)
return {
state,
inputRef
}
}
})
This time ref is a component not element, but a similar warning still pops up. So I think the solution supposed is a little ad hoc.
It should be a common situation to include template ref in reactive object, so I think @edison1105 's PR is reasonable, of course, this has additional runtime overhead. There are still other situations here, such as including host objects(e.g. window
) or components in the reactive object, but they are not as common as template ref, so we can document it.