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 94d3a8e19358..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); @@ -65,13 +65,15 @@ export class TreeItemGroup implements OnInit, OnDestroy { /** Tree item that owns the group. */ readonly ownedBy = input.required>(); - ngOnInit() { - this._deferredContent.deferredContentAware.set(this.ownedBy()); - this.ownedBy()._register(this); - } - - ngOnDestroy() { - this.ownedBy()._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 cc799708bb9d..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); @@ -119,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()); @@ -127,11 +144,6 @@ export class TreeItem extends DeferredContentAware implements OnInit, OnDestr } ngOnInit() { - if (this.parent() instanceof TreeItemGroup) { - (this.parent() as TreeItemGroup)._register(this); - } - this.tree()._collection.register(this); - const treePattern = computed(() => this.tree()._pattern); const parentPattern = computed(() => { if (this.parent() instanceof Tree) { @@ -150,13 +162,6 @@ 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); - } - _register(group: TreeItemGroup) { this._group.set(group); } diff --git a/src/aria/tree/tree.spec.ts b/src/aria/tree/tree.spec.ts index 28f2c17a6795..3495332bf40f 100644 --- a/src/aria/tree/tree.spec.ts +++ b/src/aria/tree/tree.spec.ts @@ -1616,6 +1616,137 @@ 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); + + 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 {} + +@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;