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(tree): add deep option for the watch function of data #1188

Merged
merged 1 commit into from
Jan 4, 2021

Conversation

BeADre
Copy link
Contributor

@BeADre BeADre commented Jan 3, 2021

fix #1185

@jw-foss
Copy link
Member

jw-foss commented Jan 4, 2021

Though this temporarily fix this issue, deep watch data is not suggested though, it will have perf penalty when user injects a relatively large chunk of data.

@jw-foss
Copy link
Member

jw-foss commented Jan 4, 2021

The ultimate way to solve this is that we want the watcher to take care of themselves, for example:

We have a tree data like:

const data = {
  node1: {
     children: [],
  },
  // ... others
}

We want the parent in this case node1 to watch its own length of children, and each children watches themself, so that every time we trigger a change we don't ended up triggering a full scan over the entire data, could you figure a way that can manage to do so?

@BeADre
Copy link
Contributor Author

BeADre commented Jan 4, 2021

I'm sorry, I don't understand what 'a full scan over the entire data' means

@jw-foss
Copy link
Member

jw-foss commented Jan 4, 2021

I'm sorry, I don't understand what 'a full scan over the entire data' means

For example.

If you have an object over 1M fields, the deep option adds watcher to all of them, you'll get 1M setter/getter + 1(the object itself).

@jw-foss
Copy link
Member

jw-foss commented Jan 4, 2021

We can merge this issue right now for temporary fix, but if you are interested in perf optimization, let me know.

@jw-foss jw-foss merged commit dd341a7 into element-plus:dev Jan 4, 2021
@BeADre
Copy link
Contributor Author

BeADre commented Jan 4, 2021

我英文不太好。。。 这段我就用中文表述吧 就是我个人认为proxy对字段的拦截是惰性的 不会像Object.defineProperty那样去主动给每个字段添加gettersetter,他的目标是对象数组本身 只有对对象数组的属性进行get set delete has等操作时 才会惰性执行对应的拦截器,所以并不是每个属性都会进行初始化拦截器,只有对象数组会。这样就已经绕过了很多属性的拦截操作了。我们的设计目的是达到修改任何节点(不限于children的长度操作)都能在视图中响应变化,所以在tree的数据结构中孩子节点的proxy也是绕不开的

@jw-foss
Copy link
Member

jw-foss commented Jan 4, 2021

我英文不太好。。。 这段我就用中文表述吧 就是我个人认为proxy对字段的拦截是惰性的 不会像Object.defineProperty那样去主动给每个字段添加gettersetter,他的目标是对象数组本身 只有对对象数组的属性进行get set delete has等操作时 才会惰性执行对应的拦截器,所以并不是每个属性都会进行初始化拦截器,只有对象数组会。这样就已经绕过了很多属性的拦截操作了。我们的设计目的是达到修改任何节点(不限于children的长度操作)都能在视图中响应变化,所以在tree的数据结构中孩子节点的proxy也是绕不开的

Ok,我们可以先保留看看后续会不会有什么问题。确实那个 1M getter/setter 的表述是有问题的,不过考虑到 Vue 还会提供一个 Compat 版本,这个问题可能会被人提出来,保留观察一下吧。我个人觉得大概率不会有人给 tree 喂很多数据。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug report] tree: data cannot respond
2 participants