Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(material/table): unsubscribe from source when disconnecting #21338

Merged
merged 1 commit into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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', () => {
crisbeto marked this conversation as resolved.
Show resolved Hide resolved
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