From c7bb18c5cf19c3a036ae89cd0480c2dea79ec2a8 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Wed, 23 Dec 2020 16:21:49 +0200
Subject: [PATCH] fix(material-experimental/mdc-table): error when
 flexbox-based table is initialized

The MDC-based table tries to add a class to its internal `tbody` element and it assumes
that it's always going to find it. The assumption used to be correct since we only
supported native `table` elements for the MDC implementation, however as of #20994
we also support flexbox-based ones which throw an error, because they don't have a
`tbody`.

These changes add a check to prevent the error and include a couple of sanity tests to
catch issues like this in the future.

I've also reworked the tests so that they use the MDC-based `MatPaginator` and
`MatTableDataSource`, instead of the base ones.
---
 src/cdk/table/table.ts                        |  2 +-
 .../mdc-table/BUILD.bazel                     | 12 +++-
 .../mdc-table/table.spec.ts                   | 60 ++++++++++++++++++-
 src/material-experimental/mdc-table/table.ts  | 11 +++-
 tools/public_api_guard/cdk/table.d.ts         |  1 +
 5 files changed, 76 insertions(+), 10 deletions(-)

diff --git a/src/cdk/table/table.ts b/src/cdk/table/table.ts
index 07148435d1ea..4ebaafd0b0bf 100644
--- a/src/cdk/table/table.ts
+++ b/src/cdk/table/table.ts
@@ -325,7 +325,7 @@ export class CdkTable<T> implements AfterContentChecked, CollectionViewer, OnDes
   private _cachedRenderRowsMap = new Map<T, WeakMap<CdkRowDef<T>, RenderRow<T>[]>>();
 
   /** Whether the table is applied to a native `<table>`. */
