refactor(tree): avoid improper query usage

The way things are set up at the moment we have a `CdkNestedTreeNode` which uses a `ContentChildren` to find a `CdkTreeNodeOutlet` so that it knows where to render its content. On top of this we have a `MatNestedTreeNode` which extends `CdkNestedTreeNode` and defines it's own query at the same property, but looking for a `MatTreeNodeOutlet`. With this setup it means that both `CdkNestedTreeNode` and `MatNestedTreeNode` are trying to write to the same `nodeOutlet` property and it's by pure coincidence that it works. This is very fragile and it isn't guaranteed to work in future versions of the framework.

These changes fix the issue by pulling out all of the `CdkNestedTreeNode` functionality into an abstract base class that both `CdkTreeNodeOutlet` and `MatTreeNodeOutlet` extend and which enforces that there's only one query writing to the `nodeOutlet` property.

Author: crisbeto

src/cdk/tree/nested-node.ts

@@ -19,57 +19,20 @@ import {Observable} from 'rxjs';
 import {takeUntil} from 'rxjs/operators';
 
 import {CDK_TREE_NODE_OUTLET_NODE, CdkTreeNodeOutlet} from './outlet';
-import {CdkTree, CdkTreeNode} from './tree';
+import {CdkTree, CdkTreeNode, CdkTreeNodeBase} from './tree';
 import {getTreeControlFunctionsMissingError} from './tree-errors';
 
-/**
- * Nested node is a child of `<cdk-tree>`. It works with nested tree.
- * By using `cdk-nested-tree-node` component in tree node template, children of the parent node will
- * be added in the `cdkTreeNodeOutlet` in tree node template.
- * For example:
- *   ```html
- *   <cdk-nested-tree-node>
- *     {{node.name}}
- *     <ng-template cdkTreeNodeOutlet></ng-template>
- *   </cdk-nested-tree-node>
- *   ```
- * The children of node will be automatically added to `cdkTreeNodeOutlet`, the result dom will be
- * like this:
- *   ```html
- *   <cdk-nested-tree-node>
- *     {{node.name}}
- *      <cdk-nested-tree-node>{{child1.name}}</cdk-nested-tree-node>
- *      <cdk-nested-tree-node>{{child2.name}}</cdk-nested-tree-node>
- *   </cdk-nested-tree-node>
- *   ```
- */
-@Directive({
-  selector: 'cdk-nested-tree-node',
-  exportAs: 'cdkNestedTreeNode',
-  host: {
-    '[attr.aria-expanded]': 'isExpanded',
-    '[attr.role]': 'role',
-    'class': 'cdk-tree-node cdk-nested-tree-node',
-  },
-  providers: [
-    {provide: CdkTreeNode, useExisting: CdkNestedTreeNode},
-    {provide: CDK_TREE_NODE_OUTLET_NODE, useExisting: CdkNestedTreeNode}
-  ]
-})
-export class CdkNestedTreeNode<T> extends CdkTreeNode<T> implements AfterContentInit, OnDestroy {
+/** Base class with the functionality of a `CdkNestedTreeNode`. */
+export abstract class CdkNestedTreeNodeBase<T> extends CdkTreeNodeBase<T> implements
+  AfterContentInit, OnDestroy {
   /** Differ used to find the changes in the data provided by the data source. */
   private _dataDiffer: IterableDiffer<T>;
 
   /** The children data dataNodes of current node. They will be placed in `CdkTreeNodeOutlet`. */
   protected _children: T[];
 
   /** The children node placeholder. */
-  @ContentChildren(CdkTreeNodeOutlet, {
-    // We need to use `descendants: true`, because Ivy will no longer match
-    // indirect descendants if it's left as false.
-    descendants: true
-  })
-  nodeOutlet: QueryList<CdkTreeNodeOutlet>;
+  abstract nodeOutlet: QueryList<CdkTreeNodeOutlet>;
 
   constructor(protected _elementRef: ElementRef<HTMLElement>,
               protected _tree: CdkTree<T>,
@@ -133,3 +96,51 @@ export class CdkNestedTreeNode<T> extends CdkTreeNode<T> implements AfterContent
     }
   }
 }
