Subscribe on changes!

deep watch traverse all dom if reactive element has ref to dom element

avatar
Apr 20th 2021

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

avatar
Apr 20th 2021

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.

  1. don't traverse object which is marked as ReactiveFlags.SKIP with markRaw
  2. wrap with markRaw in reactive
const state = Vue.reactive({
    inputRef: Vue.markRaw(inputRef),
    someVal: 0,
})

@edison1105

avatar
Apr 20th 2021

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

avatar
Apr 20th 2021

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

avatar
Apr 21st 2021

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.

avatar
Apr 21st 2021

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.

avatar
Apr 21st 2021

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.

avatar
May 28th 2021

This should've been fixed by 62b8f4a39ca56b48a8c8fdf7e200cb80735e16ae