From 950ca05d5c9fe4a7cc356b7cb02dc08d895d8b02 Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Fri, 12 Dec 2025 18:01:41 +0100 Subject: [PATCH] Reapply element already registered race condition fix (#283079) Fixes microsoft/vscode-pull-request-github#8073 --- .../src/singlefolder-tests/tree.test.ts | 2 +- .../workbench/api/common/extHostTreeViews.ts | 19 ++++++++++++++++--- .../workbench/browser/parts/views/treeView.ts | 12 +++++++++--- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts index cfbd8bff51c..5382fb5777e 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts @@ -18,7 +18,7 @@ suite('vscode API - tree', () => { assertNoRpc(); }); - test.skip('TreeView - element already registered', async function () { + test('TreeView - element already registered', async function () { this.timeout(60_000); type TreeElement = { readonly kind: 'leaf' }; diff --git a/src/vs/workbench/api/common/extHostTreeViews.ts b/src/vs/workbench/api/common/extHostTreeViews.ts index f21f207f426..f50257579be 100644 --- a/src/vs/workbench/api/common/extHostTreeViews.ts +++ b/src/vs/workbench/api/common/extHostTreeViews.ts @@ -863,10 +863,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; diff --git a/src/vs/workbench/browser/parts/views/treeView.ts b/src/vs/workbench/browser/parts/views/treeView.ts index 48c342e84fa..c8cae02f383 100644 --- a/src/vs/workbench/browser/parts/views/treeView.ts +++ b/src/vs/workbench/browser/parts/views/treeView.ts @@ -693,7 +693,7 @@ abstract class AbstractTreeView extends Disposable implements ITreeView { const treeMenus = this.treeDisposables.add(this.instantiationService.createInstance(TreeMenus, this.id)); this.treeLabels = this.treeDisposables.add(this.instantiationService.createInstance(ResourceLabels, this)); const dataSource = this.instantiationService.createInstance(TreeDataSource, this, (task: Promise) => this.progressService.withProgress({ location: this.id }, () => task)); - const aligner = this.treeDisposables.add(new Aligner(this.themeService)); + const aligner = this.treeDisposables.add(new Aligner(this.themeService, this.logService)); const checkboxStateHandler = this.treeDisposables.add(new CheckboxStateHandler()); const renderer = this.treeDisposables.add(this.instantiationService.createInstance(TreeRenderer, this.id, treeMenus, this.treeLabels, actionViewItemProvider, aligner, checkboxStateHandler, () => this.manuallyManageCheckboxes)); this.treeDisposables.add(renderer.onDidChangeCheckboxState(e => this._onDidChangeCheckboxState.fire(e))); @@ -1631,7 +1631,7 @@ class TreeRenderer extends Disposable implements ITreeRenderer | undefined; - constructor(private themeService: IThemeService) { + constructor(private themeService: IThemeService, private logService: ILogService) { super(); } @@ -1649,7 +1649,13 @@ class Aligner extends Disposable { if (this._tree) { const root = this._tree.getInput(); - const parent: ITreeItem = this._tree.getParentElement(treeItem) || root; + let parent: ITreeItem; + try { + parent = this._tree.getParentElement(treeItem) || root; + } catch (error) { + this.logService.error(`[TreeView] Failed to resolve parent for ${treeItem.handle}`, error); + return false; + } if (this.hasIconOrCheckbox(parent)) { return !!parent.children && parent.children.some(c => c.collapsibleState !== TreeItemCollapsibleState.None && !this.hasIconOrCheckbox(c)); }