Skip to content

Commit 1d7f70b

Browse files
committed
refactor(Observable): get rid of side-effects from rxjs throughout src
Also removed side-effects from tests to verify that code under test wouldn't accidentally rely on a patched operator.
1 parent ec58e05 commit 1d7f70b

11 files changed

+78
-86
lines changed

rollup-globals.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
export default (mod) => {
22
if (mod === 'rxjs') return 'Rx';
33
if (mod.indexOf('rxjs/operator') === 0) return `Rx.Observable.prototype`;
4+
if (mod.indexOf('rxjs/observable') === 0) return `Rx.Observable`;
45
if (mod === 'rxjs/scheduler/queue') return 'Rx.Scheduler';
56
if (mod.indexOf('rxjs/') === 0) return 'Rx';
67

src/angularfire2.spec.ts

-4
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ import {
1717
} from './angularfire2';
1818
import { Subscription } from 'rxjs/Subscription';
1919
import { COMMON_CONFIG, ANON_AUTH_CONFIG } from './test-config';
20-
import 'rxjs/add/operator/toPromise';
21-
import 'rxjs/add/operator/take';
22-
import 'rxjs/add/operator/do';
23-
import 'rxjs/add/operator/delay';
2420

2521
describe('angularfire', () => {
2622
var subscription:Subscription;

src/auth/auth.spec.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
TestBed,
77
inject
88
} from '@angular/core/testing';
9-
import 'rxjs/add/operator/do';
9+
import { _do } from 'rxjs/operator/do';
1010

1111
import {
1212
defaultFirebase,
@@ -97,9 +97,9 @@ describe('Zones', () => {
9797
});
9898
ngZone.run(() => {
9999
var afAuth = new AngularFireAuth(new FirebaseSdkAuthBackend(app), window.location);
100-
afAuth
101-
.take(1)
102-
.do(_ => {
100+
var authObs = afAuth.take(1);
101+
102+
_do.call(authObs, _ => {
103103
expect(Zone.current.name).toBe('ngZone');
104104
})
105105
.subscribe(() => {

src/auth/auth.ts

+22-24
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,9 @@ import {
1414
FirebaseAuthState,
1515
stripProviderId
1616
} from './auth_backend';
17-
import 'rxjs/add/operator/mergeMap';
18-
import 'rxjs/add/operator/take';
19-
import 'rxjs/add/operator/concat';
20-
import 'rxjs/add/operator/skip';
21-
import 'rxjs/add/observable/of';
17+
import { mergeMap } from 'rxjs/operator/mergeMap';
18+
import { of as observableOf } from 'rxjs/observable/of';
19+
import { map } from 'rxjs/operator/map';
2220

2321
const kBufferSize = 1;
2422

@@ -35,26 +33,26 @@ export class AngularFireAuth extends ReplaySubject<FirebaseAuthState> {
3533
super(kBufferSize);
3634

3735
let firstPass = true;
38-
this._authBackend.onAuth()
39-
.mergeMap((authState: FirebaseAuthState) => {
40-
if (firstPass) {
41-
firstPass = false;
42-
if(['http:', 'https:'].indexOf(loc.protocol) > -1) {
43-
// Only call getRedirectResult() in a browser
44-
return this._authBackend.getRedirectResult()
45-
.map((userCredential: firebase.auth.UserCredential) => {
46-
if (userCredential && userCredential.credential) {
47-
authState = attachCredentialToAuthState(authState, userCredential.credential, userCredential.credential.provider);
48-
this._credentialCache[userCredential.credential.provider] = userCredential.credential;
49-
}
50-
return authState;
51-
});
52-
}
53-
36+
let onAuth = this._authBackend.onAuth();
37+
38+
mergeMap.call(onAuth, (authState: FirebaseAuthState) => {
39+
if (firstPass) {
40+
firstPass = false;
41+
if(['http:', 'https:'].indexOf(loc.protocol) > -1) {
42+
// Only call getRedirectResult() in a browser
43+
return map.call(this._authBackend.getRedirectResult(), (userCredential: firebase.auth.UserCredential) => {
44+
if (userCredential && userCredential.credential) {
45+
authState = attachCredentialToAuthState(authState, userCredential.credential, userCredential.credential.provider);
46+
this._credentialCache[userCredential.credential.provider] = userCredential.credential;
47+
}
48+
return authState;
49+
});
5450
}
55-
return Observable.of(authState);
56-
})
57-
.subscribe((authData: FirebaseAuthState) => this._emitAuthData(authData));
51+
52+
}
53+
return observableOf(authState);
54+
})
55+
.subscribe((authData: FirebaseAuthState) => this._emitAuthData(authData));
5856
}
5957

6058
public login(config?: AuthConfiguration): firebase.Promise<FirebaseAuthState>;

src/auth/firebase_sdk_auth_backend.ts

+10-9
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ const {
2020
TwitterAuthProvider
2121
} = auth;
2222

23-
import 'rxjs/add/operator/map';
24-
import 'rxjs/add/observable/fromPromise';
25-
import 'rxjs/add/operator/observeOn';
23+
import { map } from 'rxjs/operator/map';
24+
import { fromPromise } from 'rxjs/observable/fromPromise';
25+
import { observeOn } from 'rxjs/operator/observeOn';
2626

2727
@Injectable()
2828
export class FirebaseSdkAuthBackend extends AuthBackend {
@@ -43,20 +43,21 @@ export class FirebaseSdkAuthBackend extends AuthBackend {
4343
}
4444

4545
onAuth(): Observable<FirebaseAuthState> {
46-
return Observable.create((observer: Observer<FirebaseAuthState>) => {
46+
let stateChange = Observable.create((observer: Observer<FirebaseAuthState>) => {
4747
return this._fbAuth.onAuthStateChanged(observer);
48-
})
49-
.map((user: firebase.User) => {
48+
});
49+
let authState = map.call(stateChange, (user: firebase.User) => {
5050
if (!user) return null;
5151
return authDataToAuthState(user, user.providerData[0]);
52-
})
52+
});
53+
5354
/**
5455
* TODO: since the auth service automatically subscribes to this before
5556
* any user, it will run in the Angular zone, instead of the subscription
5657
* zone. The auth service should be refactored to capture the subscription
5758
* zone and not use a ReplaySubject.
5859
**/
59-
.observeOn(new ZoneScheduler(Zone.current));
60+
return observeOn.call(authState, new ZoneScheduler(Zone.current));
6061
}
6162

6263
unauth(): void {
@@ -101,7 +102,7 @@ export class FirebaseSdkAuthBackend extends AuthBackend {
101102
}
102103

103104
getRedirectResult(): Observable<firebase.auth.UserCredential> {
104-
return Observable.fromPromise(castPromise<firebase.auth.UserCredential>(this._fbAuth.getRedirectResult()));
105+
return fromPromise(castPromise<firebase.auth.UserCredential>(this._fbAuth.getRedirectResult()));
105106
}
106107

107108
private _enumToAuthProvider(providerId: AuthProviders): any {

src/database/firebase_list_factory.spec.ts

+26-28
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,16 @@ import * as utils from '../utils';
2424
import { Query, AFUnwrappedDataSnapshot } from '../interfaces';
2525
import { Subscription, Observable, Subject } from 'rxjs';
2626
import { COMMON_CONFIG, ANON_AUTH_CONFIG } from '../test-config';
27-
import 'rxjs/add/operator/do';
28-
import 'rxjs/add/operator/skip';
29-
import 'rxjs/add/operator/take';
27+
import { _do } from 'rxjs/operator/do';
28+
import { skip } from 'rxjs/operator/skip';
29+
import { take } from 'rxjs/operator/take';
30+
import { toPromise } from 'rxjs/operator/toPromise';
3031

3132
const rootDatabaseUrl = COMMON_CONFIG.databaseURL;
3233

3334
function queryTest(observable: Observable<any>, subject: Subject<any>, done: any) {
3435
let nexted = false;
35-
observable
36-
.take(2)
36+
skipAndTake(observable, 2)
3737
.subscribe(val => {
3838
if (!nexted) {
3939
subject.next('2');
@@ -378,7 +378,7 @@ describe('FirebaseListFactory', () => {
378378

379379
it('should emit only when the initial data set has been loaded', (done: any) => {
380380
(<any>questions)._ref.set([{ initial1: true }, { initial2: true }, { initial3: true }, { initial4: true }])
381-
.then(() => questions.take(1).toPromise())
381+
.then(() => toPromise.call(skipAndTake(questions, 1)))
382382
.then((val: any[]) => {
383383
expect(val.length).toBe(4);
384384
})
@@ -389,10 +389,8 @@ describe('FirebaseListFactory', () => {
389389

390390

391391
it('should emit a new value when a child moves', (done: any) => {
392-
subscription = questions
393-
.skip(2)
394-
.take(1)
395-
.do((data: any) => {
392+
let question = skipAndTake(questions, 1, 2)
393+
subscription = _do.call(question, (data: any) => {
396394
expect(data.length).toBe(2);
397395
expect(data[0].push2).toBe(true);
398396
expect(data[1].push1).toBe(true);
@@ -412,30 +410,26 @@ describe('FirebaseListFactory', () => {
412410
it('should emit unwrapped data by default', (done: any) => {
413411
ref.remove(() => {
414412
ref.push({ unwrapped: true }, () => {
415-
subscription = questions
416-
.take(1)
417-
.do((data: any) => {
418-
expect(data.length).toBe(1);
419-
expect(data[0].unwrapped).toBe(true);
420-
})
421-
.subscribe(() => {
422-
done();
423-
}, done.fail);
413+
subscription = _do.call(skipAndTake(questions, 1), (data: any) => {
414+
expect(data.length).toBe(1);
415+
expect(data[0].unwrapped).toBe(true);
416+
})
417+
.subscribe(() => {
418+
done();
419+
}, done.fail);
424420
});
425421
});
426422
});
427423

428424

429425
it('should emit snapshots if preserveSnapshot option is true', (done: any) => {
430426
refSnapshotted.push('hello snapshot!', () => {
431-
subscription = questionsSnapshotted
432-
.take(1)
433-
.do((data: any) => {
434-
expect(data[0].val()).toEqual('hello snapshot!');
435-
})
436-
.subscribe(() => {
437-
done();
438-
}, done.fail);
427+
subscription = _do.call(skipAndTake(questionsSnapshotted, 1),(data: any) => {
428+
expect(data[0].val()).toEqual('hello snapshot!');
429+
})
430+
.subscribe(() => {
431+
done();
432+
}, done.fail);
439433
});
440434
});
441435

@@ -543,9 +537,13 @@ describe('FirebaseListFactory', () => {
543537

544538
expect(unwrappedValueLol.$key).toEqual('key');
545539
expect(unwrappedValueLol.$value).toEqual('lol');
546-
expect(unwrappedValueLol.$exists()).toEqual(true);
540+
expect(unwrappedValueLol.$exists()).toEqual(true);
547541
});
548542
});
549543

550544
});
551545
});
546+
547+
function skipAndTake<T>(obs: Observable<T>, takeCount: number = 1, skipCount: number = 0) {
548+
return take.call(skip.call(obs, skipCount), takeCount);
549+
}

src/database/firebase_list_factory.ts

+4-5
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import { database } from 'firebase';
55
import { observeQuery } from './query_observable';
66
import { Query, FirebaseListFactoryOpts } from '../interfaces';
77
import * as utils from '../utils';
8-
import 'rxjs/add/operator/mergeMap';
9-
import 'rxjs/add/operator/map';
8+
import { mergeMap } from 'rxjs/operator/mergeMap';
9+
import { map } from 'rxjs/operator/map';
1010

1111
export function FirebaseListFactory (
1212
absoluteUrlOrDbRef:string |
@@ -31,7 +31,7 @@ export function FirebaseListFactory (
3131

3232
const queryObs = observeQuery(query);
3333
return new FirebaseListObservable(ref, subscriber => {
34-
let sub = queryObs.map(query => {
34+
let sub = mergeMap.call(map.call(queryObs, query => {
3535
let queried: firebase.database.Query = ref;
3636
// Only apply the populated keys
3737
// apply ordering and available querying options
@@ -90,8 +90,7 @@ export function FirebaseListFactory (
9090
}
9191

9292
return queried;
93-
})
94-
.mergeMap((queryRef: firebase.database.Reference, ix: number) => {
93+
}), (queryRef: firebase.database.Reference, ix: number) => {
9594
return firebaseListObservable(queryRef, { preserveSnapshot });
9695
})
9796
.subscribe(subscriber);

src/database/firebase_list_observable.spec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { FirebaseListObservable } from './index';
22
import { Observer } from 'rxjs/Observer';
33
import { Observable } from 'rxjs/Observable';
4-
import 'rxjs/add/operator/map';
4+
import { map } from 'rxjs/operator/map';
55
import { database } from 'firebase';
66
import { unwrapMapFn } from '../utils';
77
import {
@@ -46,7 +46,7 @@ describe('FirebaseObservable', () => {
4646
it('should return an instance of FirebaseObservable when calling operators', () => {
4747
O = new FirebaseListObservable(ref, (observer:Observer<any>) => {
4848
});
49-
expect(O.map(noop) instanceof FirebaseListObservable).toBe(true);
49+
expect(map.call(O, noop) instanceof FirebaseListObservable).toBe(true);
5050
});
5151

5252
describe('push', () => {

src/database/firebase_object_factory.ts

-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import * as utils from '../utils';
55
import { Query } from '../interfaces';
66
import { observeQuery } from './query_observable';
77
import { FirebaseObjectFactoryOpts } from '../interfaces';
8-
import 'rxjs/add/operator/mergeMap';
98

109
export function FirebaseObjectFactory (
1110
absoluteUrlOrDbRef: string | firebase.database.Reference,

src/database/firebase_object_observable.spec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
import { COMMON_CONFIG, ANON_AUTH_CONFIG } from '../test-config';
1414
import { FirebaseObjectObservable } from './index';
1515
import { Observer } from 'rxjs/Observer';
16-
import 'rxjs/add/operator/map';
16+
import { map } from 'rxjs/operator/map';
1717
import { database } from 'firebase';
1818

1919
const rootDatabaseUrl = COMMON_CONFIG.databaseURL;
@@ -44,7 +44,7 @@ describe('FirebaseObjectObservable', () => {
4444

4545
it('should return an instance of FirebaseObservable when calling operators', () => {
4646
var O = new FirebaseObjectObservable((observer:Observer<any>) => {});
47-
expect(O.map(noop) instanceof FirebaseObjectObservable).toBe(true);
47+
expect(map.call(O, noop) instanceof FirebaseObjectObservable).toBe(true);
4848
});
4949

5050
describe('set', () => {

src/database/query_observable.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { Observable } from 'rxjs/Observable';
2-
import 'rxjs/add/observable/of';
2+
import { of as observableOf } from 'rxjs/observable/of';
33
import { Operator } from 'rxjs/Operator';
44
import { Observer } from 'rxjs/Observer';
5+
import { combineLatest } from 'rxjs/operator/combineLatest';
56
import { merge } from 'rxjs/operator/merge';
67
import { map } from 'rxjs/operator/map';
78
import {
@@ -13,18 +14,17 @@ import {
1314
LimitToSelection,
1415
Primitive
1516
} from '../interfaces';
16-
import 'rxjs/add/operator/merge';
17-
import 'rxjs/add/operator/combineLatest';
17+
1818

1919
export function observeQuery(query: Query): Observable<ScalarQuery> {
2020
if (!isPresent(query)) {
21-
return Observable.of(null);
21+
return observableOf(null);
2222
}
2323

2424
return Observable.create((observer: Observer<ScalarQuery>) => {
2525

2626
let obs = getOrderObservables(query) as Observable<OrderBySelection>;
27-
obs.combineLatest(
27+
combineLatest.call(obs,
2828
getStartAtObservable(query),
2929
getEndAtObservable(query),
3030
getEqualToObservable(query),
@@ -93,7 +93,7 @@ export function getOrderObservables(query: Query): Observable<OrderBySelection>
9393
if (observables.length === 1) {
9494
return observables[0];
9595
} else if (observables.length > 1) {
96-
return observables[0].merge(observables.slice(1));
96+
return merge.call(observables[0], observables.slice(1));
9797
} else {
9898
return new Observable<OrderBySelection>(subscriber => {
9999
subscriber.next(null);
@@ -110,7 +110,7 @@ export function getLimitToObservables(query: Query): Observable<LimitToSelection
110110
if (observables.length === 1) {
111111
return observables[0];
112112
} else if (observables.length > 1) {
113-
const mergedObs = observables[0].merge(observables.slice(1));
113+
const mergedObs = merge.call(observables[0], observables.slice(1));
114114
return mergedObs;
115115
} else {
116116
return new Observable<LimitToSelection>(subscriber => {

0 commit comments

Comments
 (0)