From 1051f2a1bac911e36e47b07ba29c045de83d5293 Mon Sep 17 00:00:00 2001 From: nagahara-naoki <118710021+nagahara-naoki@users.noreply.github.com> Date: Wed, 4 Nov 2026 20:43:28 +0900 Subject: [PATCH 1/2] fix(aria/tree): avoid errors when destroyed before initialization Remember the tree, parent group, and owning item used during registration so destruction does not read required inputs before ngOnInit has run. Add regression tests for early destruction and controls for initialized trees. Fixes #34058 --- src/aria/tree/tree-item-group.ts | 10 ++++-- src/aria/tree/tree-item.ts | 21 ++++++++---- src/aria/tree/tree.spec.ts | 57 ++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 10 deletions(-) diff --git a/src/aria/tree/tree-item-group.ts b/src/aria/tree/tree-item-group.ts index 94d3a8e19358..a89c9b8921ce 100644 --- a/src/aria/tree/tree-item-group.ts +++ b/src/aria/tree/tree-item-group.ts @@ -54,6 +54,9 @@ export class TreeItemGroup implements OnInit, OnDestroy { /** The DeferredContent host directive. */ private readonly _deferredContent = inject(DeferredContent); + /** The tree item that this group was registered with. */ + private _registeredOwner: TreeItem | undefined; + /** All groupable items that are descendants of the group. */ private readonly _unorderedItems = signal(new Set>()); @@ -66,12 +69,13 @@ export class TreeItemGroup implements OnInit, OnDestroy { readonly ownedBy = input.required>(); ngOnInit() { - this._deferredContent.deferredContentAware.set(this.ownedBy()); - this.ownedBy()._register(this); + this._registeredOwner = this.ownedBy(); + this._deferredContent.deferredContentAware.set(this._registeredOwner); + this._registeredOwner._register(this); } ngOnDestroy() { - this.ownedBy()._unregister(); + this._registeredOwner?._unregister(); } _register(child: TreeItem) { diff --git a/src/aria/tree/tree-item.ts b/src/aria/tree/tree-item.ts index cc799708bb9d..ac62752720fe 100644 --- a/src/aria/tree/tree-item.ts +++ b/src/aria/tree/tree-item.ts @@ -65,6 +65,12 @@ export class TreeItem extends DeferredContentAware implements OnInit, OnDestr /** The owned tree item group. */ private readonly _group = signal | undefined>(undefined); + /** The parent group that this item was registered with. */ + private _registeredParentGroup: TreeItemGroup | undefined; + + /** The tree that this item was registered with. */ + private _registeredTree: Tree | undefined; + /** A unique identifier for the tree item. */ readonly id = input(inject(_IdGenerator).getId('ng-tree-item-', true)); @@ -127,10 +133,13 @@ export class TreeItem extends DeferredContentAware implements OnInit, OnDestr } ngOnInit() { - if (this.parent() instanceof TreeItemGroup) { - (this.parent() as TreeItemGroup)._register(this); + const parent = this.parent(); + if (parent instanceof TreeItemGroup) { + this._registeredParentGroup = parent; + parent._register(this); } - this.tree()._collection.register(this); + this._registeredTree = this.tree(); + this._registeredTree._collection.register(this); const treePattern = computed(() => this.tree()._pattern); const parentPattern = computed(() => { @@ -151,10 +160,8 @@ export class TreeItem extends DeferredContentAware implements OnInit, OnDestr } ngOnDestroy() { - if (this.parent() instanceof TreeItemGroup) { - (this.parent() as TreeItemGroup)._unregister(this); - } - this.tree()._collection.unregister(this); + this._registeredParentGroup?._unregister(this); + this._registeredTree?._collection.unregister(this); } _register(group: TreeItemGroup) { diff --git a/src/aria/tree/tree.spec.ts b/src/aria/tree/tree.spec.ts index 28f2c17a6795..ee4922abf213 100644 --- a/src/aria/tree/tree.spec.ts +++ b/src/aria/tree/tree.spec.ts @@ -1616,6 +1616,63 @@ describe('Tree', () => { }); }); +describe('Tree destruction', () => { + it('should destroy a tree item without reading its required parent input', () => { + const fixture = TestBed.createComponent(StaticFlatTree); + + expect(() => fixture.destroy()).not.toThrow(); + }); + + it('should destroy a nested tree without reading required inputs', () => { + const fixture = TestBed.createComponent(StaticNestedTree); + + expect(() => fixture.destroy()).not.toThrow(); + }); + + it('should destroy an initialized tree item', async () => { + const fixture = TestBed.createComponent(StaticFlatTree); + await fixture.whenStable(); + + expect(() => fixture.destroy()).not.toThrow(); + }); + + it('should destroy an initialized nested tree', async () => { + const fixture = TestBed.createComponent(StaticNestedTree); + await fixture.whenStable(); + + expect(() => fixture.destroy()).not.toThrow(); + }); +}); + +@Component({ + template: ` +
    +
  • Item
    • +
