N00ts / vue3-treeview

a simple treeview in vue js 3
MIT License
70 stars 60 forks source link

Computed property for nodes and config #22

Open Gurzhii opened 1 year ago

Gurzhii commented 1 year ago

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

roberto910907 commented 1 year ago

@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.

roberto910907 commented 1 year ago

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

roberto910907 commented 1 year ago

@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?

roberto910907 commented 1 year ago

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

N00ts commented 1 year ago

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

roberto910907 commented 1 year ago

@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 1 year ago

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