Nodes with loadChildrenAsync and expanded:true do not expand on init (v3.0.5)

[monfortm]

Describe the bug I use v.3.0.5

I try to initialise a node as expanded, but I cannot.

I tried to set expandable: true and state: {expanded: true} on the node model, but this does not auto expand children.

Note I have a model defaults with expandable: true & expanded: false and an async children loading function set.

To Reproduce

• https://codesandbox.io/s/vigorous-glade-ri5roh?file=/src/App.vue
• Comment/Uncomment the loadChildrenAsync function

Expected behavior

I am not sure what should be the standard behavior here but IMO:

• Initialise tree with current static info
• Use loadChildrenAsync thereafter.

An alternative is of course to ignore the children information as soon as loadChildrenAsync is available and auto trigger it when expanded is true.

Does this make sense?

On latest firerefox

[grapoza]

Thanks for the writeup and the codepen! It sounds like you've played with this enough to have a good handle on what's going on.

I tried to set expandable: true and state: {expanded: true} on the node model, but this does not auto expand children.

Note I have a model defaults with expandable: true & expanded: false and an async children loading function set.

As you noted in your reproduction steps, it's the loadChildrenAsync that keeps the expanded property from taking effect. That's as designed; if you specify a loadChildrenAsync method, expanded is actually forced to false until you manually trigger an expansion. That's what you're seeing in the Node with a child node. It's assumed there will only be one source of children, either loadChildrenAsync or children defined on the node model, so this makes sense for that case.

Side note, in 3.x if you do provide both the function and the static children like in the example pen, both will show up until the function returns and the children list is overwritten. in 5.1.x the static list will be ignored in that case.

I am not sure what should be the standard behavior here but IMO:

• Initialise tree with current static info
• Use loadChildrenAsync thereafter.

An alternative is of course to ignore the children information as soon as loadChildrenAsync is available and auto trigger it when expanded is true.

Let me step back here and get a better idea of what you're trying to do. I think what you're saying is you'd like to have the node's expanded property honored instead of forced to false and have the async loader fired immediately? Assuming that's right, the only downside there is that a poorly placed default of expanded = true could cause a pretty gnarly cascade. :) What we usually do is pre-fetch the data we know we'll need expanded, before the tree is even initialized. I don't know your use case though, so I realize that may not be an option for you. Alternatively you could use the click hack from past issues to trigger expansion once the tree is mounted if you just need it for a few nodes at the root, or I could add a tree option to honor the expanded property with async children.

It'll be nice in 5.x when I finish up the refactoring into composables, and all this stuff is centralized and based off of watching the actual expanded property and other state properties. Good times.

[monfortm]

Hello,

Hmmm, yeah now I am not sure what I'd expect as a behavior ahah.

IMO, if expanded is true and a list of children is avail, one should display this children list. That is until the node is contracted and expanded again, where loadChildrenAsync takes over. This means that one can store their tree in memory somewhere, destroy the component, re-init at a later time without:

1. having to use a different modelDefaults (my model default contains the loadChildrenAsync, but I cannot just keep this method there if it prevents nodes to expand... instead I have to inject it in the model of each node that are not expanded yet... but then I have a tree that mixes nodes that can be reloaded (the ones that have the loadChildrenAsync), and nodes that cannot because they were initialised with data.
2. having to re-fetch (automatically or not) all the expanded node data with a loadChildrenAsync set.

Basically my point is, the children data is loaded and the node is expanded --> the data should be displayed, regardless of whether the node itself knows how to load its own data upon expansion

Not sure if this is so clear, let me know if I should try to write a clearer explanation :)

[grapoza]

Ah, OK, I think I get your use case now. So the crux of the problem is that rehydrating from state isn't going right for you because of the child loading rules, but I think I can get around that. I'm just spitballing here, but I'd propose something like:

• Add a new tree (node?) option, something like childLoadPrecedence that can be either asyncOverStatic (current behavior) or staticOverAsync (use static children if there, otherwise use async loading). Using a new option defaulted to current behavior will keep it from being a major version release, so we stay in the 3.x line.
• That new behavior will ignore async loading rules for the expansion, allowing you to have static children and an async loading method, and the expanded nodes with static children will initially be shown as expanded. Async loading will be ignored since static children take precedence.
• For convenience, I can build on what we've discussed before to expose a resetChildren method that will handle clearing existing children and reloading if the node is expanded, all that fun stuff

