Return undefined on tree refresh cancellation

Fixes #129019

src/vs/workbench/api/browser/mainThreadTreeViews.ts

@@ -186,14 +186,14 @@ class TreeViewDataProvider implements ITreeViewDataProvider {
 		this.hasResolve = this._proxy.$hasResolve(this.treeViewId);
 	}
 
-	getChildren(treeItem?: ITreeItem): Promise<ITreeItem[]> {
-		return Promise.resolve(this._proxy.$getChildren(this.treeViewId, treeItem ? treeItem.handle : undefined)
+	getChildren(treeItem?: ITreeItem): Promise<ITreeItem[] | undefined> {
+		return this._proxy.$getChildren(this.treeViewId, treeItem ? treeItem.handle : undefined)
 			.then(
 				children => this.postGetChildren(children),
 				err => {
 					this.notificationService.error(err);
 					return [];
-				}));
+				});
 	}
 
 	getItemsToRefresh(itemsToRefreshByHandle: { [treeItemHandle: string]: ITreeItem }): ITreeItem[] {
@@ -230,7 +230,10 @@ class TreeViewDataProvider implements ITreeViewDataProvider {
 		return this.itemsMap.size === 0;
 	}
 
-	private async postGetChildren(elements: ITreeItem[]): Promise<ResolvableTreeItem[]> {
+	private async postGetChildren(elements: ITreeItem[] | undefined): Promise<ResolvableTreeItem[] | undefined> {
+		if (elements === undefined) {
+			return undefined;
+		}
 		const result: ResolvableTreeItem[] = [];
 		const hasResolve = await this.hasResolve;
 		if (elements) {

src/vs/workbench/api/common/extHost.protocol.ts

@@ -1260,7 +1260,7 @@ export interface ExtHostDocumentsAndEditorsShape {
 }
 
 export interface ExtHostTreeViewsShape {
-	$getChildren(treeViewId: string, treeItemHandle?: string): Promise<ITreeItem[]>;
+	$getChildren(treeViewId: string, treeItemHandle?: string): Promise<ITreeItem[] | undefined>;
 	$onDrop(treeViewId: string, treeDataTransfer: TreeDataTransferDTO, newParentTreeItemHandle: string): Promise<void>;
 	$setExpanded(treeViewId: string, treeItemHandle: string, expanded: boolean): void;
 	$setSelection(treeViewId: string, treeItemHandles: string[]): void;

src/vs/workbench/api/common/extHostTreeViews.ts

@@ -121,7 +121,7 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape {
 		};
 	}
 
-	$getChildren(treeViewId: string, treeItemHandle?: string): Promise<ITreeItem[]> {
+	$getChildren(treeViewId: string, treeItemHandle?: string): Promise<ITreeItem[] | undefined> {
 		const treeView = this.treeViews.get(treeViewId);
 		if (!treeView) {
 			return Promise.reject(new Error(localize('treeView.notRegistered', 'No tree view with id \'{0}\' registered.', treeViewId)));
@@ -221,7 +221,7 @@ class ExtHostTreeView<T> extends Disposable {
 	private readonly dataProvider: vscode.TreeDataProvider<T>;
 	private readonly dndController: vscode.DragAndDropController<T> | undefined;
 
-	private roots: TreeNode[] | null = null;
+	private roots: TreeNode[] | undefined = undefined;
 	private elements: Map<TreeItemHandle, T> = new Map<TreeItemHandle, T>();
 	private nodes: Map<T, TreeNode> = new Map<T, TreeNode>();
 
@@ -309,16 +309,20 @@ class ExtHostTreeView<T> extends Disposable {
 		}));
 	}
 
-	getChildren(parentHandle: TreeItemHandle | Root): Promise<ITreeItem[]> {
+	async getChildren(parentHandle: TreeItemHandle | Root): Promise<ITreeItem[] | undefined> {
 		const parentElement = parentHandle ? this.getExtensionElement(parentHandle) : undefined;
 		if (parentHandle && !parentElement) {
 			this.logService.error(`No tree item with id \'${parentHandle}\' found.`);
 			return Promise.resolve([]);
 		}
 
-		const childrenNodes = this.getChildrenNodes(parentHandle); // Get it from cache
-		return (childrenNodes ? Promise.resolve(childrenNodes) : this.fetchChildrenNodes(parentElement))
-			.then(nodes => nodes.map(n => n.item));
+		let childrenNodes: TreeNode[] | undefined = this.getChildrenNodes(parentHandle); // Get it from cache
+
+		if (!childrenNodes) {
+			childrenNodes = await this.fetchChildrenNodes(parentElement);
+		}
+
+		return childrenNodes ? childrenNodes.map(n => n.item) : undefined;
 	}
 
 	getExtensionElement(treeItemHandle: TreeItemHandle): T | undefined {
@@ -473,7 +477,7 @@ class ExtHostTreeView<T> extends Disposable {
 				}));
 	}
 
-	private getChildrenNodes(parentNodeOrHandle: TreeNode | TreeItemHandle | Root): TreeNode[] | null {
+	private getChildrenNodes(parentNodeOrHandle: TreeNode | TreeItemHandle | Root): TreeNode[] | undefined {
 		if (parentNodeOrHandle) {
 			let parentNode: TreeNode | undefined;
 			if (typeof parentNodeOrHandle === 'string') {
@@ -482,12 +486,12 @@ class ExtHostTreeView<T> extends Disposable {
 			} else {
 				parentNode = parentNodeOrHandle;
 			}
-			return parentNode ? parentNode.children || null : null;
+			return parentNode ? parentNode.children || undefined : undefined;
 		}
 		return this.roots;
 	}
 
-	private async fetchChildrenNodes(parentElement?: T): Promise<TreeNode[]> {
+	private async fetchChildrenNodes(parentElement?: T): Promise<TreeNode[] | undefined> {
 		// clear children cache
 		this.clearChildren(parentElement);
 
@@ -497,7 +501,7 @@ class ExtHostTreeView<T> extends Disposable {
 			const parentNode = parentElement ? this.nodes.get(parentElement) : undefined;
 			const elements = await this.dataProvider.getChildren(parentElement);
 			if (cts.token.isCancellationRequested) {
-				return [];
+				return undefined;
 			}
 
 			const items = await Promise.all(coalesce(elements || []).map(async element => {
@@ -505,7 +509,7 @@ class ExtHostTreeView<T> extends Disposable {
 				return item && !cts.token.isCancellationRequested ? this.createAndRegisterTreeNode(element, item, parentNode) : null;
 			}));
 			if (cts.token.isCancellationRequested) {
-				return [];
+				return undefined;
 			}
 
 			return coalesce(items);
@@ -782,7 +786,7 @@ class ExtHostTreeView<T> extends Disposable {
 	}
 
 	private clearAll(): void {
-		this.roots = null;
+		this.roots = undefined;
 		this.elements.clear();
 		this.nodes.forEach(node => node.dispose());
 		this.nodes.clear();

src/vs/workbench/browser/parts/views/treeView.ts

@@ -274,8 +274,8 @@ abstract class AbstractTreeView extends Disposable implements ITreeView {
 						children = node.children;
 					} else {
 						node = node ?? self.root;
-						children = await (node instanceof Root ? dataProvider.getChildren() : dataProvider.getChildren(node));
-						node.children = children;
+						node.children = await (node instanceof Root ? dataProvider.getChildren() : dataProvider.getChildren(node));
+						children = node.children ?? [];
 					}
 					if (node instanceof Root) {
 						const oldEmpty = this._isEmpty;
@@ -806,7 +806,7 @@ class TreeDataSource implements IAsyncDataSource<ITreeItem, ITreeItem> {
 		let result: ITreeItem[] = [];
 		if (this.treeView.dataProvider) {
 			try {
-				result = await this.withProgress(this.treeView.dataProvider.getChildren(element));
+				result = (await this.withProgress(this.treeView.dataProvider.getChildren(element))) ?? [];
 			} catch (e) {
 				if (!(<string>e.message).startsWith('Bad progress location:')) {
 					throw e;

src/vs/workbench/common/views.ts

@@ -819,7 +819,7 @@ export class ResolvableTreeItem implements ITreeItem {
 export interface ITreeViewDataProvider {
 	readonly isTreeEmpty?: boolean;
 	onDidChangeEmpty?: Event<void>;
-	getChildren(element?: ITreeItem): Promise<ITreeItem[]>;
+	getChildren(element?: ITreeItem): Promise<ITreeItem[] | undefined>;
 }
 
 export const TREE_ITEM_DATA_TRANSFER_TYPE = 'text/treeitems';

src/vs/workbench/test/browser/api/extHostTreeViews.test.ts

@@ -90,25 +90,25 @@ suite('ExtHostTreeView', function () {
 	test('construct node tree', () => {
 		return testObject.$getChildren('testNodeTreeProvider')
 			.then(elements => {
-				const actuals = elements.map(e => e.handle);
+				const actuals = elements?.map(e => e.handle);
 				assert.deepStrictEqual(actuals, ['0/0:a', '0/0:b']);
 				return Promise.all([
 					testObject.$getChildren('testNodeTreeProvider', '0/0:a')
 						.then(children => {
-							const actuals = children.map(e => e.handle);
+							const actuals = children?.map(e => e.handle);
 							assert.deepStrictEqual(actuals, ['0/0:a/0:aa', '0/0:a/0:ab']);
 							return Promise.all([
-								testObject.$getChildren('testNodeTreeProvider', '0/0:a/0:aa').then(children => assert.strictEqual(children.length, 0)),
-								testObject.$getChildren('testNodeTreeProvider', '0/0:a/0:ab').then(children => assert.strictEqual(children.length, 0))
+								testObject.$getChildren('testNodeTreeProvider', '0/0:a/0:aa').then(children => assert.strictEqual(children?.length, 0)),
+								testObject.$getChildren('testNodeTreeProvider', '0/0:a/0:ab').then(children => assert.strictEqual(children?.length, 0))
 							]);
 						}),
 					testObject.$getChildren('testNodeTreeProvider', '0/0:b')
 						.then(children => {
-							const actuals = children.map(e => e.handle);
+							const actuals = children?.map(e => e.handle);
 							assert.deepStrictEqual(actuals, ['0/0:b/0:ba', '0/0:b/0:bb']);
 							return Promise.all([
-								testObject.$getChildren('testNodeTreeProvider', '0/0:b/0:ba').then(children => assert.strictEqual(children.length, 0)),
-								testObject.$getChildren('testNodeTreeProvider', '0/0:b/0:bb').then(children => assert.strictEqual(children.length, 0))
+								testObject.$getChildren('testNodeTreeProvider', '0/0:b/0:ba').then(children => assert.strictEqual(children?.length, 0)),
+								testObject.$getChildren('testNodeTreeProvider', '0/0:b/0:bb').then(children => assert.strictEqual(children?.length, 0))
 							]);
 						})
 				]);
@@ -118,25 +118,25 @@ suite('ExtHostTreeView', function () {
 	test('construct id tree', () => {
 		return testObject.$getChildren('testNodeWithIdTreeProvider')
 			.then(elements => {
-				const actuals = elements.map(e => e.handle);
+				const actuals = elements?.map(e => e.handle);
 				assert.deepStrictEqual(actuals, ['1/a', '1/b']);
 				return Promise.all([
 					testObject.$getChildren('testNodeWithIdTreeProvider', '1/a')
 						.then(children => {
-							const actuals = children.map(e => e.handle);
+							const actuals = children?.map(e => e.handle);
 							assert.deepStrictEqual(actuals, ['1/aa', '1/ab']);
 							return Promise.all([
-								testObject.$getChildren('testNodeWithIdTreeProvider', '1/aa').then(children => assert.strictEqual(children.length, 0)),
-								testObject.$getChildren('testNodeWithIdTreeProvider', '1/ab').then(children => assert.strictEqual(children.length, 0))
+								testObject.$getChildren('testNodeWithIdTreeProvider', '1/aa').then(children => assert.strictEqual(children?.length, 0)),
+								testObject.$getChildren('testNodeWithIdTreeProvider', '1/ab').then(children => assert.strictEqual(children?.length, 0))
 							]);
 						}),
 					testObject.$getChildren('testNodeWithIdTreeProvider', '1/b')
 						.then(children => {
-							const actuals = children.map(e => e.handle);
+							const actuals = children?.map(e => e.handle);
 							assert.deepStrictEqual(actuals, ['1/ba', '1/bb']);
 							return Promise.all([
-								testObject.$getChildren('testNodeWithIdTreeProvider', '1/ba').then(children => assert.strictEqual(children.length, 0)),
-								testObject.$getChildren('testNodeWithIdTreeProvider', '1/bb').then(children => assert.strictEqual(children.length, 0))
+								testObject.$getChildren('testNodeWithIdTreeProvider', '1/ba').then(children => assert.strictEqual(children?.length, 0)),
+								testObject.$getChildren('testNodeWithIdTreeProvider', '1/bb').then(children => assert.strictEqual(children?.length, 0))
 							]);
 						})
 				]);
@@ -200,7 +200,7 @@ suite('ExtHostTreeView', function () {
 		target.onRefresh.event(() => {
 			testObject.$getChildren('testNodeWithIdTreeProvider')
 				.then(elements => {
-					const actuals = elements.map(e => e.handle);
+					const actuals = elements?.map(e => e.handle);
 					assert.deepStrictEqual(actuals, ['1/a', '1/b']);
 					return testObject.$getChildren('testNodeWithIdTreeProvider', '1/a')
 						.then(() => testObject.$getChildren('testNodeWithIdTreeProvider', '1/b'))
@@ -398,7 +398,7 @@ suite('ExtHostTreeView', function () {
 		target.onRefresh.event(() => {
 			testObject.$getChildren('testNodeTreeProvider')
 				.then(elements => {
-					assert.deepStrictEqual(elements.map(e => e.handle), ['0/0:a//0:b']);
+					assert.deepStrictEqual(elements?.map(e => e.handle), ['0/0:a//0:b']);
 					done();
 				});
 		});
@@ -440,11 +440,11 @@ suite('ExtHostTreeView', function () {
 		target.onRefresh.event(() => {
 			testObject.$getChildren('testNodeTreeProvider')
 				.then(elements => {
-					const actuals = elements.map(e => e.handle);
+					const actuals = elements?.map(e => e.handle);
 					assert.deepStrictEqual(actuals, ['0/0:a', '0/0:b', '0/1:a', '0/0:d', '0/1:b', '0/0:f', '0/2:a']);
 					return testObject.$getChildren('testNodeTreeProvider', '0/1:b')
 						.then(elements => {
-							const actuals = elements.map(e => e.handle);
+							const actuals = elements?.map(e => e.handle);
 							assert.deepStrictEqual(actuals, ['0/1:b/0:h', '0/1:b/1:h', '0/1:b/0:j', '0/1:b/1:j', '0/1:b/2:h']);
 							done();
 						});
@@ -462,7 +462,7 @@ suite('ExtHostTreeView', function () {
 		target.onRefresh.event(() => {
 			testObject.$getChildren('testNodeTreeProvider')
 				.then(elements => {
-					assert.deepStrictEqual(elements.map(e => e.handle), ['0/0:c']);
+					assert.deepStrictEqual(elements?.map(e => e.handle), ['0/0:c']);
 					done();
 				});
 		});
@@ -477,7 +477,7 @@ suite('ExtHostTreeView', function () {
 
 		return testObject.$getChildren('testNodeTreeProvider')
 			.then(elements => {
-				assert.deepStrictEqual(elements.map(e => e.handle), ['0/0:a', '0/0:b']);
+				assert.deepStrictEqual(elements?.map(e => e.handle), ['0/0:a', '0/0:b']);
 			});
 	});
 
@@ -643,7 +643,7 @@ suite('ExtHostTreeView', function () {
 
 	function loadCompleteTree(treeId: string, element?: string): Promise<null> {
 		return testObject.$getChildren(treeId, element)
-			.then(elements => elements.map(e => loadCompleteTree(treeId, e.handle)))
+			.then(elements => elements?.map(e => loadCompleteTree(treeId, e.handle)))
 			.then(() => null);
 	}