Skip to content

Commit

Permalink
feat: remove uses of rxjs patch operators (#5314)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto authored and jelbourn committed Jun 23, 2017
1 parent 146160c commit e488e3f
Show file tree
Hide file tree
Showing 31 changed files with 504 additions and 213 deletions.
30 changes: 29 additions & 1 deletion CODING_STANDARDS.md
Expand Up @@ -124,6 +124,34 @@ class ConfigBuilder {
}
```

#### RxJS
When dealing with RxJS operators, import the operator functions directly (e.g.
`import "rxjs/operator/map"`), as opposed to using the "patch" imports which pollute the user's
global Observable object (e.g. `import "rxjs/add/operator/map"`):

```ts
// NO
import 'rxjs/add/operator/map';
someObservable.map(...).subscribe(...);

// YES
import {map} from 'rxjs/operator/map';
map.call(someObservable, ...).subscribe(...);
```

Note that this approach can be inflexible when dealing with long chains of operators. You can use
the `RxChain` class to help with it:

```ts
// Before
someObservable.filter(...).map(...).do(...);

// After
RxChain.from(someObservable).call(filter, ...).call(map, ...).call(do, ...).subscribe(...);
```
Note that not all operators are available via the `RxChain`. If the operator that you need isn't
declared, you can add it to `/core/rxjs/rx-operators.ts`.

#### Access modifiers
* Omit the `public` keyword as it is the default behavior.
* Use `private` when appropriate and possible, prefixing the name with an underscore.
Expand Down Expand Up @@ -279,7 +307,7 @@ This makes it easier to override styles when necessary. For example, rather than
```scss
the-host-element {
// ...

.some-child-element {
color: red;
}
Expand Down
1 change: 1 addition & 0 deletions src/demo-app/autocomplete/autocomplete-demo.ts
@@ -1,6 +1,7 @@
import {Component, ViewChild, ViewEncapsulation} from '@angular/core';
import {FormControl, NgModel} from '@angular/forms';
import 'rxjs/add/operator/startWith';
import 'rxjs/add/operator/map';

@Component({
moduleId: module.id,
Expand Down
1 change: 1 addition & 0 deletions src/demo-app/data-table/person-data-source.ts
Expand Up @@ -2,6 +2,7 @@ import {CollectionViewer, DataSource, MdPaginator} from '@angular/material';
import {Observable} from 'rxjs/Observable';
import {PeopleDatabase, UserData} from './people-database';
import {BehaviorSubject} from 'rxjs/BehaviorSubject';
import 'rxjs/add/operator/map';

export class PersonDataSource extends DataSource<any> {
/** Data that should be displayed by the table. */
Expand Down
65 changes: 32 additions & 33 deletions src/lib/autocomplete/autocomplete-trigger.ts
Expand Up @@ -31,11 +31,10 @@ import {ENTER, UP_ARROW, DOWN_ARROW, ESCAPE} from '../core/keyboard/keycodes';
import {Directionality} from '../core/bidi/index';
import {MdInputContainer} from '../input/input-container';
import {Subscription} from 'rxjs/Subscription';
import 'rxjs/add/observable/merge';
import 'rxjs/add/observable/fromEvent';
import 'rxjs/add/operator/filter';
import 'rxjs/add/operator/switchMap';
import 'rxjs/add/observable/of';
import {merge} from 'rxjs/observable/merge';
import {fromEvent} from 'rxjs/observable/fromEvent';
import {of as observableOf} from 'rxjs/observable/of';
import {RxChain, switchMap, first, filter} from '../core/rxjs/index';

/**
* The following style constants are necessary to save here in order
Expand Down Expand Up @@ -187,7 +186,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
* when an option is selected, on blur, and when TAB is pressed.
*/
get panelClosingActions(): Observable<MdOptionSelectionChange> {
return Observable.merge(
return merge(
this.optionSelections,
this.autocomplete._keyManager.tabOut,
this._outsideClickStream
Expand All @@ -196,7 +195,7 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {

/** Stream of autocomplete option selections. */
get optionSelections(): Observable<MdOptionSelectionChange> {
return Observable.merge(...this.autocomplete.options.map(option => option.onSelectionChange));
return merge(...this.autocomplete.options.map(option => option.onSelectionChange));
}

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

/** Stream of clicks outside of the autocomplete panel. */
private get _outsideClickStream(): Observable<any> {
if (this._document) {
return Observable.merge(
Observable.fromEvent(this._document, 'click'),
Observable.fromEvent(this._document, 'touchend')
).filter((event: MouseEvent | TouchEvent) => {
const clickTarget = event.target as HTMLElement;
const inputContainer = this._inputContainer ?
this._inputContainer._elementRef.nativeElement : null;

return this._panelOpen &&
clickTarget !== this._element.nativeElement &&
(!inputContainer || !inputContainer.contains(clickTarget)) &&
(!!this._overlayRef && !this._overlayRef.overlayElement.contains(clickTarget));
});
if (!this._document) {
return observableOf(null);
}

return Observable.of(null);
return RxChain.from(merge(
fromEvent(this._document, 'click'),
fromEvent(this._document, 'touchend')
)).call(filter, (event: MouseEvent | TouchEvent) => {
const clickTarget = event.target as HTMLElement;
const inputContainer = this._inputContainer ?
this._inputContainer._elementRef.nativeElement : null;

return this._panelOpen &&
clickTarget !== this._element.nativeElement &&
(!inputContainer || !inputContainer.contains(clickTarget)) &&
(!!this._overlayRef && !this._overlayRef.overlayElement.contains(clickTarget));
}).result();
}

/**
Expand Down Expand Up @@ -335,17 +334,17 @@ export class MdAutocompleteTrigger implements ControlValueAccessor, OnDestroy {
*/
private _subscribeToClosingActions(): void {
// When the zone is stable initially, and when the option list changes...
Observable.merge(this._zone.onStable.first(), this.autocomplete.options.changes)
// create a new stream of panelClosingActions, replacing any previous streams
// that were created, and flatten it so our stream only emits closing events...
.switchMap(() => {
this._resetPanel();
return this.panelClosingActions;
})
// when the first closing event occurs...
.first()
// set the value, close the panel, and complete.
.subscribe(event => this._setValueAndClose(event));
RxChain.from(merge(first.call(this._zone.onStable), this.autocomplete.options.changes))
// create a new stream of panelClosingActions, replacing any previous streams
// that were created, and flatten it so our stream only emits closing events...
.call(switchMap, () => {
this._resetPanel();
return this.panelClosingActions;
})
// when the first closing event occurs...
.call(first)
// set the value, close the panel, and complete.
.subscribe(event => this._setValueAndClose(event));
}

/** Destroys the autocomplete suggestion panel. */
Expand Down
24 changes: 15 additions & 9 deletions src/lib/autocomplete/autocomplete.spec.ts
Expand Up @@ -30,7 +30,7 @@ import {dispatchFakeEvent} from '../core/testing/dispatch-events';
import {createKeyboardEvent} from '../core/testing/event-objects';
import {typeInElement} from '../core/testing/type-in-element';
import {ScrollDispatcher} from '../core/overlay/scroll/scroll-dispatcher';
import 'rxjs/add/operator/map';
import {RxChain, map, startWith, filter} from '../core/rxjs/index';


describe('MdAutocomplete', () => {
Expand Down Expand Up @@ -1335,10 +1335,13 @@ class NgIfAutocomplete {
@ViewChildren(MdOption) mdOptions: QueryList<MdOption>;

constructor() {
this.filteredOptions = this.optionCtrl.valueChanges.startWith(null).map((val) => {
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
: this.options.slice();
});
this.filteredOptions = RxChain.from(this.optionCtrl.valueChanges)
.call(startWith, null)
.call(map, (val: string) => {
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
: this.options.slice();
})
.result();
}
}

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

constructor() {
this.filteredOptions = this.optionCtrl.valueChanges.startWith(null).map((val) => {
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
: this.options.slice();
});
this.filteredOptions = RxChain.from(this.optionCtrl.valueChanges)
.call(startWith, null)
.call(map, (val: string) => {
return val ? this.options.filter(option => new RegExp(val, 'gi').test(option))
: this.options.slice();
})
.result();
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/lib/core/a11y/focus-trap.ts
Expand Up @@ -15,11 +15,10 @@ import {
AfterContentInit,
Injectable,
} from '@angular/core';
import {coerceBooleanProperty} from '@angular/cdk';
import {InteractivityChecker} from './interactivity-checker';
import {Platform} from '../platform/platform';
import {coerceBooleanProperty} from '@angular/cdk';

import 'rxjs/add/operator/first';
import {first} from '../rxjs/index';


/**
Expand Down Expand Up @@ -232,7 +231,7 @@ export class FocusTrap {
if (this._ngZone.isStable) {
fn();
} else {
this._ngZone.onStable.first().subscribe(fn);
first.call(this._ngZone.onStable).subscribe(fn);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/lib/core/a11y/list-key-manager.spec.ts
Expand Up @@ -5,6 +5,7 @@ import {DOWN_ARROW, UP_ARROW, TAB} from '../keyboard/keycodes';
import {ListKeyManager} from './list-key-manager';
import {ActiveDescendantKeyManager} from './activedescendant-key-manager';
import {createKeyboardEvent} from '../testing/event-objects';
import {first} from '../rxjs/index';


class FakeFocusable {
Expand Down Expand Up @@ -189,7 +190,7 @@ describe('Key managers', () => {

it('should emit tabOut when the tab key is pressed', () => {
let spy = jasmine.createSpy('tabOut spy');
keyManager.tabOut.first().subscribe(spy);
first.call(keyManager.tabOut).subscribe(spy);
keyManager.onKeydown(fakeKeyEvents.tab);

expect(spy).toHaveBeenCalled();
Expand Down
6 changes: 4 additions & 2 deletions src/lib/core/data-table/data-table.spec.ts
Expand Up @@ -2,9 +2,11 @@ import {async, ComponentFixture, TestBed} from '@angular/core/testing';
import {Component, ViewChild} from '@angular/core';
import {CdkTable} from './data-table';
import {CollectionViewer, DataSource} from './data-source';
import {Observable} from 'rxjs/Observable';
import {BehaviorSubject} from 'rxjs/BehaviorSubject';
import {customMatchers} from '../testing/jasmine-matchers';
import {Observable} from 'rxjs/Observable';
import {combineLatest} from 'rxjs/observable/combineLatest';
import {map} from '../rxjs/index';
import {CdkDataTableModule} from './index';

describe('CdkTable', () => {
Expand Down Expand Up @@ -449,7 +451,7 @@ class FakeDataSource extends DataSource<TestData> {
connect(collectionViewer: CollectionViewer): Observable<TestData[]> {
this.isConnected = true;
const streams = [this._dataChange, collectionViewer.viewChange];
return Observable.combineLatest(streams).map(([data]) => data);
return map.call(combineLatest(streams), ([data]) => data);
}

addData() {
Expand Down
43 changes: 19 additions & 24 deletions src/lib/core/data-table/data-table.ts
Expand Up @@ -31,11 +31,9 @@ import {
} from '@angular/core';
import {CollectionViewer, DataSource} from './data-source';
import {CdkCellOutlet, CdkCellOutletRowContext, CdkHeaderRowDef, CdkRowDef} from './row';
import {Observable} from 'rxjs/Observable';
import {merge} from 'rxjs/observable/merge';
import {takeUntil} from '../rxjs/index';
import {BehaviorSubject} from 'rxjs/BehaviorSubject';
import 'rxjs/add/operator/let';
import 'rxjs/add/operator/debounceTime';
import 'rxjs/add/observable/combineLatest';
import {Subscription} from 'rxjs/Subscription';
import {Subject} from 'rxjs/Subject';
import {CdkCellDef, CdkColumnDef, CdkHeaderCellDef} from './cell';
Expand Down Expand Up @@ -196,22 +194,20 @@ export class CdkTable<T> implements CollectionViewer {

// Re-render the rows if any of their columns change.
// TODO(andrewseguin): Determine how to only re-render the rows that have their columns changed.
Observable.merge(...this._rowDefinitions.map(rowDef => rowDef.columnsChange))
.takeUntil(this._onDestroy)
.subscribe(() => {
// Reset the data to an empty array so that renderRowChanges will re-render all new rows.
this._rowPlaceholder.viewContainer.clear();
this._dataDiffer.diff([]);
this._renderRowChanges();
});
const columnChangeEvents = this._rowDefinitions.map(rowDef => rowDef.columnsChange);

takeUntil.call(merge(...columnChangeEvents), this._onDestroy).subscribe(() => {
// Reset the data to an empty array so that renderRowChanges will re-render all new rows.
this._rowPlaceholder.viewContainer.clear();
this._dataDiffer.diff([]);
this._renderRowChanges();
});

// Re-render the header row if the columns change
this._headerDefinition.columnsChange
.takeUntil(this._onDestroy)
.subscribe(() => {
this._headerRowPlaceholder.viewContainer.clear();
this._renderHeaderRow();
});
takeUntil.call(this._headerDefinition.columnsChange, this._onDestroy).subscribe(() => {
this._headerRowPlaceholder.viewContainer.clear();
this._renderHeaderRow();
});
}

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

/** Set up a subscription for the data provided by the data source. */
private _observeRenderChanges() {
this._renderChangeSubscription = this.dataSource.connect(this)
.takeUntil(this._onDestroy)
.subscribe(data => {
this._data = data;
this._renderRowChanges();
});
this._renderChangeSubscription = takeUntil.call(this.dataSource.connect(this), this._onDestroy)
.subscribe(data => {
this._data = data;
this._renderRowChanges();
});
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/lib/core/observe-content/observe-content.ts
Expand Up @@ -18,7 +18,7 @@ import {
Injectable,
} from '@angular/core';
import {Subject} from 'rxjs/Subject';
import 'rxjs/add/operator/debounceTime';
import {RxChain, debounceTime} from '../rxjs/index';

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

ngAfterContentInit() {
if (this.debounce > 0) {
this._debouncer
.debounceTime(this.debounce)
.subscribe(mutations => this.event.emit(mutations));
RxChain.from(this._debouncer)
.call(debounceTime, this.debounce)
.subscribe((mutations: MutationRecord[]) => this.event.emit(mutations));
} else {
this._debouncer.subscribe(mutations => this.event.emit(mutations));
}
Expand Down
15 changes: 7 additions & 8 deletions src/lib/core/overlay/scroll/scroll-dispatcher.ts
Expand Up @@ -10,11 +10,10 @@ import {ElementRef, Injectable, NgZone, Optional, SkipSelf} from '@angular/core'
import {Platform} from '../../platform/index';
import {Scrollable} from './scrollable';
import {Subject} from 'rxjs/Subject';
import {Observable} from 'rxjs/Observable';
import {Subscription} from 'rxjs/Subscription';
import 'rxjs/add/observable/fromEvent';
import 'rxjs/add/observable/merge';
import 'rxjs/add/operator/auditTime';
import {fromEvent} from 'rxjs/observable/fromEvent';
import {merge} from 'rxjs/observable/merge';
import {auditTime} from '../../rxjs/index';


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

this._scrolledCount++;

if (!this._globalSubscription) {
this._globalSubscription = this._ngZone.runOutsideAngular(() => {
return Observable.merge(
Observable.fromEvent(window.document, 'scroll'),
Observable.fromEvent(window, 'resize')
return merge(
fromEvent(window.document, 'scroll'),
fromEvent(window, 'resize')
).subscribe(() => this._notify());
});
}
Expand Down

0 comments on commit e488e3f

Please sign in to comment.