Skip to content

Commit e488e3f

Browse files
crisbetojelbourn
authored andcommitted
feat: remove uses of rxjs patch operators (#5314)
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 146160c commit e488e3f

31 files changed

+504
-213
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
@@ -2,6 +2,7 @@ import {CollectionViewer, DataSource, MdPaginator} from '@angular/material';
22
import {Observable} from 'rxjs/Observable';
33
import {PeopleDatabase, UserData} from './people-database';
44
import {BehaviorSubject} from 'rxjs/BehaviorSubject';
5+
import 'rxjs/add/operator/map';
56

67
export class PersonDataSource extends DataSource<any> {
78
/** Data that should be displayed by the table. */

src/lib/autocomplete/autocomplete-trigger.ts

Lines changed: 32 additions & 33 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,23 +209,23 @@ 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.merge(
215-
Observable.fromEvent(this._document, 'click'),
216-
Observable.fromEvent(this._document, 'touchend')
217-
).filter((event: MouseEvent | TouchEvent) => {
218-
const clickTarget = event.target as HTMLElement;
219-
const inputContainer = this._inputContainer ?
220-
this._inputContainer._elementRef.nativeElement : null;
221-
222-
return this._panelOpen &&
223-
clickTarget !== this._element.nativeElement &&
224-
(!inputContainer || !inputContainer.contains(clickTarget)) &&
225-
(!!this._overlayRef && !this._overlayRef.overlayElement.contains(clickTarget));
226-
});
212+
if (!this._document) {
213+
return observableOf(null);
227214
}
228215

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

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

351350
/** 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', () => {
@@ -1335,10 +1335,13 @@ class NgIfAutocomplete {
13351335
@ViewChildren(MdOption) mdOptions: QueryList<MdOption>;
13361336

13371337
constructor() {
1338-
this.filteredOptions = this.optionCtrl.valueChanges.startWith(null).map((val) => {
1339-
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
1340-
: this.options.slice();
1341-
});
1338+
this.filteredOptions = RxChain.from(this.optionCtrl.valueChanges)
1339+
.call(startWith, null)
1340+
.call(map, (val: string) => {
1341+
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
1342+
: this.options.slice();
1343+
})
1344+
.result();
13421345
}
13431346
}
13441347

@@ -1444,10 +1447,13 @@ class AutocompleteWithNativeInput {
14441447
@ViewChildren(MdOption) mdOptions: QueryList<MdOption>;
14451448

14461449
constructor() {
1447-
this.filteredOptions = this.optionCtrl.valueChanges.startWith(null).map((val) => {
1448-
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
1449-
: this.options.slice();
1450-
});
1450+
this.filteredOptions = RxChain.from(this.optionCtrl.valueChanges)
1451+
.call(startWith, null)
1452+
.call(map, (val: string) => {
1453+
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
1454+
: this.options.slice();
1455+
})
1456+
.result();
14511457
}
14521458
}
14531459

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', () => {
@@ -449,7 +451,7 @@ class FakeDataSource extends DataSource<TestData> {
449451
connect(collectionViewer: CollectionViewer): Observable<TestData[]> {
450452
this.isConnected = true;
451453
const streams = [this._dataChange, collectionViewer.viewChange];
452-
return Observable.combineLatest(streams).map(([data]) => data);
454+
return map.call(combineLatest(streams), ([data]) => data);
453455
}
454456

455457
addData() {

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

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,9 @@ import {
3131
} from '@angular/core';
3232
import {CollectionViewer, DataSource} from './data-source';
3333
import {CdkCellOutlet, CdkCellOutletRowContext, CdkHeaderRowDef, CdkRowDef} from './row';
34-
import {Observable} from 'rxjs/Observable';
34+
import {merge} from 'rxjs/observable/merge';
35+
import {takeUntil} from '../rxjs/index';
3536
import {BehaviorSubject} from 'rxjs/BehaviorSubject';
36-
import 'rxjs/add/operator/let';
37-
import 'rxjs/add/operator/debounceTime';
38-
import 'rxjs/add/observable/combineLatest';
3937
import {Subscription} from 'rxjs/Subscription';
4038
import {Subject} from 'rxjs/Subject';
4139
import {CdkCellDef, CdkColumnDef, CdkHeaderCellDef} from './cell';
@@ -196,22 +194,20 @@ export class CdkTable<T> implements CollectionViewer {
196194

197195
// Re-render the rows if any of their columns change.
198196
// TODO(andrewseguin): Determine how to only re-render the rows that have their columns changed.
199-
Observable.merge(...this._rowDefinitions.map(rowDef => rowDef.columnsChange))
200-
.takeUntil(this._onDestroy)
201-
.subscribe(() => {
202-
// Reset the data to an empty array so that renderRowChanges will re-render all new rows.
203-
this._rowPlaceholder.viewContainer.clear();
204-
this._dataDiffer.diff([]);
205-
this._renderRowChanges();
206-
});
197+
const columnChangeEvents = this._rowDefinitions.map(rowDef => rowDef.columnsChange);
198+
199+
takeUntil.call(merge(...columnChangeEvents), this._onDestroy).subscribe(() => {
200+
// Reset the data to an empty array so that renderRowChanges will re-render all new rows.
201+
this._rowPlaceholder.viewContainer.clear();
202+
this._dataDiffer.diff([]);
203+
this._renderRowChanges();
204+
});
207205

208206
// Re-render the header row if the columns change
209-
this._headerDefinition.columnsChange
210-
.takeUntil(this._onDestroy)
211-
.subscribe(() => {
212-
this._headerRowPlaceholder.viewContainer.clear();
213-
this._renderHeaderRow();
214-
});
207+
takeUntil.call(this._headerDefinition.columnsChange, this._onDestroy).subscribe(() => {
208+
this._headerRowPlaceholder.viewContainer.clear();
209+
this._renderHeaderRow();
210+
});
215211
}
216212

217213
ngAfterViewInit() {
@@ -252,12 +248,11 @@ export class CdkTable<T> implements CollectionViewer {
252248

253249
/** Set up a subscription for the data provided by the data source. */
254250
private _observeRenderChanges() {
255-
this._renderChangeSubscription = this.dataSource.connect(this)
256-
.takeUntil(this._onDestroy)
257-
.subscribe(data => {
258-
this._data = data;
259-
this._renderRowChanges();
260-
});
251+
this._renderChangeSubscription = takeUntil.call(this.dataSource.connect(this), this._onDestroy)
252+
.subscribe(data => {
253+
this._data = data;
254+
this._renderRowChanges();
255+
});
261256
}
262257

263258
/**

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
}

0 commit comments

Comments
 (0)