N00ts/vue3-treeview

Computed property for nodes and config

Gurzhii opened this issue · 7 comments

Hello! First of all, I want to thank you for the job you've done!

I have one question, I can't find in the docs. How can I use nodes and config as computed properties? It's not working for me at all. It renders the structure but select/expand features are not working at all.

Issue: I need to combine data from the parent with the initial tree state. (parent data is reactive and may be changed on the fly, so I need to use computed prop in the child component where I have the tree)

Example: https://codesandbox.io/s/basic-forked-h5h960?file=/src/App.vue

@Gurzhii, I found the same. It works if I use Vue 3 refs, but using computed properties(Pinia getters) is not working.

I think I will check the package's source code to see if I can find the issue and a workaround for it.

@N00ts here's the link to reproduce the issue, any suggestions?

@Gurzhii @N00ts, here's a quick update:

The tree will only work as expected if you pass a reactive property such as ref or reactive.

This is because the nodes are a computed property here:

    const state: IState = {
        id: uniqueId(),
        nodes: computed(() => nodes.value),
        config: computed(() => config.value),

From the docs:

A computed property automatically tracks its reactive dependencies.

Just an example so you can understand:

This is reactive:

  data() => {
    return {
      defaultNodes: {
         ....
      }
   },

   computed: {
    nodes() {
      return this.defaultNodes;
    },
  },

This is NOT reactive:

  computed: {
    nodes() {
      return {
        id1: {
          text: "text1",
          children: ["id11", "id12", "id13"],
        },
    } 

When you have computed properties not relying on reactive data, Vue returns a plain object instead of a Proxy.

Also, when using toRefs and destructuring the nodes and config props values, nodes.values and config.values will be reactive only if they're initially reactive values. Please check this example.

@N00ts That said, I think that a good solution could be checking if the refs are reactive already, if not, we can create a ref with the value and ensure that is reactive, something like this:

import { toRefs, computed, ComputedRef, Ref, ref, isReactive } from 'vue';

export function createState(props: ITreeProps): string {
    let { nodes, config } = toRefs(props);

    if(!isReactive(nodes.value)) {
        nodes = ref(nodes.value);
    }
    ....
}    

What do you think?

@N00ts If this looks good, I will be happy to open a pull request.

N00ts commented

Looks fine ! Or maybe juste error message to avoir people making this mistake !

@N00ts, there are many cases that we'll cover by adding these changes. For example, I'd say the very basic example you can show is passing the nodes and config as plain objects without declaring any variables(ref or reactive), and this is not currently working either, please check here.

N00ts commented

Yes but just a warning message can be a good thing instead of letting people having bad practices about Reactivity, don't you think ?