Skip to content

Commit

Permalink
fix(material/table): unsubscribe from source when disconnecting (angu…
Browse files Browse the repository at this point in the history
…lar#21338)

* Fixes that the table data source wasn't unsubscribing from sorting, pagination etc. when it is disconnected.
* Adds a bit of logic to properly support re-connecting the same date source again.
* Fixes up a couple of issues in the unit test setup which could cause state to leak between tests.

Fixes angular#21270.
  • Loading branch information
crisbeto authored Dec 17, 2020
1 parent 411b174 commit 6d36942
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 8 deletions.
18 changes: 16 additions & 2 deletions src/material/table/table-data-source.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {Component, ViewChild} from '@angular/core';

describe('MatTableDataSource', () => {
const dataSource = new MatTableDataSource();

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [MatSortModule, NoopAnimationsModule],
Expand All @@ -15,13 +13,16 @@ describe('MatTableDataSource', () => {
}));

describe('sort', () => {
let dataSource: MatTableDataSource<any>;
let fixture: ComponentFixture<MatSortApp>;
let sort: MatSort;

beforeEach(() => {
fixture = TestBed.createComponent(MatSortApp);
fixture.detectChanges();
dataSource = new MatTableDataSource();
sort = fixture.componentInstance.sort;
dataSource.sort = sort;
});

/** Test the data source's `sortData` function. */
Expand Down Expand Up @@ -51,6 +52,19 @@ describe('MatTableDataSource', () => {
it('should be able to correctly sort an array of strings and numbers', () => {
testSortWithValues([3, 'apples', 'bananas', 'cherries', 'lemons', 'strawberries']);
});

it('should unsubscribe from the re-render stream when disconnected', () => {
const spy = spyOn(dataSource._renderChangesSubscription!, 'unsubscribe');
dataSource.disconnect();
expect(spy).toHaveBeenCalledTimes(1);
});

it('should re-subscribe to the sort stream when re-connecting after being disconnected', () => {
dataSource.disconnect();
const spy = spyOn(fixture.componentInstance.sort.sortChange, 'subscribe');
dataSource.connect();
expect(spy).toHaveBeenCalledTimes(1);
});
});
});

Expand Down
19 changes: 14 additions & 5 deletions src/material/table/table-data-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class _MatTableDataSource<T, P extends Paginator> extends DataSource<T> {
* Subscription to the changes that should trigger an update to the table's rendered rows, such
* as filtering, sorting, pagination, or base data changes.
*/
_renderChangesSubscription = Subscription.EMPTY;
_renderChangesSubscription: Subscription|null = null;

/**
* The filtered set of data that has been matched by the filter string, or all the data if there
Expand Down Expand Up @@ -244,7 +244,7 @@ export class _MatTableDataSource<T, P extends Paginator> extends DataSource<T> {
const paginatedData = combineLatest([orderedData, pageChange])
.pipe(map(([data]) => this._pageData(data)));
// Watched for paged data changes and send the result to the table to render.
this._renderChangesSubscription.unsubscribe();
this._renderChangesSubscription?.unsubscribe();
this._renderChangesSubscription = paginatedData.subscribe(data => this._renderData.next(data));
}

Expand Down Expand Up @@ -321,13 +321,22 @@ export class _MatTableDataSource<T, P extends Paginator> extends DataSource<T> {
* Used by the MatTable. Called when it connects to the data source.
* @docs-private
*/
connect() { return this._renderData; }
connect() {
if (!this._renderChangesSubscription) {
this._updateChangeSubscription();
}

return this._renderData;
}

/**
* Used by the MatTable. Called when it is destroyed. No-op.
* Used by the MatTable. Called when it disconnects from the data source.
* @docs-private
*/
disconnect() { }
disconnect() {
this._renderChangesSubscription?.unsubscribe();
this._renderChangesSubscription = null;
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tools/public_api_guard/material/table.d.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export declare const _MAT_TEXT_COLUMN_TEMPLATE = "\n <ng-container matColumnDef>\n <th mat-header-cell *matHeaderCellDef [style.text-align]=\"justify\">\n {{headerText}}\n </th>\n <td mat-cell *matCellDef=\"let data\" [style.text-align]=\"justify\">\n {{dataAccessor(data, name)}}\n </td>\n </ng-container>\n";

export declare class _MatTableDataSource<T, P extends Paginator> extends DataSource<T> {
_renderChangesSubscription: Subscription;
_renderChangesSubscription: Subscription | null;
get data(): T[];
set data(data: T[]);
get filter(): string;
Expand Down

0 comments on commit 6d36942

Please sign in to comment.