-  private _isNativeHtmlTable: boolean;
+  protected _isNativeHtmlTable: boolean;
 
   /**
    * Utility class that is responsible for applying the appropriate sticky positioning styles to
diff --git a/src/material-experimental/mdc-table/BUILD.bazel b/src/material-experimental/mdc-table/BUILD.bazel
index 9687fb80fa6a..1dff676baba1 100644
--- a/src/material-experimental/mdc-table/BUILD.bazel
+++ b/src/material-experimental/mdc-table/BUILD.bazel
@@ -65,9 +65,8 @@ ng_test_library(
     deps = [
         ":mdc-table",
         "//src/cdk/table",
-        "//src/material/paginator",
+        "//src/material-experimental/mdc-paginator",
         "//src/material/sort",
-        "//src/material/table",
         "@npm//@angular/platform-browser",
         "@npm//rxjs",
     ],
@@ -75,7 +74,14 @@ ng_test_library(
 
 ng_web_test_suite(
     name = "unit_tests",
-    static_files = ["@npm//:node_modules/@material/data-table/dist/mdc.dataTable.js"],
+    static_files = [
+        "@npm//:node_modules/@material/textfield/dist/mdc.textfield.js",
+        "@npm//:node_modules/@material/line-ripple/dist/mdc.lineRipple.js",
+        "@npm//:node_modules/@material/notched-outline/dist/mdc.notchedOutline.js",
+        "@npm//:node_modules/@material/ripple/dist/mdc.ripple.js",
+        "@npm//:node_modules/@material/dom/dist/mdc.dom.js",
+        "@npm//:node_modules/@material/data-table/dist/mdc.dataTable.js",
+    ],
     deps = [
         ":table_tests_lib",
         "//src/material-experimental:mdc_require_config.js",
diff --git a/src/material-experimental/mdc-table/table.spec.ts b/src/material-experimental/mdc-table/table.spec.ts
index 3764a1f3544d..4404601162b8 100644
--- a/src/material-experimental/mdc-table/table.spec.ts
+++ b/src/material-experimental/mdc-table/table.spec.ts
@@ -7,12 +7,11 @@ import {
   TestBed,
   tick
 } from '@angular/core/testing';
-import {MatTable, MatTableModule} from './index';
+import {MatTable, MatTableDataSource, MatTableModule} from './index';
 import {DataSource} from '@angular/cdk/table';
 import {BehaviorSubject, Observable} from 'rxjs';
 import {MatSort, MatSortHeader, MatSortModule} from '@angular/material/sort';
-import {MatPaginator, MatPaginatorModule} from '@angular/material/paginator';
-import {MatTableDataSource} from '@angular/material/table';
+import {MatPaginator, MatPaginatorModule} from '@angular/material-experimental/mdc-paginator';
 import {NoopAnimationsModule} from '@angular/platform-browser/animations';
 
 describe('MDC-based MatTable', () => {
@@ -29,6 +28,7 @@ describe('MDC-based MatTable', () => {
         StickyTableApp,
         TableWithNgContainerRow,
         NestedTableApp,
+        MatFlexTableApp,
       ],
     }).compileComponents();
   }));
@@ -164,6 +164,14 @@ describe('MDC-based MatTable', () => {
       expect(tbody.textContent.trim()).toContain('No data');
     });
 
+    it('should set the content styling class on the tbody', () => {
+      let fixture = TestBed.createComponent(NativeHtmlTableApp);
+      fixture.detectChanges();
+
+      const tbodyElement = fixture.nativeElement.querySelector('tbody');
+      expect(tbodyElement.classList).toContain('mdc-data-table__content');
+    });
+
   });
 
   it('should render with MatTableDataSource and sort', () => {
@@ -213,6 +221,13 @@ describe('MDC-based MatTable', () => {
     }).not.toThrow();
   }));
 
+  it('should be able to render a flexbox-based table', () => {
+    expect(() => {
+      const fixture = TestBed.createComponent(MatFlexTableApp);
+      fixture.detectChanges();
+    }).not.toThrow();
+  });
+
   describe('with MatTableDataSource and sort/pagination/filter', () => {
     let tableElement: HTMLElement;
     let fixture: ComponentFixture<ArrayDataSourceMatTableApp>;
@@ -961,6 +976,45 @@ class TableWithNgContainerRow {
 }
 
 
+@Component({
+  template: `
+    <mat-table [dataSource]="dataSource">
+      <ng-container matColumnDef="column_a">
+        <mat-header-cell *matHeaderCellDef> Column A</mat-header-cell>
+        <mat-cell *matCellDef="let row"> {{row.a}}</mat-cell>
+        <mat-footer-cell *matFooterCellDef> Footer A</mat-footer-cell>
+      </ng-container>
+
+      <ng-container matColumnDef="column_b">
+        <mat-header-cell *matHeaderCellDef> Column B</mat-header-cell>
+        <mat-cell *matCellDef="let row"> {{row.b}}</mat-cell>
+        <mat-footer-cell *matFooterCellDef> Footer B</mat-footer-cell>
+      </ng-container>
+
+      <ng-container matColumnDef="column_c">
+        <mat-header-cell *matHeaderCellDef> Column C</mat-header-cell>
+        <mat-cell *matCellDef="let row"> {{row.c}}</mat-cell>
+        <mat-footer-cell *matFooterCellDef> Footer C</mat-footer-cell>
+      </ng-container>
+
+      <ng-container matColumnDef="special_column">
+        <mat-cell *matCellDef="let row"> fourth_row </mat-cell>
+      </ng-container>
+
+      <mat-header-row *matHeaderRowDef="columnsToRender"></mat-header-row>
+      <mat-row *matRowDef="let row; columns: columnsToRender"></mat-row>
+      <div *matNoDataRow>No data</div>
+      <mat-footer-row *matFooterRowDef="columnsToRender"></mat-footer-row>
+    </mat-table>
+  `
+})
+class MatFlexTableApp {
+  dataSource: FakeDataSource | null = new FakeDataSource();
+  columnsToRender = ['column_a', 'column_b', 'column_c'];
+  @ViewChild(MatTable) table: MatTable<TestData>;
+}
+
+
 function getElements(element: Element, query: string): Element[] {
   return [].slice.call(element.querySelectorAll(query));
 }
diff --git a/src/material-experimental/mdc-table/table.ts b/src/material-experimental/mdc-table/table.ts
index f942c7a5d838..cda399570668 100644
--- a/src/material-experimental/mdc-table/table.ts
+++ b/src/material-experimental/mdc-table/table.ts
@@ -43,10 +43,15 @@ export class MatTable<T> extends CdkTable<T> implements OnInit {
   /** Overrides the need to add position: sticky on every sticky cell element in `CdkTable`. */
   protected needsPositionStickyOnElement = false;
 
-  // After ngOnInit, the `CdkTable` has created and inserted the table sections (thead, tbody,
-  // tfoot). MDC requires the `mdc-data-table__content` class to be added to the body.
   ngOnInit() {
     super.ngOnInit();
-    this._elementRef.nativeElement.querySelector('tbody').classList.add('mdc-data-table__content');
+
+    // After ngOnInit, the `CdkTable` has created and inserted the table sections (thead, tbody,
+    // tfoot). MDC requires the `mdc-data-table__content` class to be added to the body. Note that
+    // this only applies to native tables, because we don't wrap the content of flexbox-based ones.
+    if (this._isNativeHtmlTable) {
+      const tbody = this._elementRef.nativeElement.querySelector('tbody');
+      tbody.classList.add('mdc-data-table__content');
+    }
   }
 }
diff --git a/tools/public_api_guard/cdk/table.d.ts b/tools/public_api_guard/cdk/table.d.ts
index 42a3b4bc8bc2..9898919616e1 100644
--- a/tools/public_api_guard/cdk/table.d.ts
+++ b/tools/public_api_guard/cdk/table.d.ts
@@ -199,6 +199,7 @@ export declare class CdkTable<T> implements AfterContentChecked, CollectionViewe
     protected readonly _elementRef: ElementRef;
     _footerRowOutlet: FooterRowOutlet;
     _headerRowOutlet: HeaderRowOutlet;
+    protected _isNativeHtmlTable: boolean;
     _multiTemplateDataRows: boolean;
     _noDataRow: CdkNoDataRow;
     _noDataRowOutlet: NoDataRowOutlet;