+
+/**
+ * Nested node is a child of `<cdk-tree>`. It works with nested tree.
+ * By using `cdk-nested-tree-node` component in tree node template, children of the parent node will
+ * be added in the `cdkTreeNodeOutlet` in tree node template.
+ * For example:
+ *   ```html
+ *   <cdk-nested-tree-node>
+ *     {{node.name}}
+ *     <ng-template cdkTreeNodeOutlet></ng-template>
+ *   </cdk-nested-tree-node>
+ *   ```
+ * The children of node will be automatically added to `cdkTreeNodeOutlet`, the result dom will be
+ * like this:
+ *   ```html
+ *   <cdk-nested-tree-node>
+ *     {{node.name}}
+ *      <cdk-nested-tree-node>{{child1.name}}</cdk-nested-tree-node>
+ *      <cdk-nested-tree-node>{{child2.name}}</cdk-nested-tree-node>
+ *   </cdk-nested-tree-node>
+ *   ```
+ */
+@Directive({
+  selector: 'cdk-nested-tree-node',
+  exportAs: 'cdkNestedTreeNode',
+  host: {
+    '[attr.aria-expanded]': 'isExpanded',
+    '[attr.role]': 'role',
+    'class': 'cdk-tree-node cdk-nested-tree-node',
+  },
+  providers: [
+    {provide: CdkTreeNode, useExisting: CdkNestedTreeNode},
+    {provide: CDK_TREE_NODE_OUTLET_NODE, useExisting: CdkNestedTreeNode}
+  ]
+})
+export class CdkNestedTreeNode<T> extends CdkNestedTreeNodeBase<T> {
+  /** The children node placeholder. */
+  @ContentChildren(CdkTreeNodeOutlet, {
+    // We need to use `descendants: true`, because Ivy will no longer match
+    // indirect descendants if it's left as false.
+    descendants: true
+  })
+  nodeOutlet: QueryList<CdkTreeNodeOutlet>;
+
+  constructor(elementRef: ElementRef<HTMLElement>, tree: CdkTree<T>, differs: IterableDiffers) {
+    super(elementRef, tree, differs);
+  }
+}

src/cdk/tree/tree.ts

@@ -279,26 +279,13 @@ export class CdkTree<T> implements AfterContentChecked, CollectionViewer, OnDest
   }
 }
 
