Skip to content

Commit 30445a2

Browse files
committed
feat: remove uses of rxjs patch operators
Refactors the entire codebase not to use the patch operators from RxJS, because they pollute the user's setup. Instead, opts into importing the operators directly and chaining them via the `RxChain` class. Fixes #2622.
1 parent 17cc4a1 commit 30445a2

31 files changed

+478
-211
lines changed

CODING_STANDARDS.md

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,34 @@ class ConfigBuilder {
124124
}
125125
```
126126

127+
#### RxJS
128+
When dealing with RxJS operators, import the operator functions directly (e.g.
129+
`import "rxjs/operator/map"`), as opposed to using the "patch" imports which pollute the user's
130+
global Observable object (e.g. `import "rxjs/add/operator/map"`):
131+
132+
```ts
133+
// NO
134+
import 'rxjs/add/operator/map';
135+
someObservable.map(...).subscribe(...);
136+
137+
// YES
138+
import {map} from 'rxjs/operator/map';
139+
map.call(someObservable, ...).subscribe(...);
140+
```
141+
142+
Note that this approach can be inflexible when dealing with long chains of operators. You can use
143+
the `RxChain` class to help with it:
144+
145+
```ts
146+
// Before
147+
someObservable.filter(...).map(...).do(...);
148+
149+
// After
150+
RxChain.from(someObservable).call(filter, ...).call(map, ...).call(do, ...).subscribe(...);
151+
```
152+
Note that not all operators are available via the `RxChain`. If the operator that you need isn't
153+
declared, you can add it to `/core/rxjs/rx-operators.ts`.
154+
127155
#### Access modifiers
128156
* Omit the `public` keyword as it is the default behavior.
129157
* Use `private` when appropriate and possible, prefixing the name with an underscore.
@@ -279,7 +307,7 @@ This makes it easier to override styles when necessary. For example, rather than
279307
```scss
280308
the-host-element {
281309
// ...
282-
310+
283311
.some-child-element {
284312
color: red;
285313
}

src/demo-app/autocomplete/autocomplete-demo.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {Component, ViewChild, ViewEncapsulation} from '@angular/core';
22
import {FormControl, NgModel} from '@angular/forms';
33
import 'rxjs/add/operator/startWith';
4+
import 'rxjs/add/operator/map';
45

56
@Component({
67
moduleId: module.id,

src/demo-app/data-table/person-data-source.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {CollectionViewer, DataSource} from '@angular/material';
22
import {Observable} from 'rxjs/Observable';
33
import {PeopleDatabase, UserData} from './people-database';
4+
import 'rxjs/add/operator/map';
45

56
export class PersonDataSource extends DataSource<any> {
67
_renderedData: any[] = [];

src/lib/autocomplete/autocomplete-trigger.ts

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,10 @@ import {ENTER, UP_ARROW, DOWN_ARROW, ESCAPE} from '../core/keyboard/keycodes';
3131
import {Directionality} from '../core/bidi/index';
3232
import {MdInputContainer} from '../input/input-container';
3333
import {Subscription} from 'rxjs/Subscription';
34-
import 'rxjs/add/observable/merge';
35-
import 'rxjs/add/observable/fromEvent';
36-
import 'rxjs/add/operator/filter';
37-
import 'rxjs/add/operator/switchMap';
38-
import 'rxjs/add/observable/of';
34+
import {merge} from 'rxjs/observable/merge';
35+
import {fromEvent} from 'rxjs/observable/fromEvent';
36+
import {of as observableOf} from 'rxjs/observable/of';
37+
import {RxChain, switchMap, first, filter} from '../core/rxjs/index';
3938

4039
/**
4140
* The following style constants are necessary to save here in order
@@ -187,7 +186,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
187186
* when an option is selected, on blur, and when TAB is pressed.
188187
*/
189188
get panelClosingActions(): Observable<MdOptionSelectionChange> {
190-
return Observable.merge(
189+
return merge(
191190
this.optionSelections,
192191
this.autocomplete._keyManager.tabOut,
193192
this._outsideClickStream
@@ -196,7 +195,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
196195

197196
/** Stream of autocomplete option selections. */
198197
get optionSelections(): Observable<MdOptionSelectionChange> {
199-
return Observable.merge(...this.autocomplete.options.map(option => option.onSelectionChange));
198+
return merge(...this.autocomplete.options.map(option => option.onSelectionChange));
200199
}
201200

202201
/** The currently active option, coerced to MdOption type. */
@@ -210,20 +209,20 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
210209

211210
/** Stream of clicks outside of the autocomplete panel. */
212211
private get _outsideClickStream(): Observable<any> {
213-
if (this._document) {
214-
return Observable.fromEvent(this._document, 'click').filter((event: MouseEvent) => {
215-
const clickTarget = event.target as HTMLElement;
216-
const inputContainer = this._inputContainer ?
217-
this._inputContainer._elementRef.nativeElement : null;
218-
219-
return this._panelOpen &&
220-
clickTarget !== this._element.nativeElement &&
221-
(!inputContainer || !inputContainer.contains(clickTarget)) &&
222-
(!!this._overlayRef && !this._overlayRef.overlayElement.contains(clickTarget));
223-
});
212+
if (!this._document) {
213+
return observableOf(null);
224214
}
225215

226-
return Observable.of(null);
216+
return filter.call(fromEvent(this._document, 'click'), (event: MouseEvent) => {
217+
const clickTarget = event.target as HTMLElement;
218+
const inputContainer = this._inputContainer ?
219+
this._inputContainer._elementRef.nativeElement : null;
220+
221+
return this._panelOpen &&
222+
clickTarget !== this._element.nativeElement &&
223+
(!inputContainer || !inputContainer.contains(clickTarget)) &&
224+
(!!this._overlayRef && !this._overlayRef.overlayElement.contains(clickTarget));
225+
});
227226
}
228227

229228
/**
@@ -332,17 +331,17 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
332331
*/
333332
private _subscribeToClosingActions(): void {
334333
// When the zone is stable initially, and when the option list changes...
335-
Observable.merge(this._zone.onStable.first(), this.autocomplete.options.changes)
336-
// create a new stream of panelClosingActions, replacing any previous streams
337-
// that were created, and flatten it so our stream only emits closing events...
338-
.switchMap(() => {
339-
this._resetPanel();
340-
return this.panelClosingActions;
341-
})
342-
// when the first closing event occurs...
343-
.first()
344-
// set the value, close the panel, and complete.
345-
.subscribe(event => this._setValueAndClose(event));
334+
RxChain.from(merge(first.call(this._zone.onStable), this.autocomplete.options.changes))
335+
// create a new stream of panelClosingActions, replacing any previous streams
336+
// that were created, and flatten it so our stream only emits closing events...
337+
.call(switchMap, () => {
338+
this._resetPanel();
339+
return this.panelClosingActions;
340+
})
341+
// when the first closing event occurs...
342+
.call(first)
343+
// set the value, close the panel, and complete.
344+
.subscribe(event => this._setValueAndClose(event));
346345
}
347346

348347
/** Destroys the autocomplete suggestion panel. */

src/lib/autocomplete/autocomplete.spec.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {dispatchFakeEvent} from '../core/testing/dispatch-events';
3030
import {createKeyboardEvent} from '../core/testing/event-objects';
3131
import {typeInElement} from '../core/testing/type-in-element';
3232
import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher';
33-
import 'rxjs/add/operator/map';
33+
import {RxChain, map, startWith, filter} from '../core/rxjs/index';
3434

3535

3636
describe('MdAutocomplete', () => {
@@ -1321,10 +1321,13 @@ class NgIfAutocomplete {
13211321
@ViewChildren(MdOption) mdOptions: QueryList<MdOption>;
13221322

13231323
constructor() {
1324-
this.filteredOptions = this.optionCtrl.valueChanges.startWith(null).map((val) => {
1325-
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
1326-
: this.options.slice();
1327-
});
1324+
this.filteredOptions = RxChain.from(this.optionCtrl.valueChanges)
1325+
.call(startWith, null)
1326+
.call(map, (val: string) => {
1327+
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
1328+
: this.options.slice();
1329+
})
1330+
.result();
13281331
}
13291332
}
13301333

@@ -1430,10 +1433,13 @@ class AutocompleteWithNativeInput {
14301433
@ViewChildren(MdOption) mdOptions: QueryList<MdOption>;
14311434

14321435
constructor() {
1433-
this.filteredOptions = this.optionCtrl.valueChanges.startWith(null).map((val) => {
1434-
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
1435-
: this.options.slice();
1436-
});
1436+
this.filteredOptions = RxChain.from(this.optionCtrl.valueChanges)
1437+
.call(startWith, null)
1438+
.call(map, (val: string) => {
1439+
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
1440+
: this.options.slice();
1441+
})
1442+
.result();
14371443
}
14381444
}
14391445

src/lib/core/a11y/focus-trap.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,10 @@ import {
1515
AfterContentInit,
1616
Injectable,
1717
} from '@angular/core';
18+
import {coerceBooleanProperty} from '@angular/cdk';
1819
import {InteractivityChecker} from './interactivity-checker';
1920
import {Platform} from '../platform/platform';
20-
import {coerceBooleanProperty} from '@angular/cdk';
21-
22-
import 'rxjs/add/operator/first';
21+
import {first} from '../rxjs/index';
2322

2423

2524
/**
@@ -232,7 +231,7 @@ export class FocusTrap {
232231
if (this._ngZone.isStable) {
233232
fn();
234233
} else {
235-
this._ngZone.onStable.first().subscribe(fn);
234+
first.call(this._ngZone.onStable).subscribe(fn);
236235
}
237236
}
238237
}

src/lib/core/a11y/list-key-manager.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {DOWN_ARROW, UP_ARROW, TAB} from '../keyboard/keycodes';
55
import {ListKeyManager} from './list-key-manager';
66
import {ActiveDescendantKeyManager} from './activedescendant-key-manager';
77
import {createKeyboardEvent} from '../testing/event-objects';
8+
import {first} from '../rxjs/index';
89

910

1011
class FakeFocusable {
@@ -189,7 +190,7 @@ describe('Key managers', () => {
189190

190191
it('should emit tabOut when the tab key is pressed', () => {
191192
let spy = jasmine.createSpy('tabOut spy');
192-
keyManager.tabOut.first().subscribe(spy);
193+
first.call(keyManager.tabOut).subscribe(spy);
193194
keyManager.onKeydown(fakeKeyEvents.tab);
194195

195196
expect(spy).toHaveBeenCalled();

src/lib/core/data-table/data-table.spec.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ import {async, ComponentFixture, TestBed} from '@angular/core/testing';
22
import {Component, ViewChild} from '@angular/core';
33
import {CdkTable} from './data-table';
44
import {CollectionViewer, DataSource} from './data-source';
5-
import {Observable} from 'rxjs/Observable';
65
import {BehaviorSubject} from 'rxjs/BehaviorSubject';
76
import {customMatchers} from '../testing/jasmine-matchers';
7+
import {Observable} from 'rxjs/Observable';
8+
import {combineLatest} from 'rxjs/observable/combineLatest';
9+
import {map} from '../rxjs/index';
810
import {CdkDataTableModule} from './index';
911

1012
describe('CdkTable', () => {
@@ -247,7 +249,7 @@ class FakeDataSource extends DataSource<TestData> {
247249
connect(collectionViewer: CollectionViewer): Observable<TestData[]> {
248250
this.isConnected = true;
249251
const streams = [this._dataChange, collectionViewer.viewChange];
250-
return Observable.combineLatest(streams).map(([data]) => data);
252+
return map.call(combineLatest(streams), ([data]) => data);
251253
}
252254

253255
addData() {

src/lib/core/data-table/data-table.ts

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,9 @@ import {
2929
import {CollectionViewer, DataSource} from './data-source';
3030
import {BaseRowDef, CdkCellOutlet, CdkHeaderRowDef, CdkRowDef} from './row';
3131
import {CdkCellDef, CdkColumnDef, CdkHeaderCellDef} from './cell';
32-
import {Observable} from 'rxjs/Observable';
32+
import {merge} from 'rxjs/observable/merge';
33+
import {takeUntil} from '../rxjs/index';
3334
import {BehaviorSubject} from 'rxjs/BehaviorSubject';
34-
import 'rxjs/add/operator/let';
35-
import 'rxjs/add/operator/debounceTime';
36-
import 'rxjs/add/observable/combineLatest';
3735
import {Subscription} from 'rxjs/Subscription';
3836
import {Subject} from 'rxjs/Subject';
3937

@@ -179,22 +177,20 @@ export class CdkTable<T> implements CollectionViewer {
179177

180178
// Re-render the rows if any of their columns change.
181179
// TODO(andrewseguin): Determine how to only re-render the rows that have their columns changed.
182-
Observable.merge(...this._rowDefinitions.map(rowDef => rowDef.columnsChange))
183-
.takeUntil(this._onDestroy)
184-
.subscribe(() => {
185-
// Reset the data to an empty array so that renderRowChanges will re-render all new rows.
186-
this._rowPlaceholder.viewContainer.clear();
187-
this._dataDiffer.diff([]);
188-
this._renderRowChanges();
189-
});
180+
const columnChangeEvents = this._rowDefinitions.map(rowDef => rowDef.columnsChange);
181+
182+
takeUntil.call(merge(...columnChangeEvents), this._onDestroy).subscribe(() => {
183+
// Reset the data to an empty array so that renderRowChanges will re-render all new rows.
184+
this._rowPlaceholder.viewContainer.clear();
185+
this._dataDiffer.diff([]);
186+
this._renderRowChanges();
187+
});
190188

191189
// Re-render the header row if the columns change
192-
this._headerDefinition.columnsChange
193-
.takeUntil(this._onDestroy)
194-
.subscribe(() => {
195-
this._headerRowPlaceholder.viewContainer.clear();
196-
this._renderHeaderRow();
197-
});
190+
takeUntil.call(this._headerDefinition.columnsChange, this._onDestroy).subscribe(() => {
191+
this._headerRowPlaceholder.viewContainer.clear();
192+
this._renderHeaderRow();
193+
});
198194
}
199195

200196
ngAfterViewInit() {
@@ -231,12 +227,11 @@ export class CdkTable<T> implements CollectionViewer {
231227

232228
/** Set up a subscription for the data provided by the data source. */
233229
private _observeRenderChanges() {
234-
this._renderChangeSubscription = this.dataSource.connect(this)
235-
.takeUntil(this._onDestroy)
236-
.subscribe(data => {
237-
this._data = data;
238-
this._renderRowChanges();
239-
});
230+
this._renderChangeSubscription = takeUntil.call(this.dataSource.connect(this), this._onDestroy)
231+
.subscribe(data => {
232+
this._data = data;
233+
this._renderRowChanges();
234+
});
240235
}
241236

242237
/**

src/lib/core/observe-content/observe-content.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
Injectable,
1919
} from '@angular/core';
2020
import {Subject} from 'rxjs/Subject';
21-
import 'rxjs/add/operator/debounceTime';
21+
import {RxChain, debounceTime} from '../rxjs/index';
2222

2323
/**
2424
* Factory that creates a new MutationObserver and allows us to stub it out in unit tests.
@@ -56,9 +56,9 @@ export class ObserveContent implements AfterContentInit, OnDestroy {
5656

5757
ngAfterContentInit() {
5858
if (this.debounce > 0) {
59-
this._debouncer
60-
.debounceTime(this.debounce)
61-
.subscribe(mutations => this.event.emit(mutations));
59+
RxChain.from(this._debouncer)
60+
.call(debounceTime, this.debounce)
61+
.subscribe((mutations: MutationRecord[]) => this.event.emit(mutations));
6262
} else {
6363
this._debouncer.subscribe(mutations => this.event.emit(mutations));
6464
}

src/lib/core/overlay/scroll/scroll-dispatcher.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,10 @@ import {ElementRef, Injectable, NgZone, Optional, SkipSelf} from '@angular/core'
1010
import {Platform} from '../../platform/index';
1111
import {Scrollable} from './scrollable';
1212
import {Subject} from 'rxjs/Subject';
13-
import {Observable} from 'rxjs/Observable';
1413
import {Subscription} from 'rxjs/Subscription';
15-
import 'rxjs/add/observable/fromEvent';
16-
import 'rxjs/add/observable/merge';
17-
import 'rxjs/add/operator/auditTime';
14+
import {fromEvent} from 'rxjs/observable/fromEvent';
15+
import {merge} from 'rxjs/observable/merge';
16+
import {auditTime} from '../../rxjs/index';
1817

1918

2019
/** Time in ms to throttle the scrolling events by default. */
@@ -81,16 +80,16 @@ export class ScrollDispatcher {
8180
// In the case of a 0ms delay, use an observable without auditTime
8281
// since it does add a perceptible delay in processing overhead.
8382
let observable = auditTimeInMs > 0 ?
84-
this._scrolled.asObservable().auditTime(auditTimeInMs) :
83+
auditTime.call(this._scrolled.asObservable(), auditTimeInMs) :
8584
this._scrolled.asObservable();
8685

8786
this._scrolledCount++;
8887

8988
if (!this._globalSubscription) {
9089
this._globalSubscription = this._ngZone.runOutsideAngular(() => {
91-
return Observable.merge(
92-
Observable.fromEvent(window.document, 'scroll'),
93-
Observable.fromEvent(window, 'resize')
90+
return merge(
91+
fromEvent(window.document, 'scroll'),
92+
fromEvent(window, 'resize')
9493
).subscribe(() => this._notify());
9594
});
9695
}

src/lib/core/overlay/scroll/scrollable.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {Directive, ElementRef, OnInit, OnDestroy, NgZone, Renderer2} from '@angu
1010
import {Observable} from 'rxjs/Observable';
1111
import {Subject} from 'rxjs/Subject';
1212
import {ScrollDispatcher} from './scroll-dispatcher';
13-
import 'rxjs/add/observable/fromEvent';
1413

1514

1615
/**

src/lib/core/rxjs/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export * from './rx-chain';
2+
export * from './rx-operators';

0 commit comments

Comments
 (0)