search

microsoft / vscode-azureresourcegroups

VS Code extension for managing Azure resources.
https://marketplace.visualstudio.com/items?itemName=ms-azuretools.vscode-azureresourcegroups
MIT License
53 stars 28 forks source link

`findTreeItem` fails for an app resource child contributed by a yet-to-be-activated extension #416

Open alexweininger opened 2 years ago

alexweininger commented 2 years ago

Databases will persist the connected database for each workspace. On activation, if the workspace has a connected database saved it does findTreeItem to get the tree item and then connects to it.

This specific use case of findTreeItem fails fairly consistently. Since the tree item we're looking for is a child of an application resource, when findTreeItem is executed, its success depends on if the tree has updated to use the newly registered Databases branch data provider to get the tree items (rather than the DefaultApplicationResourceBranchDataProvider). Since if the tree still has the tree items given by DefaultApplicationResourceBranchDataProvider, they won't have children.

Not sure what the best solution is to this. Should we have mechanisms in RGs to solve this? This could be a common scenario, basically anytime you try to do findTreeItem for a tree item that is contributed by a yet-to-be-activated extension.

Note: this repros about half the time for me. Depends on how fast the extension activates and the tree refreshes I guess.

nturinski commented 2 years ago

Doesn't the tree item have to be resolved before we can invoke findTreeItem on it? Maybe we can check the azExtResourceType property on the tree item and make sure the appropriate extension is activated before we look at which resolver to use?

EDIT: I'm not entirely what the new implementation of findTreeItem is, but I assume it still has to travel down the tree (eventually hitting AppResourceTreeItem). If that's not the case, this wouldn't work whatsoever.

alexweininger commented 2 years ago

Doesn't the tree item have to be resolved before we can invoke findTreeItem on it?

findTreeItem takes a tree item id, so you can call it whenever/however you want - even if the items haven't been resolved yet.

EDIT: I'm not entirely what the new implementation of findTreeItem is, but I assume it still has to travel down the tree (eventually hitting AppResourceTreeItem). If that's not the case, this wouldn't work whatsoever.

Correct, however we need this functionality for at least a few features I know of, and probably a lot more that I haven't thought of.

Calling findTreeItem to reveal a tree item that hasn't yet been displayed is pretty common. And with how the unified view works by lazy activating extensions, this scenario is probably pretty common.

I think one solution is if an extension contributes a BDP, to not show shallow tree items for those resources. Instead, just wait for the extension to activate and show the resolved tree items. I don't really like how it behaves anyway.

Video of what I'm talking about:

https://user-images.githubusercontent.com/12476526/199114551-e42aed86-2943-4f67-b03b-a117510a576a.mov

alexweininger commented 2 years ago

Note: this only reproduces for Databases if the extension hasn't activated when user expands the group tree item.

bwateratmsft commented 2 years ago

We really need to reconsider features that require one extension to access another's tree nodes. That's a bad design and now's the time to revisit it. Accessing data and functions from other extensions makes sense to me, but tree nodes in particular, that's just asking for trouble.