Skip to content

Commit

Permalink
fix tree view - reveal not to invalidate item command
Browse files Browse the repository at this point in the history
The issue is a regression introduced in #8783
Calculation of parent chain (for id calculation) calls getChildren which dispose nodes.
This means that reveal for a node that is already revealed dispose the node and so the frontend has an id that doesn't exist in the backend anymore so tree item command is not found.

The fix is to check the cache before calling getChildren and making sure that the technical root is also in the cache.

Signed-off-by: Amiram Wingarten <amiram.wingarten@sap.com>
  • Loading branch information
amiramw committed Jan 5, 2021
1 parent 39887bf commit 89e3293
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions packages/plugin-ext/src/plugin/tree/tree-views.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,11 @@ class TreeViewExtImpl<T> implements Disposable {
if (treeItem.id) {
return chain.concat(treeItem.id);
}
// getChildren fills this.nodes and generate ids for them which are needed later
const children = await this.getChildren(parentId);
const cachedParentNode = this.nodes.get(parentId);
// first try to get children length from cache since getChildren disposes old nodes, which can cause a race
// condition if command is executed together with reveal.
// If not in cache, getChildren fills this.nodes and generate ids for them which are needed later
const children = cachedParentNode?.children || await this.getChildren(parentId);
if (!children) {
return undefined; // parent is inconsistent
}
Expand Down Expand Up @@ -303,13 +306,17 @@ class TreeViewExtImpl<T> implements Disposable {

async getChildren(parentId: string): Promise<TreeViewItem[] | undefined> {
const parentNode = this.nodes.get(parentId);
const parent = parentNode && parentNode.value;
const parent = parentNode?.value;
if (parentId && !parent) {
console.error(`No tree item with id '${parentId}' found.`);
return [];
}
this.clearChildren(parentNode);

// place root in the cache
if (parentId === '') {
this.nodes.set(parentId, { id: '', dispose: () => { } });
}
// ask data provider for children for cached element
const result = await this.treeDataProvider.getChildren(parent);
if (result) {
Expand Down

0 comments on commit 89e3293

Please sign in to comment.