fix(aria/tree): avoid errors when destroyed before initialization

goldens/aria/tree/index.api.md

@@ -42,7 +42,7 @@ export class Tree<V> implements OnDestroy {
 }
 
 // @public
-export class TreeItem<V> extends DeferredContentAware implements OnInit, OnDestroy, HasElement {
+export class TreeItem<V> extends DeferredContentAware implements OnInit, HasElement {
     constructor();
     readonly active: Signal<boolean>;
     readonly disabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
@@ -53,8 +53,6 @@ export class TreeItem<V> extends DeferredContentAware implements OnInit, OnDestr
     readonly label: _angular_core.InputSignal<string | undefined>;
     readonly level: Signal<number>;
     // (undocumented)
-    ngOnDestroy(): void;
-    // (undocumented)
     ngOnInit(): void;
     readonly parent: _angular_core.InputSignal<Tree<V> | TreeItemGroup<V>>;
     _pattern: TreeItemPattern<V>;
@@ -75,13 +73,10 @@ export class TreeItem<V> extends DeferredContentAware implements OnInit, OnDestr
 }
 
 // @public
-export class TreeItemGroup<V> implements OnInit, OnDestroy {
+export class TreeItemGroup<V> {
+    constructor();
     readonly _childPatterns: _angular_core.Signal<TreeItemPattern<V>[]>;
     readonly element: HTMLElement;
-    // (undocumented)
-    ngOnDestroy(): void;
-    // (undocumented)
-    ngOnInit(): void;
     readonly ownedBy: _angular_core.InputSignal<TreeItem<V>>;
     // (undocumented)
     _register(child: TreeItem<V>): void;

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<V> implements OnInit, OnDestroy {
+export class TreeItemGroup<V> {
   /** A reference to the host element. */
   private readonly _elementRef = inject(ElementRef);
 
@@ -65,13 +65,15 @@ export class TreeItemGroup<V> implements OnInit, OnDestroy {
   /** Tree item that owns the group. */
   readonly ownedBy = input.required<TreeItem<V>>();
 
-  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<V>) {

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<V> extends DeferredContentAware implements OnInit, OnDestroy, HasElement {
+export class TreeItem<V> extends DeferredContentAware implements OnInit, HasElement {
   /** A reference to the host element. */
   private readonly _elementRef = inject(ElementRef);
 
@@ -119,6 +120,22 @@ export class TreeItem<V> 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<V> extends DeferredContentAware implements OnInit, OnDestr
   }
 
   ngOnInit() {
-    if (this.parent() instanceof TreeItemGroup) {
-      (this.parent() as TreeItemGroup<V>)._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<V> extends DeferredContentAware implements OnInit, OnDestr
     });
   }
 
-  ngOnDestroy() {
-    if (this.parent() instanceof TreeItemGroup) {
-      (this.parent() as TreeItemGroup<V>)._unregister(this);
-    }
-    this.tree()._collection.unregister(this);
-  }
-
   _register(group: TreeItemGroup<V>) {
     this._group.set(group);
   }

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: `
+    <ul ngTree #tree="ngTree">
+      <li ngTreeItem [parent]="tree" value="item">Item</li>
+    </ul>
+  `,
+  imports: [Tree, TreeItem],
+  changeDetection: ChangeDetectionStrategy.Eager,
+})
+class StaticFlatTree {}
+
+@Component({
+  template: `
+    <ul ngTree #tree="ngTree">
+      <li ngTreeItem [parent]="tree" value="parent" #parentItem="ngTreeItem">
+        Parent
+        <ul role="group">
+          <ng-template ngTreeItemGroup [ownedBy]="parentItem" #group="ngTreeItemGroup">
+            <li ngTreeItem [parent]="group" value="child">Child</li>
+          </ng-template>
+        </ul>
+      </li>
+    </ul>
+  `,
+  imports: [Tree, TreeItem, TreeItemGroup],
+  changeDetection: ChangeDetectionStrategy.Eager,
+})
+class StaticNestedTree {}
+
+@Component({
+  template: `
+    <ul ngTree #first="ngTree">
+      <li ngTreeItem [parent]="parent() || first" value="item">Item</li>
+    </ul>
+    <ul ngTree #second="ngTree"></ul>
+  `,
+  imports: [Tree, TreeItem],
+  changeDetection: ChangeDetectionStrategy.Eager,
+})
+class TreeWithChangingParent {
+  parent = signal<Tree<unknown> | undefined>(undefined);
+}
+
+@Component({
+  template: `
+    <ul ngTree #tree="ngTree">
+      <li ngTreeItem [parent]="tree" value="first" #first="ngTreeItem">First</li>
+      <li ngTreeItem [parent]="tree" value="second">Second</li>
+      <ng-template ngTreeItemGroup [ownedBy]="owner() || first"></ng-template>
+    </ul>
+  `,
+  imports: [Tree, TreeItem, TreeItemGroup],
+  changeDetection: ChangeDetectionStrategy.Eager,
+})
+class TreeWithChangingOwner {
+  owner = signal<TreeItem<unknown> | undefined>(undefined);
+}
+
 interface TestTreeNode<V = string> {
   value: V;
   label: string;