From ca3f2212d1665d5f6c57ffd96306e5f44037feeb Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Wed, 3 Dec 2025 18:59:37 +0100 Subject: [PATCH] Fix element already registered (#281000) Fixes microsoft/vscode-pull-request-github#8073 --- .../src/singlefolder-tests/tree.test.ts | 106 ++++++++++++++++++ .../workbench/api/common/extHostTreeViews.ts | 19 +++- 2 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts new file mode 100644 index 00000000000..1d19bff6902 --- /dev/null +++ b/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts @@ -0,0 +1,106 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import 'mocha'; +import * as vscode from 'vscode'; +import { asPromise, assertNoRpc, disposeAll, delay, DeferredPromise } from '../utils'; + +suite('vscode API - tree', () => { + + const disposables: vscode.Disposable[] = []; + + teardown(() => { + disposeAll(disposables); + disposables.length = 0; + assertNoRpc(); + }); + + test('TreeView - element already registered', async function () { + this.timeout(60_000); + + type TreeElement = { readonly kind: 'leaf' }; + + class QuickRefreshTreeDataProvider implements vscode.TreeDataProvider { + private readonly changeEmitter = new vscode.EventEmitter(); + private readonly requestEmitter = new vscode.EventEmitter(); + private readonly pendingRequests: DeferredPromise[] = []; + private readonly element: TreeElement = { kind: 'leaf' }; + + readonly onDidChangeTreeData = this.changeEmitter.event; + + getChildren(element?: TreeElement): Thenable { + if (!element) { + const deferred = new DeferredPromise(); + this.pendingRequests.push(deferred); + this.requestEmitter.fire(this.pendingRequests.length); + return deferred.p; + } + return Promise.resolve([]); + } + + getTreeItem(): vscode.TreeItem { + const item = new vscode.TreeItem('duplicate', vscode.TreeItemCollapsibleState.None); + item.id = 'dup'; + return item; + } + + getParent(): TreeElement | undefined { + return undefined; + } + + async waitForRequestCount(count: number): Promise { + while (this.pendingRequests.length < count) { + await asPromise(this.requestEmitter.event); + } + } + + async resolveNextRequest(): Promise { + const next = this.pendingRequests.shift(); + if (!next) { + return; + } + await next.complete([this.element]); + } + + dispose(): void { + this.changeEmitter.dispose(); + this.requestEmitter.dispose(); + while (this.pendingRequests.length) { + this.pendingRequests.shift()!.complete([]); + } + } + + getElement(): TreeElement { + return this.element; + } + } + + const provider = new QuickRefreshTreeDataProvider(); + disposables.push(provider); + + const treeView = vscode.window.createTreeView('test.treeId', { treeDataProvider: provider }); + disposables.push(treeView); + + const revealFirst = (treeView.reveal(provider.getElement(), { expand: true }) + .then(() => ({ error: undefined as Error | undefined })) as Promise<{ error: Error | undefined }>) + .catch(error => ({ error })); + const revealSecond = (treeView.reveal(provider.getElement(), { expand: true }) + .then(() => ({ error: undefined as Error | undefined })) as Promise<{ error: Error | undefined }>) + .catch(error => ({ error })); + + await provider.waitForRequestCount(2); + + await provider.resolveNextRequest(); + await delay(0); + await provider.resolveNextRequest(); + + const [firstResult, secondResult] = await Promise.all([revealFirst, revealSecond]); + const error = firstResult.error ?? secondResult.error; + if (error && /Element with id .+ is already registered/.test(error.message)) { + assert.fail(error.message); + } + }); +}); diff --git a/src/vs/workbench/api/common/extHostTreeViews.ts b/src/vs/workbench/api/common/extHostTreeViews.ts index ebd9978ddbe..55bd6ffe2d2 100644 --- a/src/vs/workbench/api/common/extHostTreeViews.ts +++ b/src/vs/workbench/api/common/extHostTreeViews.ts @@ -842,10 +842,23 @@ class ExtHostTreeView extends Disposable { } private _createAndRegisterTreeNode(element: T, extTreeItem: vscode.TreeItem, parentNode: TreeNode | Root): TreeNode { - const node = this._createTreeNode(element, extTreeItem, parentNode); - if (extTreeItem.id && this._elements.has(node.item.handle)) { - throw new Error(localize('treeView.duplicateElement', 'Element with id {0} is already registered', extTreeItem.id)); + const duplicateHandle = extTreeItem.id ? `${ExtHostTreeView.ID_HANDLE_PREFIX}/${extTreeItem.id}` : undefined; + if (duplicateHandle) { + const existingElement = this._elements.get(duplicateHandle); + if (existingElement) { + if (existingElement !== element) { + throw new Error(localize('treeView.duplicateElement', 'Element with id {0} is already registered', extTreeItem.id)); + } + const existingNode = this._nodes.get(existingElement); + if (existingNode) { + const newNode = this._createTreeNode(element, extTreeItem, parentNode); + this._updateNodeCache(element, newNode, existingNode, parentNode); + existingNode.dispose(); + return newNode; + } + } } + const node = this._createTreeNode(element, extTreeItem, parentNode); this._addNodeToCache(element, node); this._addNodeToParentCache(node, parentNode); return node;