Skip to content

Commit

Permalink
fix(cdk/tree): warn if mixed node types are used within the same tree
Browse files Browse the repository at this point in the history
Currently the tree somewhat works if a flat node and a nested node are used together, however they can break down depending on the data that is passed in.

These changes add a warning that will tell users to use a consistent node type.

Fixes #29927.

(cherry picked from commit cc53322)
  • Loading branch information
crisbeto committed Nov 12, 2024
1 parent 9a154cc commit f6066c2
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 22 deletions.
11 changes: 2 additions & 9 deletions src/cdk/tree/nested-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
IterableDiffer,
IterableDiffers,
OnDestroy,
OnInit,
QueryList,
inject,
} from '@angular/core';
Expand Down Expand Up @@ -40,8 +39,9 @@ import {CdkTreeNode} from './tree';
})
export class CdkNestedTreeNode<T, K = T>
extends CdkTreeNode<T, K>
implements AfterContentInit, OnDestroy, OnInit
implements AfterContentInit, OnDestroy
{
protected override _type: 'flat' | 'nested' = 'nested';
protected _differs = inject(IterableDiffers);

/** Differ used to find the changes in the data provided by the data source. */
Expand Down Expand Up @@ -75,13 +75,6 @@ export class CdkNestedTreeNode<T, K = T>
.subscribe(() => this.updateChildrenNodes());
}

// This is a workaround for https://github.com/angular/angular/issues/23091
// In aot mode, the lifecycle hooks from parent class are not called.
override ngOnInit() {
this._tree._setNodeTypeIfUnset('nested');
super.ngOnInit();
}

override ngOnDestroy() {
this._clear();
super.ngOnDestroy();
Expand Down
21 changes: 14 additions & 7 deletions src/cdk/tree/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,17 @@ export class CdkTree<T, K = T>
* This will be called by the first node that's rendered in order for the tree
* to determine what data transformations are required.
*/
_setNodeTypeIfUnset(nodeType: 'flat' | 'nested') {
if (this._nodeType.value === null) {
this._nodeType.next(nodeType);
_setNodeTypeIfUnset(newType: 'flat' | 'nested') {
const currentType = this._nodeType.value;

if (currentType === null) {
this._nodeType.next(newType);
} else if ((typeof ngDevMode === 'undefined' || ngDevMode) && currentType !== newType) {
console.warn(
`Tree is using conflicting node types which can cause unexpected behavior. ` +
`Please use tree nodes of the same type (e.g. only flat or only nested). ` +
`Current node type: "${currentType}", new node type "${newType}".`,
);
}
}

Expand Down Expand Up @@ -1169,6 +1177,7 @@ export class CdkTreeNode<T, K = T> implements OnDestroy, OnInit, TreeKeyManagerI
_elementRef = inject<ElementRef<HTMLElement>>(ElementRef);
protected _tree = inject<CdkTree<T, K>>(CdkTree);
protected _tabindex: number | null = -1;
protected readonly _type: 'flat' | 'nested' = 'flat';

/**
* The role of the tree node.
Expand Down Expand Up @@ -1368,10 +1377,8 @@ export class CdkTreeNode<T, K = T> implements OnDestroy, OnInit, TreeKeyManagerI
map(() => this.isExpanded),
distinctUntilChanged(),
)
.subscribe(() => {
this._changeDetectorRef.markForCheck();
});
this._tree._setNodeTypeIfUnset('flat');
.subscribe(() => this._changeDetectorRef.markForCheck());
this._tree._setNodeTypeIfUnset(this._type);
this._tree._registerNode(this);
}

Expand Down
4 changes: 2 additions & 2 deletions src/material/tree/testing/tree-harness.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,9 @@ interface ExampleFlatNode {
</mat-tree>
<mat-tree [dataSource]="nestedTreeDataSource" [treeControl]="nestedTreeControl">
<!-- This is the tree node template for leaf nodes -->
<mat-tree-node *matTreeNodeDef="let node" matTreeNodeToggle>
<mat-nested-tree-node *matTreeNodeDef="let node" matTreeNodeToggle>
{{node.name}}
</mat-tree-node>
</mat-nested-tree-node>
<!-- This is the tree node template for expandable nodes -->
<mat-nested-tree-node *matTreeNodeDef="let node; when: nestedTreeHasChild" isExpandable>
<button matTreeNodeToggle>
Expand Down
10 changes: 6 additions & 4 deletions tools/public_api_guard/cdk/tree.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export abstract class BaseTreeControl<T, K = T> implements TreeControl<T, K> {
export const CDK_TREE_NODE_OUTLET_NODE: InjectionToken<{}>;

// @public
export class CdkNestedTreeNode<T, K = T> extends CdkTreeNode<T, K> implements AfterContentInit, OnDestroy, OnInit {
export class CdkNestedTreeNode<T, K = T> extends CdkTreeNode<T, K> implements AfterContentInit, OnDestroy {
constructor(...args: unknown[]);
protected _children: T[];
protected _clear(): void;
Expand All @@ -64,9 +64,9 @@ export class CdkNestedTreeNode<T, K = T> extends CdkTreeNode<T, K> implements Af
ngAfterContentInit(): void;
// (undocumented)
ngOnDestroy(): void;
// (undocumented)
ngOnInit(): void;
nodeOutlet: QueryList<CdkTreeNodeOutlet>;
// (undocumented)
protected _type: 'flat' | 'nested';
protected updateChildrenNodes(children?: T[]): void;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<CdkNestedTreeNode<any, any>, "cdk-nested-tree-node", ["cdkNestedTreeNode"], {}, {}, ["nodeOutlet"], never, true, never>;
Expand Down Expand Up @@ -118,7 +118,7 @@ export class CdkTree<T, K = T> implements AfterContentChecked, AfterContentInit,
_registerNode(node: CdkTreeNode<T, K>): void;
renderNodeChanges(data: readonly T[], dataDiffer?: IterableDiffer<T>, viewContainer?: ViewContainerRef, parentData?: T): void;
protected _sendKeydownToKeyManager(event: KeyboardEvent): void;
_setNodeTypeIfUnset(nodeType: 'flat' | 'nested'): void;
_setNodeTypeIfUnset(newType: 'flat' | 'nested'): void;
toggle(dataNode: T): void;
toggleDescendants(dataNode: T): void;
trackBy: TrackByFunction<T>;
Expand Down Expand Up @@ -205,6 +205,8 @@ export class CdkTreeNode<T, K = T> implements OnDestroy, OnInit, TreeKeyManagerI
protected _tabindex: number | null;
// (undocumented)
protected _tree: CdkTree<T, K>;
// (undocumented)
protected readonly _type: 'flat' | 'nested';
typeaheadLabel: string | null;
unfocus(): void;
// (undocumented)
Expand Down

0 comments on commit f6066c2

Please sign in to comment.