-
-/**
- * Tree node for CdkTree. It contains the data in the tree node.
- */
-@Directive({
-  selector: 'cdk-tree-node',
-  exportAs: 'cdkTreeNode',
-  host: {
-    '[attr.aria-expanded]': 'isExpanded',
-    '[attr.aria-level]': 'role === "treeitem" ? level : null',
-    '[attr.role]': 'role',
-    'class': 'cdk-tree-node',
-  },
-})
-export class CdkTreeNode<T> implements FocusableOption, OnDestroy {
+/** Base class with the functionality of a `CdkTreeNode`. */
+export class CdkTreeNodeBase<T> implements FocusableOption, OnDestroy {
   /**
    * The most recently created `CdkTreeNode`. We save it in static variable so we can retrieve it
    * in `CdkTree` and set the data to it.
    */
-  static mostRecentTreeNode: CdkTreeNode<any> | null = null;
+  static mostRecentTreeNode: CdkTreeNodeBase<any> | null = null;
 
   /** Subject that emits when the component has been destroyed. */
   protected _destroyed = new Subject<void>();
@@ -333,14 +320,14 @@ export class CdkTreeNode<T> implements FocusableOption, OnDestroy {
 
   constructor(protected _elementRef: ElementRef<HTMLElement>,
               protected _tree: CdkTree<T>) {
-    CdkTreeNode.mostRecentTreeNode = this as CdkTreeNode<T>;
+    CdkTreeNodeBase.mostRecentTreeNode = this as CdkTreeNodeBase<T>;
   }
 
   ngOnDestroy() {
     // If this is the last tree node being destroyed,
     // clear out the reference to avoid leaking memory.
-    if (CdkTreeNode.mostRecentTreeNode === this) {
-      CdkTreeNode.mostRecentTreeNode = null;
+    if (CdkTreeNodeBase.mostRecentTreeNode === this) {
+      CdkTreeNodeBase.mostRecentTreeNode = null;
     }
 
     this._dataChanges.complete();
@@ -374,3 +361,22 @@ export class CdkTreeNode<T> implements FocusableOption, OnDestroy {
     this.role = children && children.length ? 'group' : 'treeitem';
   }
 }
+
+/**
+ * Tree node for CdkTree. It contains the data in the tree node.
+ */
+@Directive({
+  selector: 'cdk-tree-node',
+  exportAs: 'cdkTreeNode',
+  host: {
+    '[attr.aria-expanded]': 'isExpanded',
+    '[attr.aria-level]': 'role === "treeitem" ? level : null',
+    '[attr.role]': 'role',
+    'class': 'cdk-tree-node',
+  },
+})
+export class CdkTreeNode<T> extends CdkTreeNodeBase<T> {
+  constructor(elementRef: ElementRef<HTMLElement>, tree: CdkTree<T>) {
+    super(elementRef, tree);
+  }
+}

src/material/tree/node.ts

@@ -12,6 +12,7 @@ import {
   CdkTree,
   CdkTreeNode,
   CdkTreeNodeDef,
+  CdkNestedTreeNodeBase,
 } from '@angular/cdk/tree';
 import {
   AfterContentInit,
@@ -32,16 +33,13 @@ import {
   mixinDisabled,
   mixinTabIndex,
 } from '@angular/material/core';
+import {coerceBooleanProperty} from '@angular/cdk/coercion';
 
 import {MatTreeNodeOutlet} from './outlet';
 
 const _MatTreeNodeMixinBase: HasTabIndexCtor & CanDisableCtor & typeof CdkTreeNode =
     mixinTabIndex(mixinDisabled(CdkTreeNode));
 
-const _MatNestedTreeNodeMixinBase:
-    HasTabIndexCtor & CanDisableCtor & typeof CdkNestedTreeNode =
-        mixinTabIndex(mixinDisabled(CdkNestedTreeNode));
-
 /**
  * Wrapper for the CdkTree node with Material design styles.
  */
@@ -95,15 +93,14 @@ export class MatTreeNodeDef<T> extends CdkTreeNodeDef<T> {
     '[attr.role]': 'role',
     'class': 'mat-nested-tree-node',
   },
-  inputs: ['disabled', 'tabIndex'],
   providers: [
     {provide: CdkNestedTreeNode, useExisting: MatNestedTreeNode},
     {provide: CdkTreeNode, useExisting: MatNestedTreeNode},
     {provide: CDK_TREE_NODE_OUTLET_NODE, useExisting: MatNestedTreeNode}
   ]
 })
-export class MatNestedTreeNode<T> extends _MatNestedTreeNodeMixinBase<T> implements
-    AfterContentInit, CanDisable, HasTabIndex, OnDestroy {
+export class MatNestedTreeNode<T> extends CdkNestedTreeNodeBase<T> implements AfterContentInit,
+  OnDestroy {
   @Input('matNestedTreeNode') node: T;
 
   /** The children node placeholder. */
@@ -114,12 +111,25 @@ export class MatNestedTreeNode<T> extends _MatNestedTreeNodeMixinBase<T> impleme
   })
   nodeOutlet: QueryList<MatTreeNodeOutlet>;
 
+  /** Whether the tree node is disabled. */
+  @Input()
+  get disabled() { return this._disabled; }
+  set disabled(value: any) { this._disabled = coerceBooleanProperty(value); }
+  private _disabled = false;
+
+  /** Tabindex for the tree node. */
+  @Input()
+  get tabIndex(): number { return this.disabled ? -1 : this._tabIndex; }
+  set tabIndex(value: number) {
+    this._tabIndex = value != null ? value : 0;
+  }
+  private _tabIndex: number;
+
   constructor(protected _elementRef: ElementRef<HTMLElement>,
               protected _tree: CdkTree<T>,
               protected _differs: IterableDiffers,
               @Attribute('tabindex') tabIndex: string) {
     super(_elementRef, _tree, _differs);
-
     this.tabIndex = Number(tabIndex) || 0;
   }
 

tools/public_api_guard/cdk/tree.d.ts

@@ -18,12 +18,17 @@ export declare abstract class BaseTreeControl<T> implements TreeControl<T> {
 
 export declare const CDK_TREE_NODE_OUTLET_NODE: InjectionToken<{}>;
 
-export declare class CdkNestedTreeNode<T> extends CdkTreeNode<T> implements AfterContentInit, OnDestroy {
+export declare class CdkNestedTreeNode<T> extends CdkNestedTreeNodeBase<T> {
+    nodeOutlet: QueryList<CdkTreeNodeOutlet>;
+    constructor(elementRef: ElementRef<HTMLElement>, tree: CdkTree<T>, differs: IterableDiffers);
+}
+
+export declare abstract class CdkNestedTreeNodeBase<T> extends CdkTreeNodeBase<T> implements AfterContentInit, OnDestroy {
     protected _children: T[];
     protected _differs: IterableDiffers;
     protected _elementRef: ElementRef<HTMLElement>;
     protected _tree: CdkTree<T>;
-    nodeOutlet: QueryList<CdkTreeNodeOutlet>;
+    abstract nodeOutlet: QueryList<CdkTreeNodeOutlet>;
     constructor(_elementRef: ElementRef<HTMLElement>, _tree: CdkTree<T>, _differs: IterableDiffers);
     protected _clear(): void;
     ngAfterContentInit(): void;
@@ -53,7 +58,11 @@ export declare class CdkTree<T> implements AfterContentChecked, CollectionViewer
 export declare class CdkTreeModule {
 }
 
-export declare class CdkTreeNode<T> implements FocusableOption, OnDestroy {
+export declare class CdkTreeNode<T> extends CdkTreeNodeBase<T> {
+    constructor(elementRef: ElementRef<HTMLElement>, tree: CdkTree<T>);
+}
+
+export declare class CdkTreeNodeBase<T> implements FocusableOption, OnDestroy {
     protected _data: T;
     _dataChanges: Subject<void>;
     protected _destroyed: Subject<void>;
@@ -68,7 +77,7 @@ export declare class CdkTreeNode<T> implements FocusableOption, OnDestroy {
     protected _setRoleFromData(): void;
     focus(): void;
     ngOnDestroy(): void;
-    static mostRecentTreeNode: CdkTreeNode<any> | null;
+    static mostRecentTreeNode: CdkTreeNodeBase<any> | null;
 }
 
 export declare class CdkTreeNodeDef<T> {

tools/public_api_guard/material/tree.d.ts

@@ -1,9 +1,11 @@
-export declare class MatNestedTreeNode<T> extends _MatNestedTreeNodeMixinBase<T> implements AfterContentInit, CanDisable, HasTabIndex, OnDestroy {
+export declare class MatNestedTreeNode<T> extends CdkNestedTreeNodeBase<T> implements AfterContentInit, OnDestroy {
     protected _differs: IterableDiffers;
     protected _elementRef: ElementRef<HTMLElement>;
     protected _tree: CdkTree<T>;
+    disabled: any;
     node: T;
     nodeOutlet: QueryList<MatTreeNodeOutlet>;
+    tabIndex: number;
     constructor(_elementRef: ElementRef<HTMLElement>, _tree: CdkTree<T>, _differs: IterableDiffers, tabIndex: string);
     ngAfterContentInit(): void;
     ngOnDestroy(): void;