From 4d4ee182de3f306c5fcf49853ef6ac71c4000eea Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 2 Oct 2018 22:40:46 +0200 Subject: [PATCH] refactor(selection): avoid awkward null check for onChange event (#12348) Doesn't type the `SelectionModel.onChange` event to `Subject | null` anymore, in order to avoid having people null-check it on every consumption. Fixes #12347. --- src/cdk/collections/selection.spec.ts | 20 ++++++++----------- src/cdk/collections/selection.ts | 16 +++++++-------- .../dynamic-tree-demo/dynamic-database.ts | 2 +- src/lib/list/selection-list.ts | 2 +- src/lib/select/select.ts | 2 +- src/lib/tree/data-source/flat-data-source.ts | 2 +- .../tree-dynamic/tree-dynamic-example.ts | 2 +- 7 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/cdk/collections/selection.spec.ts b/src/cdk/collections/selection.spec.ts index 3f285f4d0d2..391c3b2bb2e 100644 --- a/src/cdk/collections/selection.spec.ts +++ b/src/cdk/collections/selection.spec.ts @@ -43,7 +43,7 @@ describe('SelectionModel', () => { it('should be able to select multiple options', () => { const onChangeSpy = jasmine.createSpy('onChange spy'); - model.changed!.subscribe(onChangeSpy); + model.changed.subscribe(onChangeSpy); model.select(1); model.select(2); @@ -56,7 +56,7 @@ describe('SelectionModel', () => { it('should be able to select multiple options at the same time', () => { const onChangeSpy = jasmine.createSpy('onChange spy'); - model.changed!.subscribe(onChangeSpy); + model.changed.subscribe(onChangeSpy); model.select(1, 2); expect(model.selected.length).toBe(2); @@ -97,7 +97,7 @@ describe('SelectionModel', () => { let model = new SelectionModel(); let spy = jasmine.createSpy('SelectionModel change event'); - model.onChange!.subscribe(spy); + model.onChange.subscribe(spy); model.select(1); let event = spy.calls.mostRecent().args[0]; @@ -112,7 +112,7 @@ describe('SelectionModel', () => { model.select(1); - model.changed!.subscribe(spy); + model.changed.subscribe(spy); model.select(2); @@ -130,7 +130,7 @@ describe('SelectionModel', () => { // Note: this assertion is only here to run the getter. expect(model.selected).toEqual([]); - model.onChange!.subscribe(() => spy(model.selected)); + model.onChange.subscribe(() => spy(model.selected)); model.select(1); expect(spy).toHaveBeenCalledWith([1]); @@ -144,7 +144,7 @@ describe('SelectionModel', () => { model = new SelectionModel(true); spy = jasmine.createSpy('SelectionModel change event'); - model.changed!.subscribe(spy); + model.changed.subscribe(spy); }); it('should emit an event when a value is selected', () => { @@ -167,7 +167,7 @@ describe('SelectionModel', () => { it('should not emit an event when preselecting values', () => { model = new SelectionModel(false, [1]); spy = jasmine.createSpy('SelectionModel initial change event'); - model.changed!.subscribe(spy); + model.changed.subscribe(spy); expect(spy).not.toHaveBeenCalled(); }); @@ -181,7 +181,7 @@ describe('SelectionModel', () => { model = new SelectionModel(true, [1, 2, 3]); spy = jasmine.createSpy('SelectionModel change event'); - model.changed!.subscribe(spy); + model.changed.subscribe(spy); }); it('should emit an event when a value is deselected', () => { @@ -217,10 +217,6 @@ describe('SelectionModel', () => { model = new SelectionModel(true, undefined, false); }); - it('should not have an onChange stream if change events are disabled', () => { - expect(model.changed).toBeFalsy(); - }); - it('should still update the select value', () => { model.select(1); expect(model.selected).toEqual([1]); diff --git a/src/cdk/collections/selection.ts b/src/cdk/collections/selection.ts index 3a9d62ef2b6..90ff94d1776 100644 --- a/src/cdk/collections/selection.ts +++ b/src/cdk/collections/selection.ts @@ -34,14 +34,14 @@ export class SelectionModel { } /** Event emitted when the value has changed. */ - changed: Subject> | null = this._emitChanges ? new Subject() : null; + changed: Subject> = new Subject(); /** * Event emitted when the value has changed. * @deprecated Use `changed` instead. * @breaking-change 8.0.0 To be changed to `changed` */ - onChange: Subject> | null = this.changed; + onChange: Subject> = this.changed; constructor( private _multiple = false, @@ -136,13 +136,11 @@ export class SelectionModel { this._selected = null; if (this._selectedToEmit.length || this._deselectedToEmit.length) { - if (this.changed) { - this.changed.next({ - source: this, - added: this._selectedToEmit, - removed: this._deselectedToEmit - }); - } + this.changed.next({ + source: this, + added: this._selectedToEmit, + removed: this._deselectedToEmit + }); this._deselectedToEmit = []; this._selectedToEmit = []; diff --git a/src/demo-app/tree/dynamic-tree-demo/dynamic-database.ts b/src/demo-app/tree/dynamic-tree-demo/dynamic-database.ts index c0cbfc5c248..29d7522578c 100644 --- a/src/demo-app/tree/dynamic-tree-demo/dynamic-database.ts +++ b/src/demo-app/tree/dynamic-tree-demo/dynamic-database.ts @@ -68,7 +68,7 @@ export class DynamicDataSource { private database: DynamicDatabase) {} connect(collectionViewer: CollectionViewer): Observable { - this.treeControl.expansionModel.onChange!.subscribe(change => { + this.treeControl.expansionModel.onChange.subscribe(change => { if (change.added || change.removed) { this.handleTreeControl(change); } diff --git a/src/lib/list/selection-list.ts b/src/lib/list/selection-list.ts index dd282d10b64..23993ba42ce 100644 --- a/src/lib/list/selection-list.ts +++ b/src/lib/list/selection-list.ts @@ -346,7 +346,7 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu } // Sync external changes to the model back to the options. - this._modelChanges = this.selectedOptions.onChange!.subscribe(event => { + this._modelChanges = this.selectedOptions.onChange.subscribe(event => { if (event.added) { for (let item of event.added) { item.selected = true; diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index b61700322b3..8cedce98523 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -527,7 +527,7 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, ngAfterContentInit() { this._initKeyManager(); - this._selectionModel.onChange!.pipe(takeUntil(this._destroy)).subscribe(event => { + this._selectionModel.onChange.pipe(takeUntil(this._destroy)).subscribe(event => { event.added.forEach(option => option.select()); event.removed.forEach(option => option.deselect()); }); diff --git a/src/lib/tree/data-source/flat-data-source.ts b/src/lib/tree/data-source/flat-data-source.ts index 29e9b1f8986..03463b437cc 100644 --- a/src/lib/tree/data-source/flat-data-source.ts +++ b/src/lib/tree/data-source/flat-data-source.ts @@ -146,7 +146,7 @@ export class MatTreeFlatDataSource extends DataSource { connect(collectionViewer: CollectionViewer): Observable { const changes = [ collectionViewer.viewChange, - this.treeControl.expansionModel.onChange!, + this.treeControl.expansionModel.onChange, this._flattenedData ]; return merge(...changes).pipe(map(() => { diff --git a/src/material-examples/tree-dynamic/tree-dynamic-example.ts b/src/material-examples/tree-dynamic/tree-dynamic-example.ts index 8f560382650..7933e19e6bd 100644 --- a/src/material-examples/tree-dynamic/tree-dynamic-example.ts +++ b/src/material-examples/tree-dynamic/tree-dynamic-example.ts @@ -59,7 +59,7 @@ export class DynamicDataSource { private database: DynamicDatabase) {} connect(collectionViewer: CollectionViewer): Observable { - this.treeControl.expansionModel.onChange!.subscribe(change => { + this.treeControl.expansionModel.onChange.subscribe(change => { if ((change as SelectionChange).added || (change as SelectionChange).removed) { this.handleTreeControl(change as SelectionChange);