+ `, + imports: [Tree, TreeItem], + changeDetection: ChangeDetectionStrategy.Eager, +}) +class StaticFlatTree {} + +@Component({ + template: ` +
    +
  • + Parent +
      + +
    • Child
      • +       +
    +
    • +
+ `, + imports: [Tree, TreeItem, TreeItemGroup], + changeDetection: ChangeDetectionStrategy.Eager, +}) +class StaticNestedTree {} + interface TestTreeNode { value: V; label: string; From ebc2c98f038b69ff2afa31cf8c171af3c5fb2978 Mon Sep 17 00:00:00 2001 From: nagahara-naoki <118710021+nagahara-naoki@users.noreply.github.com> Date: Thu, 5 Nov 2026 21:26:36 +0900 Subject: [PATCH 2/2] fix(aria/tree): manage registration with effects Keep registration and cleanup together using effects instead of retained references. Cover changing tree and group owner inputs and update the API golden for removed lifecycle hooks. --- goldens/aria/tree/index.api.md | 11 ++--- src/aria/tree/tree-item-group.ts | 26 ++++++----- src/aria/tree/tree-item.ts | 40 ++++++++--------- src/aria/tree/tree.spec.ts | 74 ++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 43 deletions(-) diff --git a/goldens/aria/tree/index.api.md b/goldens/aria/tree/index.api.md index 43ebd9647e00..629680f41f8e 100644 --- a/goldens/aria/tree/index.api.md +++ b/goldens/aria/tree/index.api.md @@ -42,7 +42,7 @@ export class Tree implements OnDestroy { } // @public -export class TreeItem extends DeferredContentAware implements OnInit, OnDestroy, HasElement { +export class TreeItem extends DeferredContentAware implements OnInit, HasElement { constructor(); readonly active: Signal; readonly disabled: _angular_core.InputSignalWithTransform; @@ -53,8 +53,6 @@ export class TreeItem extends DeferredContentAware implements OnInit, OnDestr readonly label: _angular_core.InputSignal; readonly level: Signal; // (undocumented) - ngOnDestroy(): void; - // (undocumented) ngOnInit(): void; readonly parent: _angular_core.InputSignal | TreeItemGroup>; _pattern: TreeItemPattern; @@ -75,13 +73,10 @@ export class TreeItem extends DeferredContentAware implements OnInit, OnDestr } // @public -export class TreeItemGroup implements OnInit, OnDestroy { +export class TreeItemGroup { + constructor(); readonly _childPatterns: _angular_core.Signal[]>; readonly element: HTMLElement; - // (undocumented) - ngOnDestroy(): void; - // (undocumented) - ngOnInit(): void; readonly ownedBy: _angular_core.InputSignal>; // (undocumented) _register(child: TreeItem): void; diff --git a/src/aria/tree/tree-item-group.ts b/src/aria/tree/tree-item-group.ts index a89c9b8921ce..fcf3b5e15722 100644 --- a/src/aria/tree/tree-item-group.ts +++ b/src/aria/tree/tree-item-group.ts @@ -13,8 +13,8 @@ import { inject, input, signal, - OnInit, - OnDestroy, + effect, + untracked, } from '@angular/core'; import {TreeItemPattern, DeferredContent, sortDirectives} from '../private'; import type {TreeItem} from './tree-item'; @@ -44,7 +44,7 @@ import type {TreeItem} from './tree-item'; exportAs: 'ngTreeItemGroup', hostDirectives: [DeferredContent], }) -export class TreeItemGroup implements OnInit, OnDestroy { +export class TreeItemGroup { /** A reference to the host element. */ private readonly _elementRef = inject(ElementRef); @@ -54,9 +54,6 @@ export class TreeItemGroup implements OnInit, OnDestroy { /** The DeferredContent host directive. */ private readonly _deferredContent = inject(DeferredContent); - /** The tree item that this group was registered with. */ - private _registeredOwner: TreeItem | undefined; - /** All groupable items that are descendants of the group. */ private readonly _unorderedItems = signal(new Set>()); @@ -68,14 +65,15 @@ export class TreeItemGroup implements OnInit, OnDestroy { /** Tree item that owns the group. */ readonly ownedBy = input.required>(); - ngOnInit() { - this._registeredOwner = this.ownedBy(); - this._deferredContent.deferredContentAware.set(this._registeredOwner); - this._registeredOwner._register(this); - } - - ngOnDestroy() { - this._registeredOwner?._unregister(); + constructor() { + effect(onCleanup => { + const owner = this.ownedBy(); + untracked(() => { + this._deferredContent.deferredContentAware.set(owner); + owner._register(this); + }); + onCleanup(() => owner._unregister()); + }); } _register(child: TreeItem) { diff --git a/src/aria/tree/tree-item.ts b/src/aria/tree/tree-item.ts index ac62752720fe..a14991fe1bc5 100644 --- a/src/aria/tree/tree-item.ts +++ b/src/aria/tree/tree-item.ts @@ -18,7 +18,8 @@ import { signal, Signal, OnInit, - OnDestroy, + effect, + untracked, } from '@angular/core'; import {_IdGenerator} from '@angular/cdk/a11y'; import {TreeItemPattern, DeferredContentAware, HasElement} from '../private'; @@ -55,7 +56,7 @@ import {TreeItemGroup} from './tree-item-group'; '[attr.tabindex]': '_pattern.tabIndex()', }, }) -export class TreeItem extends DeferredContentAware implements OnInit, OnDestroy, HasElement { +export class TreeItem extends DeferredContentAware implements OnInit, HasElement { /** A reference to the host element. */ private readonly _elementRef = inject(ElementRef); @@ -65,12 +66,6 @@ export class TreeItem extends DeferredContentAware implements OnInit, OnDestr /** The owned tree item group. */ private readonly _group = signal | undefined>(undefined); - /** The parent group that this item was registered with. */ - private _registeredParentGroup: TreeItemGroup | undefined; - - /** The tree that this item was registered with. */ - private _registeredTree: Tree | undefined; - /** A unique identifier for the tree item. */ readonly id = input(inject(_IdGenerator).getId('ng-tree-item-', true)); @@ -125,6 +120,22 @@ export class TreeItem extends DeferredContentAware implements OnInit, OnDestr constructor() { super(); + effect(onCleanup => { + const parent = this.parent(); + const tree = this.tree(); + untracked(() => { + if (parent instanceof TreeItemGroup) { + parent._register(this); + } + tree._collection.register(this); + }); + onCleanup(() => { + if (parent instanceof TreeItemGroup) { + parent._unregister(this); + } + tree._collection.unregister(this); + }); + }); afterRenderEffect({ write: () => { this.contentVisible.set(this._pattern.expanded()); @@ -133,14 +144,6 @@ export class TreeItem extends DeferredContentAware implements OnInit, OnDestr } ngOnInit() { - const parent = this.parent(); - if (parent instanceof TreeItemGroup) { - this._registeredParentGroup = parent; - parent._register(this); - } - this._registeredTree = this.tree(); - this._registeredTree._collection.register(this); - const treePattern = computed(() => this.tree()._pattern); const parentPattern = computed(() => { if (this.parent() instanceof Tree) { @@ -159,11 +162,6 @@ export class TreeItem extends DeferredContentAware implements OnInit, OnDestr }); } - ngOnDestroy() { - this._registeredParentGroup?._unregister(this); - this._registeredTree?._collection.unregister(this); - } - _register(group: TreeItemGroup) { this._group.set(group); } diff --git a/src/aria/tree/tree.spec.ts b/src/aria/tree/tree.spec.ts index ee4922abf213..3495332bf40f 100644 --- a/src/aria/tree/tree.spec.ts +++ b/src/aria/tree/tree.spec.ts @@ -1617,6 +1617,51 @@ describe('Tree', () => { }); describe('Tree destruction', () => { + it('should move an item to its new tree and unregister it on destruction', async () => { + const fixture = TestBed.createComponent(TreeWithChangingParent); + await fixture.whenStable(); + const trees = fixture.debugElement + .queryAll(By.directive(Tree)) + .map(el => el.injector.get(Tree)); + const item = fixture.debugElement.query(By.directive(TreeItem)).injector.get(TreeItem); + + expect(trees[0]._collection.orderedItems()).toEqual([item]); + expect(trees[1]._collection.orderedItems()).toEqual([]); + + fixture.componentInstance.parent.set(trees[1]); + await fixture.whenStable(); + + expect(trees[0]._collection.orderedItems()).toEqual([]); + expect(trees[1]._collection.orderedItems()).toEqual([item]); + + fixture.destroy(); + expect(trees[1]._collection.orderedItems()).toEqual([]); + }); + + it('should move a group to its new owner and unregister it on destruction', async () => { + const fixture = TestBed.createComponent(TreeWithChangingOwner); + await fixture.whenStable(); + const items = fixture.debugElement + .queryAll(By.directive(TreeItem)) + .map(el => el.injector.get(TreeItem)); + const group = fixture.debugElement + .queryAllNodes(By.directive(TreeItemGroup))[0] + .injector.get(TreeItemGroup); + const unregisterFirst = spyOn(items[0], '_unregister').and.callThrough(); + const registerSecond = spyOn(items[1], '_register').and.callThrough(); + const unregisterSecond = spyOn(items[1], '_unregister').and.callThrough(); + + fixture.componentInstance.owner.set(items[1]); + await fixture.whenStable(); + + expect(unregisterFirst).toHaveBeenCalledTimes(1); + expect(registerSecond).toHaveBeenCalledOnceWith(group); + + fixture.destroy(); + expect(unregisterFirst).toHaveBeenCalledTimes(1); + expect(unregisterSecond).toHaveBeenCalledTimes(1); + }); + it('should destroy a tree item without reading its required parent input', () => { const fixture = TestBed.createComponent(StaticFlatTree); @@ -1673,6 +1718,35 @@ class StaticFlatTree {} }) class StaticNestedTree {} +@Component({ + template: ` +
    +
  • Item
    • +
+
    + `, + imports: [Tree, TreeItem], + changeDetection: ChangeDetectionStrategy.Eager, +}) +class TreeWithChangingParent { + parent = signal | undefined>(undefined); +} + +@Component({ + template: ` +
      +
    • First
      • +
    • Second
      • + +
    + `, + imports: [Tree, TreeItem, TreeItemGroup], + changeDetection: ChangeDetectionStrategy.Eager, +}) +class TreeWithChangingOwner { + owner = signal | undefined>(undefined); +} + interface TestTreeNode { value: V; label: string;