Skip to content

Commit c5fed41

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 d75720b commit c5fed41

31 files changed

+504
-214
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: 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', () => {
@@ -337,7 +339,7 @@ class FakeDataSource extends DataSource<TestData> {
337339
connect(collectionViewer: CollectionViewer): Observable<TestData[]> {
338340
this.isConnected = true;
339341
const streams = [this._dataChange, collectionViewer.viewChange];
340-
return Observable.combineLatest(streams).map(([data]) => data);
342+
return map.call(combineLatest(streams), ([data]) => data);
341343
}
342344

343345
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
} from '@angular/core';
3030
import {CollectionViewer, DataSource} from './data-source';
3131
import {CdkCellOutlet, CdkCellOutletRowContext, CdkHeaderRowDef, CdkRowDef} from './row';
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
import {CdkCellDef, CdkColumnDef, CdkHeaderCellDef} from './cell';
@@ -180,22 +178,20 @@ export class CdkTable<T> implements CollectionViewer {
180178

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

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

201197
ngAfterViewInit() {
@@ -233,12 +229,11 @@ export class CdkTable<T> implements CollectionViewer {
233229

234230
/** Set up a subscription for the data provided by the data source. */
235231
private _observeRenderChanges() {
236-
this._renderChangeSubscription = this.dataSource.connect(this)
237-
.takeUntil(this._onDestroy)
238-
.subscribe(data => {
239-
this._data = data;
240-
this._renderRowChanges();
241-
});
232+
this._renderChangeSubscription = takeUntil.call(this.dataSource.connect(this), this._onDestroy)
233+
.subscribe(data => {
234+
this._data = data;
235+
this._renderRowChanges();
236+
});
242237
}
243238

244239
/**

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)