I think that combo would allow you to build the initial tree, initialized with async loading methods in your modelDefaults, then let the user interact with it to async load data. When they navigate away and come back, you'd be able to rehydrate the tree with your stored data which will include all of the already-loaded children and their state, and the modelDefaults wouldn't need to be changed based on whether something has been loaded or not. The resetChildren method would be needed since we don't want to re-fetch already fetched async data automatically, but you could do it programmatically as your needs dictate.

Again, just spitballing on my lunch break, but I think an approach like this would work. Your thoughts? Anything I missed?

[monfortm]

Yes, that sound about right! Cool if you manage to add it to v3!

[grapoza]

Yeah, I think I can do that. I'm flying today, so this may give me something fun to do on the plane. :) I'll switch this issue to an enhancement and track it here for 3.x, and probably split out a separate issue to add it to 5.x.

[grapoza]

@monfortm Alright, I've released 3.1.1 which includes the changes for the child node handling. Before I close this, I'd like to have you give it a look and see if it meets your needs. The documentation has been updated with the new childrenLoadPrecedence property, and here's the demo page that shows the reload function in use.

[monfortm]

Hey @grapoza thanks a lot for the update!

I tried it out but cannot make it work as expected. Let me know if I am using it wrong but in this codesandbox example, the model does not initialise as I was expecting it to do, i.e. the data is there but the state in treeNodeSpec: { expandable: true, state: { expanded: true } } set on nodes and/or inside modelDefaults is ignored.

I was expecting the initial tree to load and expand nodes accordingly. Did I use it correctly?

With this sandbox, I am expected that:

• The tree load based on the data model (because childrenLoadPrecedence is set to "staticBeforeAsync")
• Nodes expand following intrinsic state props and/or model defaults state
• Contracting and later (re)expanding a node reloads its content based on loadChildrenAsync

(and sorry for the sluggish response but I can work on it only some days, cheers)

[grapoza]

No worries about the response time, I'm also only able to spend some time during the evenings on this so if either of us will be slowing this down it'll be me. :) For points one and two there's definitely something going on here that I wasn't seeing locally. I'll have to figure it out add some more tests to cover this, but I'm hoping it'll be trivial. I'll push a 3.1.2 once I nail it down. I'll see if I can get that done tonight.

For the third point, the contracting and re-expanding won't automatically trigger a reload. For that you'll need to track if the children have reloaded yourself and use the reloadNodeChildren method to reload them, probably in response to the treeViewNodeExpandedChange event which passes the expanded node's data.

[grapoza]

Okay, I see it. So the way I wrote this was with serialized data similar to what I thought your use case would be. So, the private treeNodeSpec._.areChildrenLoaded property was set in the tests and I used that in the code as part of the check for whether to use the provided children. Here's your sandbox with that modification where you can see it working. That approach bugged me because it relied on private data, but I figured it would probably be good enough for this case until I make a more robust approach for 5.x. Part of the reasoning there is that the children are already always an array by the time the state initialization code runs. Now that I have to look at it again though, I feel like I should just bite the bullet and refactor the node initialization to perform state initialization before children, so it can key off of the existence of children instead of relying on private data.

So yeah, that's how my night's going.

[grapoza]

3.1.2 is published. Here's the updated sandbox using that version. Looking back, I have no idea why I didn't just do it this way in the first place. Let me know what you think.

[monfortm]

Oh yeah, I seem to be able to achieve what I want 🔥 .

A bit sorry to annoy you with v3, but cool if it can remain one of the feature in v5 as well! I close the solved issue, will re open if I identify an issue with the feature!

Thank you!

[grapoza]

Awesome! Not annoying at all, I love knowing people are actually using this component, at any version, and it's even better when I get great feedback on how to improve it.

Just an FYI, I'm debating whether this new option will go into 5.x as-is, or if I just introduce the behavior of staticBeforeAsync as a breaking change in 6.0 and have it be the default behavior (no flag, it'd just be how it behaves from that point on). If you get to the point of upgrading to Vue 3 then keep that in mind, who knows which way I'll end up going...